Fix some magic values about revocation info type...

Add comments, document -valid option.
Add some const qualifiers.

Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1560)

1 files changed, 47 insertions, 52 deletions

diff --git a/apps/ca.c b/apps/ca.c
index 34dfd9b956..ecd628ff62 100644
--- a/apps/ca.c
+++ b/apps/ca.c
@@ -82,12 +82,14 @@
 #define ENV_DATABASE            "database"
 
 /* Additional revocation information types */
-
-#define REV_NONE                0 /* No additional information */
-#define REV_CRL_REASON          1 /* Value is CRL reason code */
-#define REV_HOLD                2 /* Value is hold instruction */
-#define REV_KEY_COMPROMISE      3 /* Value is cert key compromise time */
-#define REV_CA_COMPROMISE       4 /* Value is CA key compromise time */
+typedef enum {
+    REV_VALID             = -1, /* Valid (not-revoked) status */
+    REV_NONE              = 0, /* No additional information */
+    REV_CRL_REASON        = 1, /* Value is CRL reason code */
+    REV_HOLD              = 2, /* Value is hold instruction */
+    REV_KEY_COMPROMISE    = 3, /* Value is cert key compromise time */
+    REV_CA_COMPROMISE     = 4  /* Value is CA key compromise time */
+} REVINFO_TYPE;
 
 static char *lookup_conf(const CONF *conf, const char *group, const char *tag);
 
@@ -117,7 +119,6 @@ static int certify_spkac(X509 **xret, const char *infile, EVP_PKEY *pkey,
                          const char *enddate, long days, const char *ext_sect, CONF *conf,
                          int verbose, unsigned long certopt,
                          unsigned long nameopt, int default_op, int ext_copy);
-static void write_new_certificate(BIO *bp, X509 *x, int output_der, int notext);
 static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509,
                    const EVP_MD *dgst, STACK_OF(OPENSSL_STRING) *sigopts,
                    STACK_OF(CONF_VALUE) *policy, CA_DB *db, BIGNUM *serial,
@@ -126,13 +127,15 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509,
                    int batch, int verbose, X509_REQ *req, const char *ext_sect,
                    CONF *conf, unsigned long certopt, unsigned long nameopt,
                    int default_op, int ext_copy, int selfsign);
-static int do_revoke(X509 *x509, CA_DB *db, int ext, char *extval);
 static int get_certificate_status(const char *ser_status, CA_DB *db);
 static int do_updatedb(CA_DB *db);
 static int check_time_format(const char *str);
-char *make_revocation_str(int rev_type, char *rev_arg);
-int make_revoked(X509_REVOKED *rev, const char *str);
+static int do_revoke(X509 *x509, CA_DB *db, REVINFO_TYPE rev_type,
+                     const char *extval);
+static char *make_revocation_str(REVINFO_TYPE rev_type, const char *rev_arg);
+static int make_revoked(X509_REVOKED *rev, const char *str);
 static int old_entry_print(const ASN1_OBJECT *obj, const ASN1_STRING *str);
+static void write_new_certificate(BIO *bp, X509 *x, int output_der, int notext);
 
 static CONF *extconf = NULL;
 static int preserve = 0;
