Some cleanups to apps/ca.c

Few code format fixup
Fix limit computation; was too strict by 2 bytes.
Simplify computation of buffer limits
Checking is strictly same as sizeof(".pem") == 5
Simplify loop of code for certificate filename creation
Fix MAX_PATH usage

Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1936)
This commit is contained in:
FdaSilvaYY 2017-10-16 15:46:50 -04:00 committed by Rich Salz
parent 8176431d59
commit b5895815ab

View File

@ -42,11 +42,6 @@
#ifndef PATH_MAX #ifndef PATH_MAX
# define PATH_MAX 4096 # define PATH_MAX 4096
#endif #endif
#ifndef NAME_MAX
# define NAME_MAX 255
#endif
#define CERT_MAX (PATH_MAX + NAME_MAX)
#define BASE_SECTION "ca" #define BASE_SECTION "ca"
@ -250,10 +245,11 @@ int ca_main(int argc, char **argv)
const char *serialfile = NULL, *subj = NULL; const char *serialfile = NULL, *subj = NULL;
char *prog, *startdate = NULL, *enddate = NULL; char *prog, *startdate = NULL, *enddate = NULL;
char *dbfile = NULL, *f; char *dbfile = NULL, *f;
char new_cert[CERT_MAX + 1]; char new_cert[PATH_MAX];
char tmp[10 + 1] = "\0"; char tmp[10 + 1] = "\0";
char *const *pp; char *const *pp;
const char *p; const char *p;
size_t outdirlen = 0;
int create_ser = 0, free_key = 0, total = 0, total_done = 0; int create_ser = 0, free_key = 0, total = 0, total_done = 0;
int batch = 0, default_op = 1, doupdatedb = 0, ext_copy = EXT_COPY_NONE; 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 keyformat = FORMAT_PEM, multirdn = 0, notext = 0, output_der = 0;
@ -266,8 +262,6 @@ int ca_main(int argc, char **argv)
X509_REVOKED *r = NULL; X509_REVOKED *r = NULL;
OPTION_CHOICE o; OPTION_CHOICE o;
new_cert[CERT_MAX] = '\0';
prog = opt_init(argc, argv, ca_options); prog = opt_init(argc, argv, ca_options);
while ((o = opt_next()) != OPT_EOF) { while ((o = opt_next()) != OPT_EOF) {
switch (o) { switch (o) {
@ -356,8 +350,7 @@ opthelp:
case OPT_SIGOPT: case OPT_SIGOPT:
if (sigopts == NULL) if (sigopts == NULL)
sigopts = sk_OPENSSL_STRING_new_null(); sigopts = sk_OPENSSL_STRING_new_null();
if (sigopts == NULL if (sigopts == NULL || !sk_OPENSSL_STRING_push(sigopts, opt_arg()))
|| !sk_OPENSSL_STRING_push(sigopts, opt_arg()))
goto end; goto end;
break; break;
case OPT_NOTEXT: case OPT_NOTEXT:
@ -421,7 +414,7 @@ opthelp:
case OPT_CRLEXTS: case OPT_CRLEXTS:
crl_ext = opt_arg(); crl_ext = opt_arg();
break; break;
case OPT_CRL_REASON: /* := REV_CRL_REASON */ case OPT_CRL_REASON: /* := REV_CRL_REASON */
case OPT_CRL_HOLD: case OPT_CRL_HOLD:
case OPT_CRL_COMPROMISE: case OPT_CRL_COMPROMISE:
case OPT_CRL_CA_COMPROMISE: case OPT_CRL_CA_COMPROMISE:
@ -709,8 +702,7 @@ end_of_options:
goto end; goto end;
if (verbose) if (verbose)
BIO_printf(bio_err, BIO_printf(bio_err, "Done. %d entries marked as expired\n", i);
"Done. %d entries marked as expired\n", i);
} }
} }
@ -742,8 +734,7 @@ end_of_options:
goto end; goto end;
} }
if (md == NULL if (md == NULL && (md = lookup_conf(conf, section, ENV_DEFAULT_MD)) == NULL)
&& (md = lookup_conf(conf, section, ENV_DEFAULT_MD)) == NULL)
goto end; goto end;
if (strcmp(md, "default") == 0) { if (strcmp(md, "default") == 0) {
@ -811,8 +802,7 @@ end_of_options:
} }
if (startdate == NULL) { if (startdate == NULL) {
startdate = NCONF_get_string(conf, section, startdate = NCONF_get_string(conf, section, ENV_DEFAULT_STARTDATE);
ENV_DEFAULT_STARTDATE);
if (startdate == NULL) if (startdate == NULL)
ERR_clear_error(); ERR_clear_error();
} }
@ -839,9 +829,8 @@ end_of_options:
if (!NCONF_get_number(conf, section, ENV_DEFAULT_DAYS, &days)) if (!NCONF_get_number(conf, section, ENV_DEFAULT_DAYS, &days))
days = 0; days = 0;
} }
if (enddate == NULL && (days == 0)) { if (enddate == NULL && days == 0) {
BIO_printf(bio_err, BIO_printf(bio_err, "cannot lookup how many days to certify for\n");
"cannot lookup how many days to certify for\n");
goto end; goto end;
} }
@ -972,8 +961,7 @@ end_of_options:
(void)BIO_flush(bio_err); (void)BIO_flush(bio_err);
tmp[0] = '\0'; tmp[0] = '\0';
if (fgets(tmp, sizeof(tmp), stdin) == NULL) { if (fgets(tmp, sizeof(tmp), stdin) == NULL) {
BIO_printf(bio_err, BIO_printf(bio_err, "CERTIFICATION CANCELED: I/O error\n");
"CERTIFICATION CANCELED: I/O error\n");
ret = 0; ret = 0;
goto end; goto end;
} }
@ -995,37 +983,34 @@ end_of_options:
goto end; goto end;
} }
outdirlen = OPENSSL_strlcpy(new_cert, outdir, sizeof(new_cert));
#ifndef OPENSSL_SYS_VMS
outdirlen = OPENSSL_strlcat(new_cert, "/", sizeof(new_cert));
#endif
if (verbose) if (verbose)
BIO_printf(bio_err, "writing new certificates\n"); BIO_printf(bio_err, "writing new certificates\n");
for (i = 0; i < sk_X509_num(cert_sk); i++) { for (i = 0; i < sk_X509_num(cert_sk); i++) {
BIO *Cout = NULL; BIO *Cout = NULL;
X509 *xi = sk_X509_value(cert_sk, i); X509 *xi = sk_X509_value(cert_sk, i);
ASN1_INTEGER *serialNumber = X509_get_serialNumber(xi); ASN1_INTEGER *serialNumber = X509_get_serialNumber(xi);
int k; const unsigned char *psn = ASN1_STRING_get0_data(serialNumber);
char *n; const int snl = ASN1_STRING_length(serialNumber);
const int filen_len = 2 * (snl > 0 ? snl : 1) + sizeof(".pem");
char *n = new_cert + outdirlen;
j = ASN1_STRING_length(serialNumber); if (outdirlen + filen_len > PATH_MAX) {
p = (const char *)ASN1_STRING_get0_data(serialNumber);
if (strlen(outdir) >= (size_t)(j ? CERT_MAX - j * 2 - 6 : CERT_MAX - 8)) {
BIO_printf(bio_err, "certificate file name too long\n"); BIO_printf(bio_err, "certificate file name too long\n");
goto end; goto end;
} }
strcpy(new_cert, outdir); if (snl > 0) {
#ifndef OPENSSL_SYS_VMS static const char HEX_DIGITS[] = "0123456789ABCDEF";
OPENSSL_strlcat(new_cert, "/", sizeof(new_cert));
#endif
n = (char *)&(new_cert[strlen(new_cert)]); for (j = 0; j < snl; j++, psn++) {
if (j > 0) { *n++ = HEX_DIGITS[*psn >> 4];
for (k = 0; k < j; k++) { *n++ = HEX_DIGITS[*psn & 0x0F];
if (n >= &(new_cert[sizeof(new_cert)]))
break;
BIO_snprintf(n,
&new_cert[0] + sizeof(new_cert) - n,
"%02X", (unsigned char)*(p++));
n += 2;
} }
} else { } else {
*(n++) = '0'; *(n++) = '0';
@ -1035,7 +1020,7 @@ end_of_options:
*(n++) = 'p'; *(n++) = 'p';
*(n++) = 'e'; *(n++) = 'e';
*(n++) = 'm'; *(n++) = 'm';
*n = '\0'; *n = '\0'; /* closing new_cert */
if (verbose) if (verbose)
BIO_printf(bio_err, "writing %s\n", new_cert); BIO_printf(bio_err, "writing %s\n", new_cert);
@ -1076,8 +1061,7 @@ end_of_options:
X509V3_set_nconf(&ctx, conf); X509V3_set_nconf(&ctx, conf);
if (!X509V3_EXT_add_nconf(conf, &ctx, crl_ext, NULL)) { if (!X509V3_EXT_add_nconf(conf, &ctx, crl_ext, NULL)) {
BIO_printf(bio_err, BIO_printf(bio_err,
"Error Loading CRL extension section %s\n", "Error Loading CRL extension section %s\n", crl_ext);
crl_ext);
ret = 1; ret = 1;
goto end; goto end;
} }
@ -1393,8 +1377,7 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509,
CONF *lconf, unsigned long certopt, unsigned long nameopt, CONF *lconf, unsigned long certopt, unsigned long nameopt,
int default_op, int ext_copy, int selfsign) int default_op, int ext_copy, int selfsign)
{ {
X509_NAME *name = NULL, *CAname = NULL, *subject = NULL, *dn_subject = X509_NAME *name = NULL, *CAname = NULL, *subject = NULL, *dn_subject = NULL;
NULL;
const ASN1_TIME *tm; const ASN1_TIME *tm;
ASN1_STRING *str, *str2; ASN1_STRING *str, *str2;
ASN1_OBJECT *obj; ASN1_OBJECT *obj;
@ -1424,8 +1407,7 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509,
} }
if (default_op) if (default_op)
BIO_printf(bio_err, BIO_printf(bio_err, "The Subject's Distinguished Name is as follows\n");
"The Subject's Distinguished Name is as follows\n");
name = X509_REQ_get_subject_name(req); name = X509_REQ_get_subject_name(req);
for (i = 0; i < X509_NAME_entry_count(name); i++) { for (i = 0; i < X509_NAME_entry_count(name); i++) {
@ -1543,8 +1525,7 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509,
if ((j < 0) && (last2 == -1)) { if ((j < 0) && (last2 == -1)) {
BIO_printf(bio_err, BIO_printf(bio_err,
"The %s field does not exist in the CA certificate,\n" "The %s field does not exist in the CA certificate,\n"
"the 'policy' is misconfigured\n", "the 'policy' is misconfigured\n", cv->name);
cv->name);
goto end; goto end;
} }
if (j >= 0) { if (j >= 0) {
@ -1888,8 +1869,7 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509,
return (ok); return (ok);
} }
static void write_new_certificate(BIO *bp, X509 *x, int output_der, static void write_new_certificate(BIO *bp, X509 *x, int output_der, int notext)
int notext)
{ {
if (output_der) { if (output_der) {
@ -2005,8 +1985,7 @@ static int certify_spkac(X509 **xret, const char *infile, EVP_PKEY *pkey,
* Now extract the key from the SPKI structure. * Now extract the key from the SPKI structure.
*/ */
BIO_printf(bio_err, BIO_printf(bio_err, "Check that the SPKAC request matches the signature\n");
"Check that the SPKAC request matches the signature\n");
if ((pktmp = NETSCAPE_SPKI_get_pubkey(spki)) == NULL) { if ((pktmp = NETSCAPE_SPKI_get_pubkey(spki)) == NULL) {
BIO_printf(bio_err, "error unpacking SPKAC public key\n"); BIO_printf(bio_err, "error unpacking SPKAC public key\n");
@ -2546,8 +2525,7 @@ int unpack_revinfo(ASN1_TIME **prevtm, int *preason, ASN1_OBJECT **phold,
hold = OBJ_txt2obj(arg_str, 0); hold = OBJ_txt2obj(arg_str, 0);
if (!hold) { if (!hold) {
BIO_printf(bio_err, "invalid object identifier %s\n", BIO_printf(bio_err, "invalid object identifier %s\n", arg_str);
arg_str);
goto end; goto end;
} }
if (phold) if (phold)