APPS: replace awkward and error-prone pattern by calls to new app_conf_try_string()

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from https://github.com/openssl/openssl/pull/20971)

9 files changed, 86 insertions, 154 deletions

diff --git a/apps/ca.c b/apps/ca.c
index 91ce4e88ab..8bdbc9a1f0 100644
--- a/apps/ca.c
+++ b/apps/ca.c
@@ -514,9 +514,7 @@ end_of_options:
         && (section = lookup_conf(conf, BASE_SECTION, ENV_DEFAULT_CA)) == NULL)
         goto end;
 
-    p = NCONF_get_string(conf, NULL, "oid_file");
-    if (p == NULL)
-        ERR_clear_error();
+    p = app_conf_try_string(conf, NULL, "oid_file");
     if (p != NULL) {
         BIO *oid_bio = BIO_new_file(p, "r");
 
@@ -534,28 +532,22 @@ end_of_options:
     if (!app_RAND_load())
         goto end;
 
-    f = NCONF_get_string(conf, section, STRING_MASK);
-    if (f == NULL)
-        ERR_clear_error();
+    f = app_conf_try_string(conf, section, STRING_MASK);
     if (f != NULL && !ASN1_STRING_set_default_mask_asc(f)) {
         BIO_printf(bio_err, "Invalid global string mask setting %s\n", f);
         goto end;
     }
 
     if (chtype != MBSTRING_UTF8) {
-        f = NCONF_get_string(conf, section, UTF8_IN);
-        if (f == NULL)
-            ERR_clear_error();
-        else if (strcmp(f, "yes") == 0)
+        f = app_conf_try_string(conf, section, UTF8_IN);
+        if (f != NULL && strcmp(f, "yes") == 0)
             chtype = MBSTRING_UTF8;
     }
 
     db_attr.unique_subject = 1;
-    p = NCONF_get_string(conf, section, ENV_UNIQUE_SUBJECT);
+    p = app_conf_try_string(conf, section, ENV_UNIQUE_SUBJECT);
     if (p != NULL)
         db_attr.unique_subject = parse_yesno(p, 1);
-    else
-        ERR_clear_error();
 
     /*****************************************************************/
     /* report status of cert with serial number given on command line */
@@ -618,20 +610,14 @@ end_of_options:
     if (!selfsign)
         x509p = x509;
 
-    f = NCONF_get_string(conf, BASE_SECTION, ENV_PRESERVE);
-    if (f == NULL)
-        ERR_clear_error();
-    if ((f != NULL) && ((*f == 'y') || (*f == 'Y')))
+    f = app_conf_try_string(conf, BASE_SECTION, ENV_PRESERVE);
+    if (f != NULL && (*f == 'y' || *f == 'Y'))
         preserve = 1;
-    f = NCONF_get_string(conf, BASE_SECTION, ENV_MSIE_HACK);
-    if (f == NULL)
-        ERR_clear_error();
-    if ((f != NULL) && ((*f == 'y') || (*f == 'Y')))
+    f = app_conf_try_string(conf, BASE_SECTION, ENV_MSIE_HACK);
+    if (f != NULL && (*f == 'y' || *f == 'Y'))
         msie_hack = 1;
 
-    f = NCONF_get_string(conf, section, ENV_NAMEOPT);
-    if (f == NULL)
-        ERR_clear_error();
+    f = app_conf_try_string(conf, section, ENV_NAMEOPT);
     if (f != NULL) {
         if (!set_nameopt(f)) {
             BIO_printf(bio_err, "Invalid name options: \"%s\"\n", f);
@@ -640,25 +626,21 @@ end_of_options:
         default_op = 0;
     }
 
-    f = NCONF_get_string(conf, section, ENV_CERTOPT);
+    f = app_conf_try_string(conf, section, ENV_CERTOPT);
     if (f != NULL) {
         if (!set_cert_ex(&certopt, f)) {
             BIO_printf(bio_err, "Invalid certificate options: \"%s\"\n", f);
             goto end;
         }
         default_op = 0;
-    } else {
-        ERR_clear_error();
     }
 
-    f = NCONF_get_string(conf, section, ENV_EXTCOPY);
+    f = app_conf_try_string(conf, section, ENV_EXTCOPY);
     if (f != NULL) {
         if (!set_ext_copy(&ext_copy, f)) {
             BIO_printf(bio_err, "Invalid extension copy option: \"%s\"\n", f);
             goto end;
         }
-    } else {
-        ERR_clear_error();
     }
 
     /*****************************************************************/
@@ -786,11 +768,10 @@ end_of_options:
 
         /* We can have sections in the ext file */
         if (extensions == NULL) {
-            extensions = NCONF_get_string(extfile_conf, "default", "extensions");
-            if (extensions == NULL) {
-                ERR_clear_error();
+            extensions =
+                app_conf_try_string(extfile_conf, "default", "extensions");
+            if (extensions == NULL)
                 extensions = "default";
-            }
         }
     }
 
@@ -827,9 +808,8 @@ end_of_options:
         if (email_dn == 1) {
             char *tmp_email_dn = NULL;
 
-            tmp_email_dn = NCONF_get_string(conf, section, ENV_DEFAULT_EMAIL_DN);
-            if (tmp_email_dn == NULL)
-                ERR_clear_error();
+            tmp_email_dn =
+                app_conf_try_string(conf, section, ENV_DEFAULT_EMAIL_DN);
             if (tmp_email_dn != NULL && strcmp(tmp_email_dn, "no") == 0)
                 email_dn = 0;
         }
@@ -842,10 +822,9 @@ end_of_options:
         if (verbose)
             BIO_printf(bio_err, "policy is %s\n", policy);
 
-        if (NCONF_get_string(conf, section, ENV_RAND_SERIAL) != NULL) {
+        if (app_conf_try_string(conf, section, ENV_RAND_SERIAL) != NULL) {
             rand_ser = 1;
         } else {
-            ERR_clear_error();
             serialfile = lookup_conf(conf, section, ENV_SERIAL);
             if (serialfile == NULL)
                 goto end;
@@ -869,11 +848,8 @@ end_of_options:
              * no '-extfile' option, so we look for extensions in the main
              * configuration file
              */
-            if (extensions == NULL) {
-                extensions = NCONF_get_string(conf, section, ENV_EXTENSIONS);
-                if (extensions == NULL)
-                    ERR_clear_error();
-            }
+            if (extensions == NULL)
+                extensions = app_conf_try_string(conf, section, ENV_EXTENSIONS);
             if (extensions != NULL) {
                 /* Check syntax of config file section */
                 X509V3_CTX ctx;
@@ -890,11 +866,9 @@ end_of_options:
             }
         }
 
-        if (startdate == NULL) {
-            startdate = NCONF_get_string(conf, section, ENV_DEFAULT_STARTDATE);
-            if (startdate == NULL)
-                ERR_clear_error();
-        }
+        if (startdate == NULL)
+            startdate =
+                app_conf_try_string(conf, section, ENV_DEFAULT_STARTDATE);
         if (startdate != NULL && !ASN1_TIME_set_string_X509(NULL, startdate)) {
             BIO_printf(bio_err,
                        "start date is invalid, it should be YYMMDDHHMMSSZ or YYYYMMDDHHMMSSZ\n");
@@ -903,11 +877,8 @@ end_of_options:
         if (startdate == NULL)
             startdate = "today";
 
-        if (enddate == NULL) {
-            enddate = NCONF_get_string(conf, section, ENV_DEFAULT_ENDDATE);
-            if (enddate == NULL)
-                ERR_clear_error();
-        }
+        if (enddate == NULL)
+            enddate = app_conf_try_string(conf, section, ENV_DEFAULT_ENDDATE);
         if (enddate != NULL && !ASN1_TIME_set_string_X509(NULL, enddate)) {
             BIO_printf(bio_err,
                        "end date is invalid, it should be YYMMDDHHMMSSZ or YYYYMMDDHHMMSSZ\n");
@@ -1151,11 +1122,9 @@ end_of_options:
     /*****************************************************************/
     if (gencrl) {
         int crl_v2 = 0;
-        if (crl_ext == NULL) {
-            crl_ext = NCONF_get_string(conf, section, ENV_CRLEXT);
-            if (crl_ext == NULL)
-                ERR_clear_error();
-        }
+
+        if (crl_ext == NULL)
+            crl_ext = app_conf_try_string(conf, section, ENV_CRLEXT);
         if (crl_ext != NULL) {
             /* Check syntax of file */
             X509V3_CTX ctx;
@@ -1170,15 +1139,13 @@ end_of_options:
             }
         }
 
-        crlnumberfile = NCONF_get_string(conf, section, ENV_CRLNUMBER);
+        crlnumberfile = app_conf_try_string(conf, section, ENV_CRLNUMBER);
         if (crlnumberfile != NULL) {
             if ((crlnumber = load_serial(crlnumberfile, NULL, 0, NULL))
                 == NULL) {
                 BIO_printf(bio_err, "error while loading CRL number\n");
                 goto end;
             }
-        } else {
-            ERR_clear_error();
         }
 
         if (!crldays && !crlhours && !crlsec) {
diff --git a/apps/cmp.c b/apps/cmp.c
index 1cd8841b61..a3d0c19dd3 100644
--- a/apps/cmp.c
+++ b/apps/cmp.c
@@ -2187,7 +2187,7 @@ static char *conf_get_string(const CONF *src_conf, const char *groups,
     const char *end = groups + strlen(groups);
 
     while ((end = prev_item(groups, end)) != NULL) {
-        if ((res = NCONF_get_string(src_conf, opt_item, name)) != NULL)
+        if ((res = app_conf_try_string(src_conf, opt_item, name)) != NULL)
             return res;
     }
     return res;
diff --git a/apps/include/apps.h b/apps/include/apps.h
index b48937a8c2..c9e0d440e8 100644
--- a/apps/include/apps.h
+++ b/apps/include/apps.h
@@ -65,6 +65,7 @@ BIO *dup_bio_err(int format);
 BIO *bio_open_owner(const char *filename, int format, int private);
 BIO *bio_open_default(const char *filename, char mode, int format);
 BIO *bio_open_default_quiet(const char *filename, char mode, int format);
+char *app_conf_try_string(const CONF *cnf, const char *group, const char *name);
 CONF *app_load_config_bio(BIO *in, const char *filename);
 # define app_load_config(filename) app_load_config_internal(filename, 0)
 # define app_load_config_quiet(filename) app_load_config_internal(filename, 1)
diff --git a/apps/lib/app_rand.c b/apps/lib/app_rand.c
index ad93858bfd..9691e71d7c 100644
--- a/apps/lib/app_rand.c
+++ b/apps/lib/app_rand.c
@@ -18,12 +18,10 @@ static STACK_OF(OPENSSL_STRING) *randfiles;
 
 void app_RAND_load_conf(CONF *c, const char *section)
 {
-    const char *randfile = NCONF_get_string(c, section, "RANDFILE");
+    const char *randfile = app_conf_try_string(c, section, "RANDFILE");
 
-    if (randfile == NULL) {
-        ERR_clear_error();
+    if (randfile == NULL)
         return;
-    }
     if (RAND_load_file(randfile, -1) < 0) {
         BIO_printf(bio_err, "Can't load %s into RNG\n", randfile);
         ERR_print_errors(bio_err);
diff --git a/apps/lib/apps.c b/apps/lib/apps.c
index 4a749b0df3..bfa983a351 100644
--- a/apps/lib/apps.c
+++ b/apps/lib/apps.c
@@ -336,6 +336,20 @@ static char *app_get_pass(const char *arg, int keepbio)
     return OPENSSL_strdup(tpass);
 }
 
+char *app_conf_try_string(const CONF *conf, const char *group, const char *name)
+{
+    char *res;
+
+    ERR_set_mark();
+    res = NCONF_get_string(conf, group, name);
+    if (res == NULL)
+        ERR_pop_to_mark();
+    else
+        ERR_clear_last_mark();
+    return res;
+}
+
+
 CONF *app_load_config_bio(BIO *in, const char *filename)
 {
     long errorline = -1;
@@ -416,10 +430,8 @@ int add_oid_section(CONF *conf)
     CONF_VALUE *cnf;
     int i;
 
-    if ((p = NCONF_get_string(conf, NULL, "oid_section")) == NULL) {
-        ERR_clear_error();
+    if ((p = app_conf_try_string(conf, NULL, "oid_section")) == NULL)
         return 1;
-    }
     if ((sktmp = NCONF_get_section(conf, p)) == NULL) {
         BIO_printf(bio_err, "problem loading oid section %s\n", p);
         return 0;
@@ -1684,12 +1696,11 @@ CA_DB *load_index(const char *dbfile, DB_ATTR *db_attr)
     else
         retdb->attributes.unique_subject = 1;
 
-    if (dbattr_conf) {
-        char *p = NCONF_get_string(dbattr_conf, NULL, "unique_subject");
+    if (dbattr_conf != NULL) {
+        char *p = app_conf_try_string(dbattr_conf, NULL, "unique_subject");
 
-        if (p) {
+        if (p != NULL)
             retdb->attributes.unique_subject = parse_yesno(p, 1);
-        }
     }
 
     retdb->dbfname = OPENSSL_strdup(dbfile);
diff --git a/apps/pkcs12.c b/apps/pkcs12.c
index 4f2d3f7f2e..b8efb536dd 100644
--- a/apps/pkcs12.c
+++ b/apps/pkcs12.c
@@ -683,7 +683,8 @@ int pkcs12_main(int argc, char **argv)
         if (!app_load_modules(conf))
             goto export_end;
         /* Find the cert bag section */
-        if ((cb_attr = NCONF_get_string(conf, "pkcs12", "certBagAttr")) != NULL) {
+        cb_attr = app_conf_try_string(conf, "pkcs12", "certBagAttr");
+        if (cb_attr != NULL) {
             if ((cb_sk = NCONF_get_section(conf, cb_attr)) != NULL) {
                 for (i = 0; i < sk_CONF_VALUE_num(cb_sk); i++) {
                     val = sk_CONF_VALUE_value(cb_sk, i);
@@ -695,8 +696,6 @@ int pkcs12_main(int argc, char **argv)
             } else {
                 ERR_clear_error();
             }
-        } else {
-            ERR_clear_error();
         }
 
         p12 = PKCS12_create_ex2(cpass, name, key, ee_cert, certs,
diff --git a/apps/req.c b/apps/req.c
index f24101cefa..11ecf6cad7 100644
--- a/apps/req.c
+++ b/apps/req.c
@@ -518,9 +518,7 @@ int req_main(int argc, char **argv)
         goto end;
 
     if (req_conf != NULL) {
-        p = NCONF_get_string(req_conf, NULL, "oid_file");
-        if (p == NULL)
-            ERR_clear_error();
+        p = app_conf_try_string(req_conf, NULL, "oid_file");
         if (p != NULL) {
             BIO *oid_bio = BIO_new_file(p, "r");
 
@@ -543,19 +541,14 @@ int req_main(int argc, char **argv)
             goto opthelp;
     } else {
         /* No digest specified, default to configuration */
-        p = NCONF_get_string(req_conf, section, "default_md");
-        if (p == NULL)
-            ERR_clear_error();
-        else
+        p = app_conf_try_string(req_conf, section, "default_md");
+        if (p != NULL)
             digest = p;
     }
 
-    if (extsect == NULL) {
-        extsect = NCONF_get_string(req_conf, section,
+    if (extsect == NULL)
+        extsect = app_conf_try_string(req_conf, section,
                                    gen_x509 ? V3_EXTENSIONS : REQ_EXTENSIONS);
-        if (extsect == NULL)
-            ERR_clear_error();
-    }
     if (extsect != NULL) {
         /* Check syntax of extension section in config file */
         X509V3_CTX ctx;
@@ -581,34 +574,23 @@ int req_main(int argc, char **argv)
         }
     }
 
-    if (passin == NULL) {
+    if (passin == NULL)
         passin = nofree_passin =
-            NCONF_get_string(req_conf, section, "input_password");
-        if (passin == NULL)
-            ERR_clear_error();
-    }
+            app_conf_try_string(req_conf, section, "input_password");
 
-    if (passout == NULL) {
+    if (passout == NULL)
         passout = nofree_passout =
-            NCONF_get_string(req_conf, section, "output_password");
-        if (passout == NULL)
-            ERR_clear_error();
-    }
-
-    p = NCONF_get_string(req_conf, section, STRING_MASK);
-    if (p == NULL)
-        ERR_clear_error();
+            app_conf_try_string(req_conf, section, "output_password");
 
+    p = app_conf_try_string(req_conf, section, STRING_MASK);
     if (p != NULL && !ASN1_STRING_set_default_mask_asc(p)) {
         BIO_printf(bio_err, "Invalid global string mask setting %s\n", p);
         goto end;
     }
 
     if (chtype != MBSTRING_UTF8) {
-        p = NCONF_get_string(req_conf, section, UTF8_IN);
-        if (p == NULL)
-            ERR_clear_error();
-        else if (strcmp(p, "yes") == 0)
+        p = app_conf_try_string(req_conf, section, UTF8_IN);
+        if (p != NULL && strcmp(p, "yes") == 0)
             chtype = MBSTRING_UTF8;
     }
 
@@ -678,11 +660,8 @@ int req_main(int argc, char **argv)
         EVP_PKEY_CTX_free(genctx);
         genctx = NULL;
     }
-    if (keyout == NULL && keyfile == NULL) {
-        keyout = NCONF_get_string(req_conf, section, KEYFILE);
-        if (keyout == NULL)
-            ERR_clear_error();
-    }
+    if (keyout == NULL && keyfile == NULL)
+        keyout = app_conf_try_string(req_conf, section, KEYFILE);
 
     if (pkey != NULL && (keyfile == NULL || keyout != NULL)) {
         if (verbose) {
@@ -696,14 +675,10 @@ int req_main(int argc, char **argv)
         if (out == NULL)
             goto end;
 
-        p = NCONF_get_string(req_conf, section, "encrypt_rsa_key");
-        if (p == NULL) {
-            ERR_clear_error();
-            p = NCONF_get_string(req_conf, section, "encrypt_key");
-            if (p == NULL)
-                ERR_clear_error();
-        }
-        if ((p != NULL) && (strcmp(p, "no") == 0))
+        p = app_conf_try_string(req_conf, section, "encrypt_rsa_key");
+        if (p == NULL)
+            p = app_conf_try_string(req_conf, section, "encrypt_key");
+        if (p != NULL && strcmp(p, "no") == 0)
             cipher = NULL;
         if (noenc)
             cipher = NULL;
@@ -1072,16 +1047,12 @@ static int make_REQ(X509_REQ *req, EVP_PKEY *pkey, X509_NAME *fsubj,
     STACK_OF(CONF_VALUE) *dn_sk = NULL, *attr_sk = NULL;
     char *tmp, *dn_sect, *attr_sect;
 
-    tmp = NCONF_get_string(req_conf, section, PROMPT);
-    if (tmp == NULL)
-        ERR_clear_error();
-    if ((tmp != NULL) && strcmp(tmp, "no") == 0)
+    tmp = app_conf_try_string(req_conf, section, PROMPT);
+    if (tmp != NULL && strcmp(tmp, "no") == 0)
         no_prompt = 1;
 
-    dn_sect = NCONF_get_string(req_conf, section, DISTINGUISHED_NAME);
-    if (dn_sect == NULL) {
-        ERR_clear_error();
-    } else {
+    dn_sect = app_conf_try_string(req_conf, section, DISTINGUISHED_NAME);
+    if (dn_sect != NULL) {
         dn_sk = NCONF_get_section(req_conf, dn_sect);
         if (dn_sk == NULL) {
             BIO_printf(bio_err, "Unable to get '%s' section\n", dn_sect);
@@ -1089,10 +1060,8 @@ static int make_REQ(X509_REQ *req, EVP_PKEY *pkey, X509_NAME *fsubj,
         }
     }
 
-    attr_sect = NCONF_get_string(req_conf, section, ATTRIBUTES);
-    if (attr_sect == NULL) {
-        ERR_clear_error();
-    } else {
+    attr_sect = app_conf_try_string(req_conf, section, ATTRIBUTES);
+    if (attr_sect != NULL) {
         attr_sk = NCONF_get_section(req_conf, attr_sect);
         if (attr_sk == NULL) {
             BIO_printf(bio_err, "Unable to get '%s' section\n", attr_sect);
@@ -1188,17 +1157,13 @@ static int prompt_info(X509_REQ *req,
                 goto start;
             if (!join(buf, sizeof(buf), v->name, "_default", "Name"))
                 return 0;
-            if ((def = NCONF_get_string(req_conf, dn_sect, buf)) == NULL) {
-                ERR_clear_error();
+            if ((def = app_conf_try_string(req_conf, dn_sect, buf)) == NULL)
                 def = "";
-            }
 
             if (!join(buf, sizeof(buf), v->name, "_value", "Name"))
                 return 0;
-            if ((value = NCONF_get_string(req_conf, dn_sect, buf)) == NULL) {
-                ERR_clear_error();
+            if ((value = app_conf_try_string(req_conf, dn_sect, buf)) == NULL)
                 value = NULL;
-            }
 
             if (!join(buf, sizeof(buf), v->name, "_min", "Name"))
                 return 0;
@@ -1246,19 +1211,13 @@ static int prompt_info(X509_REQ *req,
 
                 if (!join(buf, sizeof(buf), type, "_default", "Name"))
                     return 0;
-                if ((def = NCONF_get_string(req_conf, attr_sect, buf))
-                    == NULL) {
-                    ERR_clear_error();
+                def = app_conf_try_string(req_conf, attr_sect, buf);
+                if (def == NULL)
                     def = "";
-                }
 
                 if (!join(buf, sizeof(buf), type, "_value", "Name"))
                     return 0;
-                if ((value = NCONF_get_string(req_conf, attr_sect, buf))
-                    == NULL) {
-                    ERR_clear_error();
-                    value = NULL;
-                }
+                value = app_conf_try_string(req_conf, attr_sect, buf);
 
                 if (!join(buf, sizeof(buf), type, "_min", "Name"))
                     return 0;
diff --git a/apps/ts.c b/apps/ts.c
index 78c3aacced..a4218c9bf2 100644
--- a/apps/ts.c
+++ b/apps/ts.c
@@ -375,7 +375,7 @@ static CONF *load_config_file(const char *configfile)
         const char *p;
 
         BIO_printf(bio_err, "Using configuration from %s\n", configfile);
-        p = NCONF_get_string(conf, NULL, ENV_OID_FILE);
+        p = app_conf_try_string(conf, NULL, ENV_OID_FILE);
         if (p != NULL) {
             BIO *oid_bio = BIO_new_file(p, "r");
             if (!oid_bio)
@@ -384,8 +384,7 @@ static CONF *load_config_file(const char *configfile)
                 OBJ_create_objects(oid_bio);
                 BIO_free_all(oid_bio);
             }
-        } else
-            ERR_clear_error();
+        }
         if (!add_oid_section(conf))
             ERR_print_errors(bio_err);
     }
diff --git a/apps/x509.c b/apps/x509.c
index 7a935e1f70..35f788c6dd 100644
--- a/apps/x509.c
+++ b/apps/x509.c
@@ -687,11 +687,9 @@ int x509_main(int argc, char **argv)
         if ((extconf = app_load_config(extfile)) == NULL)
             goto end;
         if (extsect == NULL) {
-            extsect = NCONF_get_string(extconf, "default", "extensions");
-            if (extsect == NULL) {
-                ERR_clear_error();
+            extsect = app_conf_try_string(extconf, "default", "extensions");
+            if (extsect == NULL)
                 extsect = "default";
-            }
         }
         X509V3_set_ctx_test(&ctx2);
         X509V3_set_nconf(&ctx2, extconf);