apps/x509.c: Improve print_name() and coding style of large print loop in x509_main()

Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14340)

5 files changed, 71 insertions, 119 deletions

diff --git a/apps/crl.c b/apps/crl.c
index dd9d41e8ea..1f12e24a4b 100644
--- a/apps/crl.c
+++ b/apps/crl.c
@@ -286,8 +286,7 @@ int crl_main(int argc, char **argv)
     if (num) {
         for (i = 1; i <= num; i++) {
             if (issuer == i) {
-                print_name(bio_out, "issuer=", X509_CRL_get_issuer(x),
-                           get_nameopt());
+                print_name(bio_out, "issuer=", X509_CRL_get_issuer(x));
             }
             if (crlnumber == i) {
                 ASN1_INTEGER *crlnum;
diff --git a/apps/include/apps.h b/apps/include/apps.h
index 45a9c4e758..8c365c44bd 100644
--- a/apps/include/apps.h
+++ b/apps/include/apps.h
@@ -94,8 +94,7 @@ int wrap_password_callback(char *buf, int bufsiz, int verify, void *cb_data);
 
 int chopup_args(ARGS *arg, char *buf);
 int dump_cert_text(BIO *out, X509 *x);
-void print_name(BIO *out, const char *title, const X509_NAME *nm,
-                unsigned long lflags);
+void print_name(BIO *out, const char *title, const X509_NAME *nm);
 void print_bignum_var(BIO *, const BIGNUM *, const char*,
                       int, unsigned char *);
 void print_array(BIO *, const char *, int, const unsigned char *);
diff --git a/apps/lib/apps.c b/apps/lib/apps.c
index 7c1015737d..3f1cf5f247 100644
--- a/apps/lib/apps.c
+++ b/apps/lib/apps.c
@@ -188,9 +188,9 @@ unsigned long get_nameopt(void)
 
 int dump_cert_text(BIO *out, X509 *x)
 {
-    print_name(out, "subject=", X509_get_subject_name(x), get_nameopt());
+    print_name(out, "subject=", X509_get_subject_name(x));
     BIO_puts(out, "\n");
-    print_name(out, "issuer=", X509_get_issuer_name(x), get_nameopt());
+    print_name(out, "issuer=", X509_get_issuer_name(x));
     BIO_puts(out, "\n");
 
     return 0;
@@ -1071,14 +1071,13 @@ static int set_table_opts(unsigned long *flags, const char *arg,
     return 0;
 }
 
-void print_name(BIO *out, const char *title, const X509_NAME *nm,
-                unsigned long lflags)
+void print_name(BIO *out, const char *title, const X509_NAME *nm)
 {
     char *buf;
     char mline = 0;
     int indent = 0;
-
-    if (title)
+    unsigned long lflags = get_nameopt();
+    if (title != NULL)
         BIO_puts(out, title);
     if ((lflags & XN_FLAG_SEP_MASK) == XN_FLAG_SEP_MULTILINE) {
         mline = 1;
diff --git a/apps/req.c b/apps/req.c
index 881cbb45c7..4056b18f51 100644
--- a/apps/req.c
+++ b/apps/req.c
@@ -921,9 +921,8 @@ int req_main(int argc, char **argv)
 
     if (subj != NULL && !newreq && !gen_x509) {
         if (verbose) {
-            BIO_printf(bio_err, "Modifying subject of certificate request\n");
-            print_name(bio_err, "Old subject=",
-                       X509_REQ_get_subject_name(req), get_nameopt());
+            BIO_printf(out, "Modifying subject of certificate request\n");
+            print_name(out, "Old subject=", X509_REQ_get_subject_name(req));
         }
 
         if (!X509_REQ_set_subject_name(req, fsubj)) {
@@ -932,8 +931,7 @@ int req_main(int argc, char **argv)
         }
 
         if (verbose) {
-            print_name(bio_err, "New subject=",
-                       X509_REQ_get_subject_name(req), get_nameopt());
+            print_name(out, "New subject=", X509_REQ_get_subject_name(req));
         }
     }
 
@@ -996,12 +994,9 @@ int req_main(int argc, char **argv)
     }
 
     if (subject) {
-        if (gen_x509)
-            print_name(out, "subject=", X509_get_subject_name(new_x509),
-                       get_nameopt());
-        else
-            print_name(out, "subject=", X509_REQ_get_subject_name(req),
-                       get_nameopt());
+        print_name(out, "subject=", gen_x509
+                   ? X509_get_subject_name(new_x509)
+                   : X509_REQ_get_subject_name(req));
     }
 
     if (modulus) {
diff --git a/apps/x509.c b/apps/x509.c
index 8a48394e4d..152537f90a 100644
--- a/apps/x509.c
+++ b/apps/x509.c
@@ -245,6 +245,7 @@ int x509_main(int argc, char **argv)
     int ext_copy = EXT_COPY_UNSET;
     X509V3_CTX ext_ctx;
     EVP_PKEY *signkey = NULL, *CAkey = NULL, *pubkey = NULL;
+    EVP_PKEY *pkey;
     int newcert = 0;
     char *subj = NULL, *digestname = NULL;
     X509_NAME *fsubj = NULL;
@@ -270,7 +271,7 @@ int x509_main(int argc, char **argv)
     int next_serial = 0, subject_hash = 0, issuer_hash = 0, ocspid = 0;
     int noout = 0, CA_createserial = 0, email = 0;
     int ocsp_uri = 0, trustout = 0, clrtrust = 0, clrreject = 0, aliasout = 0;
-    int ret = 1, i, num = 0, badsig = 0, clrext = 0, nocert = 0;
+    int ret = 1, i, j, num = 0, badsig = 0, clrext = 0, nocert = 0;
     int text = 0, serial = 0, subject = 0, issuer = 0, startdate = 0, ext = 0;
     int enddate = 0;
     time_t checkoffset = 0;
@@ -403,30 +404,25 @@ int x509_main(int argc, char **argv)
             subj = opt_arg();
             break;
         case OPT_ADDTRUST:
+            if (trust == NULL && (trust = sk_ASN1_OBJECT_new_null()) == NULL)
+                goto end;
             if ((objtmp = OBJ_txt2obj(opt_arg(), 0)) == NULL) {
-                BIO_printf(bio_err,
-                           "%s: Invalid trust object value %s\n",
+                BIO_printf(bio_err, "%s: Invalid trust object value %s\n",
                            prog, opt_arg());
                 goto opthelp;
             }
-            if (trust == NULL && (trust = sk_ASN1_OBJECT_new_null()) == NULL)
-                goto end;
             sk_ASN1_OBJECT_push(trust, objtmp);
-            objtmp = NULL;
             trustout = 1;
             break;
         case OPT_ADDREJECT:
+            if (reject == NULL && (reject = sk_ASN1_OBJECT_new_null()) == NULL)
+                goto end;
             if ((objtmp = OBJ_txt2obj(opt_arg(), 0)) == NULL) {
-                BIO_printf(bio_err,
-                           "%s: Invalid reject object value %s\n",
+                BIO_printf(bio_err, "%s: Invalid reject object value %s\n",
                            prog, opt_arg());
                 goto opthelp;
             }
-            if (reject == NULL
-                && (reject = sk_ASN1_OBJECT_new_null()) == NULL)
-                goto end;
             sk_ASN1_OBJECT_push(reject, objtmp);
-            objtmp = NULL;
             trustout = 1;
             break;
         case OPT_SETALIAS:
@@ -674,32 +670,24 @@ int x509_main(int argc, char **argv)
     }
 
     if (reqfile) {
-        EVP_PKEY *pkey;
-
         req = load_csr(infile, informat, "certificate request input");
         if (req == NULL)
             goto end;
 
         if ((pkey = X509_REQ_get0_pubkey(req)) == NULL) {
-            BIO_printf(bio_err, "Error unpacking public key\n");
+            BIO_printf(bio_err, "Error unpacking public key from CSR\n");
             goto end;
         }
         i = do_X509_REQ_verify(req, pkey, vfyopts);
-        if (i < 0) {
-            BIO_printf(bio_err,
-                       "Error while verifying certificate request self-signature\n");
+        if (i <= 0) {
+            BIO_printf(bio_err, i < 0
+                       ? "Error while verifying certificate request self-signature\n"
+                       : "Certificate request self-signature did not match the contents\n");
             goto end;
         }
-        if (i == 0) {
-            BIO_printf(bio_err,
-                       "Certificate request self-signature did not match the contents\n");
-            goto end;
-        } else {
-            BIO_printf(bio_err, "Certificate request self-signature ok\n");
-        }
+        BIO_printf(out, "Certificate request self-signature ok\n");
 
-        print_name(bio_err, "subject=", X509_REQ_get_subject_name(req),
-                   get_nameopt());
+        print_name(out, "subject=", X509_REQ_get_subject_name(req));
     } else if (!x509toreq && ext_copy != EXT_COPY_UNSET) {
         BIO_printf(bio_err, "Warning: ignoring -copy_extensions since neither -x509toreq nor -req is given\n");
     }
@@ -768,19 +756,13 @@ int x509_main(int argc, char **argv)
         X509_reject_clear(x);
 
     if (trust != NULL) {
-        for (i = 0; i < sk_ASN1_OBJECT_num(trust); i++) {
-            objtmp = sk_ASN1_OBJECT_value(trust, i);
-            X509_add1_trust_object(x, objtmp);
-        }
-        objtmp = NULL;
+        for (i = 0; i < sk_ASN1_OBJECT_num(trust); i++)
+            X509_add1_trust_object(x, sk_ASN1_OBJECT_value(trust, i));
     }
 
     if (reject != NULL) {
-        for (i = 0; i < sk_ASN1_OBJECT_num(reject); i++) {
-            objtmp = sk_ASN1_OBJECT_value(reject, i);
-            X509_add1_reject_object(x, objtmp);
-        }
-        objtmp = NULL;
+        for (i = 0; i < sk_ASN1_OBJECT_num(reject); i++)
+            X509_add1_reject_object(x, sk_ASN1_OBJECT_value(reject, i));
     }
 
     if (clrext && ext_names != NULL)
@@ -824,6 +806,12 @@ int x509_main(int argc, char **argv)
 
     /* At this point the contents of the certificate x have been finished. */
 
+    pkey = X509_get0_pubkey(x);
+    if ((print_pubkey != 0 || modulus != 0) && pkey == NULL) {
+        BIO_printf(bio_err, "Error getting public key\n");
+        goto end;
+    }
+
     if (x509toreq) { /* also works in conjunction with -req */
         if (signkey == NULL) {
             BIO_printf(bio_err, "Must specify request key using -signkey\n");
@@ -890,77 +878,60 @@ int x509_main(int argc, char **argv)
 
     /* Process print options in the given order, as indicated by index i */
     for (i = 1; i <= num; i++) {
-        if (issuer == i) {
-            print_name(out, "issuer=", X509_get_issuer_name(x), get_nameopt());
-        } else if (subject == i) {
-            print_name(out, "subject=", X509_get_subject_name(x), get_nameopt());
-        } else if (serial == i) {
+        if (i == issuer) {
+            print_name(out, "issuer=", X509_get_issuer_name(x));
+        } else if (i == subject) {
+            print_name(out, "subject=", X509_get_subject_name(x));
+        } else if (i == serial) {
             BIO_printf(out, "serial=");
             i2a_ASN1_INTEGER(out, X509_get0_serialNumber(x));
             BIO_printf(out, "\n");
-        } else if (next_serial == i) {
+        } else if (i == next_serial) {
             ASN1_INTEGER *ser = X509_get_serialNumber(x);
             BIGNUM *bnser = ASN1_INTEGER_to_BN(ser, NULL);
 
-            if (!bnser)
+            if (bnser == NULL)
                 goto end;
             if (!BN_add_word(bnser, 1))
                 goto end;
             ser = BN_to_ASN1_INTEGER(bnser, NULL);
-            if (!ser)
+            if (ser == NULL)
                 goto end;
             BN_free(bnser);
             i2a_ASN1_INTEGER(out, ser);
             ASN1_INTEGER_free(ser);
             BIO_puts(out, "\n");
-        } else if (email == i || ocsp_uri == i) {
-            STACK_OF(OPENSSL_STRING) *emlst;
-            int j;
+        } else if (i == email || i == ocsp_uri) {
+            STACK_OF(OPENSSL_STRING) *emlst =
+                i == email ? X509_get1_email(x) : X509_get1_ocsp(x);
 
-            if (email == i)
-                emlst = X509_get1_email(x);
-            else
-                emlst = X509_get1_ocsp(x);
             for (j = 0; j < sk_OPENSSL_STRING_num(emlst); j++)
                 BIO_printf(out, "%s\n", sk_OPENSSL_STRING_value(emlst, j));
             X509_email_free(emlst);
-        } else if (aliasout == i) {
-            unsigned char *alstr;
+        } else if (i == aliasout) {
+            unsigned char *alstr = X509_alias_get0(x, NULL);
 
-            alstr = X509_alias_get0(x, NULL);
             if (alstr)
                 BIO_printf(out, "%s\n", alstr);
             else
                 BIO_puts(out, "<No Alias>\n");
-        } else if (subject_hash == i) {
+        } else if (i == subject_hash) {
             BIO_printf(out, "%08lx\n", X509_subject_name_hash(x));
 #ifndef OPENSSL_NO_MD5
-        } else if (subject_hash_old == i) {
+        } else if (i == subject_hash_old) {
             BIO_printf(out, "%08lx\n", X509_subject_name_hash_old(x));
 #endif
-        } else if (issuer_hash == i) {
+        } else if (i == issuer_hash) {
             BIO_printf(out, "%08lx\n", X509_issuer_name_hash(x));
 #ifndef OPENSSL_NO_MD5
-        } else if (issuer_hash_old == i) {
+        } else if (i == issuer_hash_old) {
             BIO_printf(out, "%08lx\n", X509_issuer_name_hash_old(x));
 #endif
-        } else if (pprint == i) {
-            X509_PURPOSE *ptmp;
-            int j;
-
+        } else if (i == pprint) {
             BIO_printf(out, "Certificate purposes:\n");
-            for (j = 0; j < X509_PURPOSE_get_count(); j++) {
-                ptmp = X509_PURPOSE_get0(j);
-                purpose_print(out, x, ptmp);
-            }
-        } else if (modulus == i) {
-            EVP_PKEY *pkey;
-
-            pkey = X509_get0_pubkey(x);
-            if (pkey == NULL) {
-                BIO_printf(bio_err, "Modulus unavailable: cannot get key\n");
-                goto end;
-            }
+            for (j = 0; j < X509_PURPOSE_get_count(); j++)
+                purpose_print(out, x, X509_PURPOSE_get0(j));
+        } else if (i == modulus) {
             BIO_printf(out, "Modulus=");
             if (EVP_PKEY_is_a(pkey, "RSA")) {
                 BIGNUM *n;
@@ -972,7 +943,7 @@ int x509_main(int argc, char **argv)
             } else if (EVP_PKEY_is_a(pkey, "DSA")) {
                 BIGNUM *dsapub;
 
-                /* Every DSA key has an 'pub' */
+                /* Every DSA key has a 'pub' */
                 EVP_PKEY_get_bn_param(pkey, "pub", &dsapub);
                 BN_print(out, dsapub);
                 BN_free(dsapub);
@@ -980,27 +951,19 @@ int x509_main(int argc, char **argv)
                 BIO_printf(out, "No modulus for this public key type");
             }
             BIO_printf(out, "\n");
-        } else if (print_pubkey == i) {
-            EVP_PKEY *pkey;
-
-            pkey = X509_get0_pubkey(x);
-            if (pkey == NULL) {
-                BIO_printf(bio_err, "Error getting public key\n");
-                goto end;
-            }
+        } else if (i == print_pubkey) {
             PEM_write_bio_PUBKEY(out, pkey);
-        } else if (text == i) {
+        } else if (i == text) {
             X509_print_ex(out, x, get_nameopt(), certflag);
-        } else if (startdate == i) {
+        } else if (i == startdate) {
             BIO_puts(out, "notBefore=");
             ASN1_TIME_print(out, X509_get0_notBefore(x));
             BIO_puts(out, "\n");
-        } else if (enddate == i) {
+        } else if (i == enddate) {
             BIO_puts(out, "notAfter=");
             ASN1_TIME_print(out, X509_get0_notAfter(x));
             BIO_puts(out, "\n");
-        } else if (fingerprint == i) {
-            int j;
+        } else if (i == fingerprint) {
             unsigned int n;
             unsigned char md[EVP_MAX_MD_SIZE];
             const EVP_MD *fdig = digest;
@@ -1016,9 +979,9 @@ int x509_main(int argc, char **argv)
             for (j = 0; j < (int)n; j++) {
                 BIO_printf(out, "%02X%c", md[j], (j + 1 == (int)n) ? '\n' : ':');
             }
-        } else if (ocspid == i) {
+        } else if (i == ocspid) {
             X509_ocspid_print(out, x);
-        } else if (ext == i) {
+        } else if (i == ext) {
             print_x509v3_exts(out, x, ext_names);
         }
     }
@@ -1026,13 +989,11 @@ int x509_main(int argc, char **argv)
     if (checkend) {
         time_t tcheck = time(NULL) + checkoffset;
 
-        if (X509_cmp_time(X509_get0_notAfter(x), &tcheck) < 0) {
+        ret = X509_cmp_time(X509_get0_notAfter(x), &tcheck) < 0;
+        if (ret)
             BIO_printf(out, "Certificate will expire\n");
-            ret = 1;
-        } else {
+        else
             BIO_printf(out, "Certificate will not expire\n");
-            ret = 0;
-        }
         goto end;
     }
 
@@ -1079,7 +1040,6 @@ int x509_main(int argc, char **argv)
     ASN1_INTEGER_free(sno);
     sk_ASN1_OBJECT_pop_free(trust, ASN1_OBJECT_free);
     sk_ASN1_OBJECT_pop_free(reject, ASN1_OBJECT_free);
-    ASN1_OBJECT_free(objtmp);
     release_engine(e);
     clear_free(passin);
     return ret;
@@ -1144,7 +1104,7 @@ static int callb(int ok, X509_STORE_CTX *ctx)
         return 0;
     } else {
         err_cert = X509_STORE_CTX_get_current_cert(ctx);
-        print_name(bio_err, NULL, X509_get_subject_name(err_cert), 0);
+        print_name(bio_err, "subject=", X509_get_subject_name(err_cert));
         BIO_printf(bio_err,
                    "Error with certificate - error %d at depth %d\n%s\n", err,
                    X509_STORE_CTX_get_error_depth(ctx),