@@ -148,8 +151,8 @@ typedef enum OPTION_choice {
     OPT_GENCRL, OPT_MSIE_HACK, OPT_CRLDAYS, OPT_CRLHOURS, OPT_CRLSEC,
     OPT_INFILES, OPT_SS_CERT, OPT_SPKAC, OPT_REVOKE, OPT_VALID,
     OPT_EXTENSIONS, OPT_EXTFILE, OPT_STATUS, OPT_UPDATEDB, OPT_CRLEXTS,
-    OPT_CRL_REASON, OPT_CRL_HOLD, OPT_CRL_COMPROMISE,
-    OPT_CRL_CA_COMPROMISE
+    /* Do not change the order here; see related case statements below */
+    OPT_CRL_REASON, OPT_CRL_HOLD, OPT_CRL_COMPROMISE, OPT_CRL_CA_COMPROMISE
 } OPTION_CHOICE;
 
 OPTIONS ca_options[] = {
@@ -250,10 +253,11 @@ int ca_main(int argc, char **argv)
     int batch = 0, default_op = 1, doupdatedb = 0, ext_copy = EXT_COPY_NONE;
     int keyformat = FORMAT_PEM, multirdn = 0, notext = 0, output_der = 0;
     int ret = 1, email_dn = 1, req = 0, verbose = 0, gencrl = 0, dorevoke = 0;
-    int i, j, rev_type = REV_NONE, selfsign = 0;
+    int i, j, selfsign = 0;
     long crldays = 0, crlhours = 0, crlsec = 0, days = 0;
     unsigned long chtype = MBSTRING_ASC, nameopt = 0, certopt = 0;
     X509 *x509 = NULL, *x509p = NULL, *x = NULL;
+    REVINFO_TYPE rev_type = REV_NONE;
     X509_REVOKED *r = NULL;
     OPTION_CHOICE o;
 
@@ -403,21 +407,12 @@ opthelp:
         case OPT_CRLEXTS:
             crl_ext = opt_arg();
             break;
-        case OPT_CRL_REASON:
-            rev_arg = opt_arg();
-            rev_type = REV_CRL_REASON;
-            break;
+        case OPT_CRL_REASON:        /* := REV_CRL_REASON */
         case OPT_CRL_HOLD:
-            rev_arg = opt_arg();
-            rev_type = REV_HOLD;
-            break;
         case OPT_CRL_COMPROMISE:
-            rev_arg = opt_arg();
-            rev_type = REV_KEY_COMPROMISE;
-            break;
         case OPT_CRL_CA_COMPROMISE:
             rev_arg = opt_arg();
-            rev_type = REV_CA_COMPROMISE;
+            rev_type = (o - OPT_CRL_REASON) + REV_CRL_REASON;
             break;
         case OPT_ENGINE:
             e = setup_engine(opt_arg(), 0);
@@ -1199,7 +1194,7 @@ end_of_options:
             if (revcert == NULL)
                 goto end;
             if (dorevoke == 2)
-                rev_type = -1;
+                rev_type = REV_VALID;
             j = do_revoke(revcert, db, rev_type, rev_arg);
             if (j <= 0)
                 goto end;
@@ -1640,16 +1635,16 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509,
 
     if (rrow != NULL) {
         BIO_printf(bio_err, "The matching entry has the following details\n");
-        if (rrow[DB_type][0] == 'E')
+        if (rrow[DB_type][0] == DB_TYPE_EXP)
             p = "Expired";
-        else if (rrow[DB_type][0] == 'R')
+        else if (rrow[DB_type][0] == DB_TYPE_REV)
             p = "Revoked";
-        else if (rrow[DB_type][0] == 'V')
+        else if (rrow[DB_type][0] == DB_TYPE_VAL)
             p = "Valid";
         else
             p = "\ninvalid type, Data base error\n";
         BIO_printf(bio_err, "Type          :%s\n", p);;
-        if (rrow[DB_type][0] == 'R') {
+        if (rrow[DB_type][0] == DB_TYPE_REV) {
             p = rrow[DB_exp_date];
             if (p == NULL)
                 p = "undef";
@@ -1821,7 +1816,7 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509,
     if (!do_X509_sign(ret, pkey, dgst, sigopts))
         goto end;
 
-    /* We now just add it to the database */
+    /* We now just add it to the database as DB_TYPE_VAL('V') */
     row[DB_type] = OPENSSL_strdup("V");
     tm = X509_get0_notAfter(ret);
     row[DB_exp_date] = app_malloc(tm->length + 1, "row expdate");
@@ -2020,7 +2015,8 @@ static int check_time_format(const char *str)
     return ASN1_TIME_set_string(NULL, str);
 }
 
-static int do_revoke(X509 *x509, CA_DB *db, int type, char *value)
+static int do_revoke(X509 *x509, CA_DB *db, REVINFO_TYPE rev_type,
+                     const char *value)
 {
     const ASN1_TIME *tm = NULL;
     char *row[DB_NUMBER], **rrow, **irow;
@@ -2053,7 +2049,7 @@ static int do_revoke(X509 *x509, CA_DB *db, int type, char *value)
                    "Adding Entry with serial number %s to DB for %s\n",
                    row[DB_serial], row[DB_name]);
 
-        /* We now just add it to the database */
+        /* We now just add it to the database as DB_TYPE_REV('V') */
         row[DB_type] = OPENSSL_strdup("V");
         tm = X509_get0_notAfter(x509);
         row[DB_exp_date] = app_malloc(tm->length + 1, "row exp_data");
@@ -2076,32 +2072,33 @@ static int do_revoke(X509 *x509, CA_DB *db, int type, char *value)
         }
 
         /* Revoke Certificate */
-        if (type == -1)
+        if (rev_type == REV_VALID)
             ok = 1;
         else
-            ok = do_revoke(x509, db, type, value);
+            /* Retry revocation after DB insertion */
+            ok = do_revoke(x509, db, rev_type, value);
 
         goto end;
 
     } else if (index_name_cmp_noconst(row, rrow)) {
         BIO_printf(bio_err, "ERROR:name does not match %s\n", row[DB_name]);
         goto end;
-    } else if (type == -1) {
+    } else if (rev_type == REV_VALID) {
         BIO_printf(bio_err, "ERROR:Already present, serial number %s\n",
                    row[DB_serial]);
         goto end;
-    } else if (rrow[DB_type][0] == 'R') {
+    } else if (rrow[DB_type][0] == DB_TYPE_REV) {
         BIO_printf(bio_err, "ERROR:Already revoked, serial number %s\n",
                    row[DB_serial]);
         goto end;
     } else {
         BIO_printf(bio_err, "Revoking Certificate %s.\n", rrow[DB_serial]);
-        rev_str = make_revocation_str(type, value);
+        rev_str = make_revocation_str(rev_type, value);
         if (!rev_str) {
             BIO_printf(bio_err, "Error in revocation arguments\n");
             goto end;
         }
-        rrow[DB_type][0] = 'R';
+        rrow[DB_type][0] = DB_TYPE_REV;
         rrow[DB_type][1] = '\0';
         rrow[DB_rev_date] = rev_str;
     }
@@ -2153,19 +2150,19 @@ static int get_certificate_status(const char *serial, CA_DB *db)
         BIO_printf(bio_err, "Serial %s not present in db.\n", row[DB_serial]);
         ok = -1;
         goto end;
-    } else if (rrow[DB_type][0] == 'V') {
+    } else if (rrow[DB_type][0] == DB_TYPE_VAL) {
         BIO_printf(bio_err, "%s=Valid (%c)\n",
                    row[DB_serial], rrow[DB_type][0]);
         goto end;
-    } else if (rrow[DB_type][0] == 'R') {
+    } else if (rrow[DB_type][0] == DB_TYPE_REV) {
         BIO_printf(bio_err, "%s=Revoked (%c)\n",
                    row[DB_serial], rrow[DB_type][0]);
         goto end;
-    } else if (rrow[DB_type][0] == 'E') {
+    } else if (rrow[DB_type][0] == DB_TYPE_EXP) {
         BIO_printf(bio_err, "%s=Expired (%c)\n",
                    row[DB_serial], rrow[DB_type][0]);
         goto end;
-    } else if (rrow[DB_type][0] == 'S') {
+    } else if (rrow[DB_type][0] == DB_TYPE_SUSP) {
         BIO_printf(bio_err, "%s=Suspended (%c)\n",
                    row[DB_serial], rrow[DB_type][0]);
         goto end;
@@ -2207,7 +2204,7 @@ static int do_updatedb(CA_DB *db)
     for (i = 0; i < sk_OPENSSL_PSTRING_num(db->db->data); i++) {
         rrow = sk_OPENSSL_PSTRING_value(db->db->data, i);
 
-        if (rrow[DB_type][0] == 'V') {
+        if (rrow[DB_type][0] == DB_TYPE_VAL) {
             /* ignore entries that are not valid */
             if (strncmp(rrow[DB_exp_date], "49", 2) <= 0)
                 db_y2k = 1;
@@ -2217,14 +2214,14 @@ static int do_updatedb(CA_DB *db)
             if (db_y2k == a_y2k) {
                 /* all on the same y2k side */
                 if (strcmp(rrow[DB_exp_date], a_tm_s) <= 0) {
-                    rrow[DB_type][0] = 'E';
+                    rrow[DB_type][0] = DB_TYPE_EXP;
                     rrow[DB_type][1] = '\0';
                     cnt++;
 
                     BIO_printf(bio_err, "%s=Expired\n", rrow[DB_serial]);
                 }
             } else if (db_y2k < a_y2k) {
-                rrow[DB_type][0] = 'E';
+                rrow[DB_type][0] = DB_TYPE_EXP;
                 rrow[DB_type][1] = '\0';
                 cnt++;
 
@@ -2264,16 +2261,17 @@ static const char *crl_reasons[] = {
  * additional argument
  */
 
-char *make_revocation_str(int rev_type, char *rev_arg)
+static char *make_revocation_str(REVINFO_TYPE rev_type, const char *rev_arg)
 {
     char *str;
-    const char *other = NULL;
-    const char *reason = NULL;
+    const char *reason = NULL, *other = NULL;
     ASN1_OBJECT *otmp;
     ASN1_UTCTIME *revtm = NULL;
     int i;
+
     switch (rev_type) {
     case REV_NONE:
+    case REV_VALID:
         break;
 
     case REV_CRL_REASON:
@@ -2291,7 +2289,6 @@ char *make_revocation_str(int rev_type, char *rev_arg)
 
     case REV_HOLD:
         /* Argument is an OID */
-
         otmp = OBJ_txt2obj(rev_arg, 0);
         ASN1_OBJECT_free(otmp);
 
@@ -2306,7 +2303,6 @@ char *make_revocation_str(int rev_type, char *rev_arg)
 
     case REV_KEY_COMPROMISE:
     case REV_CA_COMPROMISE:
-
         /* Argument is the key compromise time  */
         if (!ASN1_GENERALIZEDTIME_set_string(NULL, rev_arg)) {
             BIO_printf(bio_err,
@@ -2321,7 +2317,6 @@ char *make_revocation_str(int rev_type, char *rev_arg)
             reason = "CAkeyTime";
 
         break;
-
     }
 
     revtm = X509_gmtime_adj(NULL, 0);
@@ -2358,7 +2353,7 @@ char *make_revocation_str(int rev_type, char *rev_arg)
  * 2 OK and some extensions added (i.e. V2 CRL)
  */
 
-int make_revoked(X509_REVOKED *rev, const char *str)
+static int make_revoked(X509_REVOKED *rev, const char *str)
 {
     char *tmp = NULL;
     int reason_code = -1;