From a8d8e06b0ac06c421fd11cc1772126dcb98f79ae Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Wed, 2 Sep 2015 22:01:18 +0100 Subject: [PATCH] Avoid direct X509 structure access Reviewed-by: Tim Hudson --- apps/ca.c | 23 ++++++----------------- apps/x509.c | 9 +++++++-- crypto/ocsp/ocsp_vfy.c | 4 ++-- crypto/pkcs7/pk7_doit.c | 6 +++--- crypto/ts/ts_rsp_sign.c | 6 +++--- crypto/ts/ts_rsp_verify.c | 14 +++++++------- crypto/x509/x509type.c | 2 +- crypto/x509v3/pcy_tree.c | 14 +++++++++----- ssl/ssl_cert.c | 3 +-- ssl/ssl_lib.c | 36 ++++++++++++------------------------ test/ssltest.c | 2 +- 11 files changed, 52 insertions(+), 67 deletions(-) diff --git a/apps/ca.c b/apps/ca.c index b93cff5619..5cd8002067 100644 --- a/apps/ca.c +++ b/apps/ca.c @@ -1052,13 +1052,14 @@ end_of_options: if (verbose) BIO_printf(bio_err, "writing new certificates\n"); for (i = 0; i < sk_X509_num(cert_sk); i++) { + ASN1_INTEGER *serialNumber = X509_get_serialNumber(x); int k; char *n; x = sk_X509_value(cert_sk, i); - j = x->cert_info->serialNumber->length; - p = (const char *)x->cert_info->serialNumber->data; + j = ASN1_STRING_length(serialNumber); + p = (const char *)ASN1_STRING_data(serialNumber); if (strlen(outdir) >= (size_t)(j ? BSIZE - j * 2 - 6 : BSIZE - 8)) { BIO_printf(bio_err, "certificate file name too long\n"); @@ -1450,7 +1451,6 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509, ASN1_STRING *str, *str2; ASN1_OBJECT *obj; X509 *ret = NULL; - X509_CINF *ci; X509_NAME_ENTRY *ne; X509_NAME_ENTRY *tne, *push; EVP_PKEY *pktmp; @@ -1546,7 +1546,7 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509, if (selfsign) CAname = X509_NAME_dup(name); else - CAname = X509_NAME_dup(x509->cert_info->subject); + CAname = X509_NAME_dup(X509_get_subject_name(x509)); if (CAname == NULL) goto end; str = str2 = NULL; @@ -1755,7 +1755,6 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509, if ((ret = X509_new()) == NULL) goto end; - ci = ret->cert_info; #ifdef X509_V3 /* Make it an X509 v3 certificate. */ @@ -1763,7 +1762,7 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509, goto end; #endif - if (BN_to_ASN1_INTEGER(serial, ci->serialNumber) == NULL) + if (BN_to_ASN1_INTEGER(serial, X509_get_serialNumber(ret)) == NULL) goto end; if (selfsign) { if (!X509_set_issuer_name(ret, subject)) @@ -1799,17 +1798,7 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509, /* Lets add the extensions, if there are any */ if (ext_sect) { X509V3_CTX ctx; - if (ci->version == NULL) - if ((ci->version = ASN1_INTEGER_new()) == NULL) - goto end; - ASN1_INTEGER_set(ci->version, 2); /* version 3 certificate */ - - /* - * Free the current entries if any, there should not be any I believe - */ - sk_X509_EXTENSION_pop_free(ci->extensions, X509_EXTENSION_free); - - ci->extensions = NULL; + X509_set_version(ret, 2); /* Initialize the context structure */ if (selfsign) diff --git a/apps/x509.c b/apps/x509.c index 6b41a7501a..acce9e9ddd 100644 --- a/apps/x509.c +++ b/apps/x509.c @@ -894,8 +894,13 @@ int x509_main(int argc, char **argv) goto end; } - if (badsig) - x->signature->data[x->signature->length - 1] ^= 0x1; + if (badsig) { + ASN1_BIT_STRING *signature; + unsigned char *s; + X509_get0_signature(&signature, NULL, x); + s = ASN1_STRING_data(signature); + s[ASN1_STRING_length(signature) - 1] ^= 0x1; + } if (outformat == FORMAT_ASN1) i = i2d_X509_bio(out, x); diff --git a/crypto/ocsp/ocsp_vfy.c b/crypto/ocsp/ocsp_vfy.c index d2693c7c54..9dd3f3a20a 100644 --- a/crypto/ocsp/ocsp_vfy.c +++ b/crypto/ocsp/ocsp_vfy.c @@ -355,8 +355,8 @@ static int ocsp_match_issuerid(X509 *cert, OCSP_CERTID *cid, static int ocsp_check_delegated(X509 *x, int flags) { - X509_check_purpose(x, -1, 0); - if ((x->ex_flags & EXFLAG_XKUSAGE) && (x->ex_xkusage & XKU_OCSP_SIGN)) + if ((X509_get_extension_flags(x) & EXFLAG_XKUSAGE) + && (X509_get_extended_key_usage(x) & XKU_OCSP_SIGN)) return 1; OCSPerr(OCSP_F_OCSP_CHECK_DELEGATED, OCSP_R_MISSING_OCSPSIGNING_USAGE); return 0; diff --git a/crypto/pkcs7/pk7_doit.c b/crypto/pkcs7/pk7_doit.c index cc2f3be554..1ac68937eb 100644 --- a/crypto/pkcs7/pk7_doit.c +++ b/crypto/pkcs7/pk7_doit.c @@ -393,11 +393,11 @@ static int pkcs7_cmp_ri(PKCS7_RECIP_INFO *ri, X509 *pcert) { int ret; ret = X509_NAME_cmp(ri->issuer_and_serial->issuer, - pcert->cert_info->issuer); + X509_get_issuer_name(pcert)); if (ret) return ret; - return ASN1_INTEGER_cmp(pcert->cert_info->serialNumber, - ri->issuer_and_serial->serial); + return ASN1_INTEGER_cmp(X509_get_serialNumber(pcert), + ri->issuer_and_serial->serial); } /* int */ diff --git a/crypto/ts/ts_rsp_sign.c b/crypto/ts/ts_rsp_sign.c index 3343dce275..f7fb762d5b 100644 --- a/crypto/ts/ts_rsp_sign.c +++ b/crypto/ts/ts_rsp_sign.c @@ -657,7 +657,7 @@ static TS_TST_INFO *ts_RESP_create_tst_info(TS_RESP_CTX *ctx, goto end; tsa_name->type = GEN_DIRNAME; tsa_name->d.dirn = - X509_NAME_dup(ctx->signer_cert->cert_info->subject); + X509_NAME_dup(X509_get_subject_name(ctx->signer_cert)); if (!tsa_name->d.dirn) goto end; if (!TS_TST_INFO_set_tsa(tst_info, tsa_name)) @@ -869,7 +869,7 @@ static ESS_CERT_ID *ess_CERT_ID_new_init(X509 *cert, int issuer_needed) if ((name = GENERAL_NAME_new()) == NULL) goto err; name->type = GEN_DIRNAME; - if ((name->d.dirn = X509_NAME_dup(cert->cert_info->issuer)) == NULL) + if ((name->d.dirn = X509_NAME_dup(X509_get_issuer_name(cert))) == NULL) goto err; if (!sk_GENERAL_NAME_push(cid->issuer_serial->issuer, name)) goto err; @@ -877,7 +877,7 @@ static ESS_CERT_ID *ess_CERT_ID_new_init(X509 *cert, int issuer_needed) /* Setting the serial number. */ ASN1_INTEGER_free(cid->issuer_serial->serial); if (!(cid->issuer_serial->serial = - ASN1_INTEGER_dup(cert->cert_info->serialNumber))) + ASN1_INTEGER_dup(X509_get_serialNumber(cert)))) goto err; } diff --git a/crypto/ts/ts_rsp_verify.c b/crypto/ts/ts_rsp_verify.c index c01d6a6565..93a775efec 100644 --- a/crypto/ts/ts_rsp_verify.c +++ b/crypto/ts/ts_rsp_verify.c @@ -72,7 +72,7 @@ static int ts_check_signing_certs(PKCS7_SIGNER_INFO *si, STACK_OF(X509) *chain); static ESS_SIGNING_CERT *ess_get_signing_cert(PKCS7_SIGNER_INFO *si); static int ts_find_cert(STACK_OF(ESS_CERT_ID) *cert_ids, X509 *cert); -static int ts_issuer_serial_cmp(ESS_ISSUER_SERIAL *is, X509_CINF *cinfo); +static int ts_issuer_serial_cmp(ESS_ISSUER_SERIAL *is, X509 *cert); static int int_ts_RESP_verify_token(TS_VERIFY_CTX *ctx, PKCS7 *token, TS_TST_INFO *tst_info); static int ts_check_status_info(TS_RESP *response); @@ -328,7 +328,7 @@ static int ts_find_cert(STACK_OF(ESS_CERT_ID) *cert_ids, X509 *cert) sizeof(cert->sha1_hash))) { /* Check the issuer/serial as well if specified. */ ESS_ISSUER_SERIAL *is = cid->issuer_serial; - if (!is || !ts_issuer_serial_cmp(is, cert->cert_info)) + if (!is || !ts_issuer_serial_cmp(is, cert)) return i; } } @@ -336,21 +336,21 @@ static int ts_find_cert(STACK_OF(ESS_CERT_ID) *cert_ids, X509 *cert) return -1; } -static int ts_issuer_serial_cmp(ESS_ISSUER_SERIAL *is, X509_CINF *cinfo) +static int ts_issuer_serial_cmp(ESS_ISSUER_SERIAL *is, X509 *cert) { GENERAL_NAME *issuer; - if (!is || !cinfo || sk_GENERAL_NAME_num(is->issuer) != 1) + if (!is || !cert || sk_GENERAL_NAME_num(is->issuer) != 1) return -1; /* Check the issuer first. It must be a directory name. */ issuer = sk_GENERAL_NAME_value(is->issuer, 0); if (issuer->type != GEN_DIRNAME - || X509_NAME_cmp(issuer->d.dirn, cinfo->issuer)) + || X509_NAME_cmp(issuer->d.dirn, X509_get_issuer_name(cert))) return -1; /* Check the serial number, too. */ - if (ASN1_INTEGER_cmp(is->serial, cinfo->serialNumber)) + if (ASN1_INTEGER_cmp(is->serial, X509_get_serialNumber(cert))) return -1; return 0; @@ -687,7 +687,7 @@ static int ts_check_signer_name(GENERAL_NAME *tsa_name, X509 *signer) /* Check the subject name first. */ if (tsa_name->type == GEN_DIRNAME - && X509_name_cmp(tsa_name->d.dirn, signer->cert_info->subject) == 0) + && X509_name_cmp(tsa_name->d.dirn, X509_get_subject_name(signer)) == 0) return 1; /* Check all the alternative names. */ diff --git a/crypto/x509/x509type.c b/crypto/x509/x509type.c index 232ba9bc79..8332d9e9d4 100644 --- a/crypto/x509/x509type.c +++ b/crypto/x509/x509type.c @@ -100,7 +100,7 @@ int X509_certificate_type(X509 *x, EVP_PKEY *pkey) break; } - i = OBJ_obj2nid(x->sig_alg->algorithm); + i = X509_get_signature_nid(x); if (i && OBJ_find_sigid_algs(i, NULL, &i)) { switch (i) { diff --git a/crypto/x509v3/pcy_tree.c b/crypto/x509v3/pcy_tree.c index c6be015193..bbc9ada143 100644 --- a/crypto/x509v3/pcy_tree.c +++ b/crypto/x509v3/pcy_tree.c @@ -184,7 +184,9 @@ static int tree_init(X509_POLICY_TREE **ptree, STACK_OF(X509) *certs, * explicit_policy value at this point. */ for (i = n - 2; i >= 0; i--) { + uint32_t ex_flags; x = sk_X509_value(certs, i); + ex_flags = X509_get_extension_flags(x); X509_check_purpose(x, -1, -1); cache = policy_cache_set(x); /* If cache NULL something bad happened: return immediately */ @@ -193,7 +195,7 @@ static int tree_init(X509_POLICY_TREE **ptree, STACK_OF(X509) *certs, /* * If inconsistent extensions keep a note of it but continue */ - if (x->ex_flags & EXFLAG_INVALID_POLICY) + if (ex_flags & EXFLAG_INVALID_POLICY) ret = -1; /* * Otherwise if we have no data (hence no CertificatePolicies) and @@ -202,7 +204,7 @@ static int tree_init(X509_POLICY_TREE **ptree, STACK_OF(X509) *certs, else if ((ret == 1) && !cache->data) ret = 2; if (explicit_policy > 0) { - if (!(x->ex_flags & EXFLAG_SI)) + if (!(ex_flags & EXFLAG_SI)) explicit_policy--; if ((cache->explicit_skip != -1) && (cache->explicit_skip < explicit_policy)) @@ -235,8 +237,10 @@ static int tree_init(X509_POLICY_TREE **ptree, STACK_OF(X509) *certs, goto bad_tree; for (i = n - 2; i >= 0; i--) { + uint32_t ex_flags; level++; x = sk_X509_value(certs, i); + ex_flags = X509_get_extension_flags(x); cache = policy_cache_set(x); X509_up_ref(x); level->cert = x; @@ -250,10 +254,10 @@ static int tree_init(X509_POLICY_TREE **ptree, STACK_OF(X509) *certs, * Any matching allowed if certificate is self issued and not the * last in the chain. */ - if (!(x->ex_flags & EXFLAG_SI) || (i == 0)) + if (!(ex_flags & EXFLAG_SI) || (i == 0)) level->flags |= X509_V_FLAG_INHIBIT_ANY; } else { - if (!(x->ex_flags & EXFLAG_SI)) + if (!(ex_flags & EXFLAG_SI)) any_skip--; if ((cache->any_skip >= 0) && (cache->any_skip < any_skip)) @@ -263,7 +267,7 @@ static int tree_init(X509_POLICY_TREE **ptree, STACK_OF(X509) *certs, if (map_skip == 0) level->flags |= X509_V_FLAG_INHIBIT_MAP; else { - if (!(x->ex_flags & EXFLAG_SI)) + if (!(ex_flags & EXFLAG_SI)) map_skip--; if ((cache->map_skip >= 0) && (cache->map_skip < map_skip)) diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c index c3e2c2ed0a..555b1d7d82 100644 --- a/ssl/ssl_cert.c +++ b/ssl/ssl_cert.c @@ -1028,8 +1028,7 @@ int ssl_build_cert_chain(SSL *s, SSL_CTX *ctx, int flags) if (sk_X509_num(chain) > 0) { /* See if last cert is self signed */ x = sk_X509_value(chain, sk_X509_num(chain) - 1); - X509_check_purpose(x, -1, 0); - if (x->ex_flags & EXFLAG_SS) { + if (X509_get_extension_flags(x) & EXFLAG_SS) { x = sk_X509_pop(chain); X509_free(x); } diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index fe07d2c4c0..c84ea15ed1 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -1900,7 +1900,7 @@ void ssl_set_masks(SSL *s, const SSL_CIPHER *cipher) int have_ecdh_tmp, ecdh_ok; X509 *x = NULL; EVP_PKEY *ecc_pkey = NULL; - int signature_nid = 0, pk_nid = 0, md_nid = 0; + int pk_nid = 0, md_nid = 0; #endif if (c == NULL) return; @@ -2004,23 +2004,18 @@ void ssl_set_masks(SSL *s, const SSL_CIPHER *cipher) */ #ifndef OPENSSL_NO_EC if (have_ecc_cert) { + uint32_t ex_kusage; cpk = &c->pkeys[SSL_PKEY_ECC]; x = cpk->x509; - /* This call populates extension flags (ex_flags) */ - X509_check_purpose(x, -1, 0); - ecdh_ok = (x->ex_flags & EXFLAG_KUSAGE) ? - (x->ex_kusage & X509v3_KU_KEY_AGREEMENT) : 1; - ecdsa_ok = (x->ex_flags & EXFLAG_KUSAGE) ? - (x->ex_kusage & X509v3_KU_DIGITAL_SIGNATURE) : 1; + ex_kusage = X509_get_key_usage(x); + ecdh_ok = ex_kusage & X509v3_KU_KEY_AGREEMENT; + ecdsa_ok = ex_kusage & X509v3_KU_DIGITAL_SIGNATURE; if (!(pvalid[SSL_PKEY_ECC] & CERT_PKEY_SIGN)) ecdsa_ok = 0; ecc_pkey = X509_get_pubkey(x); ecc_pkey_size = (ecc_pkey != NULL) ? EVP_PKEY_bits(ecc_pkey) : 0; EVP_PKEY_free(ecc_pkey); - if ((x->sig_alg) && (x->sig_alg->algorithm)) { - signature_nid = OBJ_obj2nid(x->sig_alg->algorithm); - OBJ_find_sigid_algs(signature_nid, &md_nid, &pk_nid); - } + OBJ_find_sigid_algs(X509_get_signature_nid(x), &md_nid, &pk_nid); if (ecdh_ok) { if (pk_nid == NID_rsaEncryption || pk_nid == NID_rsa) { @@ -2074,10 +2069,6 @@ void ssl_set_masks(SSL *s, const SSL_CIPHER *cipher) s->s3->tmp.export_mask_a = emask_a; } -/* This handy macro borrowed from crypto/x509v3/v3_purp.c */ -#define ku_reject(x, usage) \ - (((x)->ex_flags & EXFLAG_KUSAGE) && !((x)->ex_kusage & (usage))) - #ifndef OPENSSL_NO_EC int ssl_check_srvr_ecc_cert_and_alg(X509 *x, SSL *s) @@ -2085,8 +2076,9 @@ int ssl_check_srvr_ecc_cert_and_alg(X509 *x, SSL *s) unsigned long alg_k, alg_a; EVP_PKEY *pkey = NULL; int keysize = 0; - int signature_nid = 0, md_nid = 0, pk_nid = 0; + int md_nid = 0, pk_nid = 0; const SSL_CIPHER *cs = s->s3->tmp.new_cipher; + uint32_t ex_kusage = X509_get_key_usage(x); alg_k = cs->algorithm_mkey; alg_a = cs->algorithm_auth; @@ -2102,15 +2094,11 @@ int ssl_check_srvr_ecc_cert_and_alg(X509 *x, SSL *s) return 0; } - /* This call populates the ex_flags field correctly */ - X509_check_purpose(x, -1, 0); - if ((x->sig_alg) && (x->sig_alg->algorithm)) { - signature_nid = OBJ_obj2nid(x->sig_alg->algorithm); - OBJ_find_sigid_algs(signature_nid, &md_nid, &pk_nid); - } + OBJ_find_sigid_algs(X509_get_signature_nid(x), &md_nid, &pk_nid); + if (alg_k & SSL_kECDHe || alg_k & SSL_kECDHr) { /* key usage, if present, must allow key agreement */ - if (ku_reject(x, X509v3_KU_KEY_AGREEMENT)) { + if (!(ex_kusage & X509v3_KU_KEY_AGREEMENT)) { SSLerr(SSL_F_SSL_CHECK_SRVR_ECC_CERT_AND_ALG, SSL_R_ECC_CERT_NOT_FOR_KEY_AGREEMENT); return 0; @@ -2135,7 +2123,7 @@ int ssl_check_srvr_ecc_cert_and_alg(X509 *x, SSL *s) } if (alg_a & SSL_aECDSA) { /* key usage, if present, must allow signing */ - if (ku_reject(x, X509v3_KU_DIGITAL_SIGNATURE)) { + if (!(ex_kusage & X509v3_KU_DIGITAL_SIGNATURE)) { SSLerr(SSL_F_SSL_CHECK_SRVR_ECC_CERT_AND_ALG, SSL_R_ECC_CERT_NOT_FOR_SIGNING); return 0; diff --git a/test/ssltest.c b/test/ssltest.c index adf1368020..6f9d16c51b 100644 --- a/test/ssltest.c +++ b/test/ssltest.c @@ -2422,7 +2422,7 @@ static int verify_callback(int ok, X509_STORE_CTX *ctx) if (ok == 1) { X509 *xs = ctx->current_cert; - if (xs->ex_flags & EXFLAG_PROXY) { + if (X509_get_extension_flags(xs) & EXFLAG_PROXY) { unsigned int *letters = X509_STORE_CTX_get_ex_data(ctx, get_proxy_auth_ex_data_idx ()); -- 2.25.1