From d4391bd99789223306c35c9429459167ca77bb63 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Wed, 28 Jul 2021 13:09:09 +0200 Subject: [PATCH] Parse distinguished names in `DoCertificateRequest` The CA names sent by the server are now being parsed in `DoCertificateRequest` and are saved on a stack in `ssl->ca_names`. --- src/internal.c | 107 +++++++++++++++++++++++++++----------- src/ssl.c | 58 ++++++++++++++------- tests/api.c | 78 ++++++++++++++++++++++++++- wolfcrypt/src/asn.c | 14 ++++- wolfssl/openssl/objects.h | 1 - 5 files changed, 204 insertions(+), 54 deletions(-) diff --git a/src/internal.c b/src/internal.c index 7d4a53299..831a213bd 100644 --- a/src/internal.c +++ b/src/internal.c @@ -2195,14 +2195,10 @@ void SSL_CtxResourceFree(WOLFSSL_CTX* ctx) ctx->x509_store.objs = NULL; } #endif - #ifdef OPENSSL_EXTRA + #if defined(OPENSSL_EXTRA) || defined(WOLFSSL_EXTRA) || defined(HAVE_LIGHTY) wolfSSL_X509_STORE_free(ctx->x509_store_pt); - while (ctx->ca_names != NULL) { - WOLFSSL_STACK *next = ctx->ca_names->next; - wolfSSL_X509_NAME_free(ctx->ca_names->data.name); - XFREE(ctx->ca_names, NULL, DYNAMIC_TYPE_OPENSSL); - ctx->ca_names = next; - } + wolfSSL_sk_X509_NAME_pop_free(ctx->ca_names, NULL); + ctx->ca_names = NULL; #endif #if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY) if (ctx->x509Chain) { @@ -7076,12 +7072,8 @@ void SSL_ResourceFree(WOLFSSL* ssl) wolfSSL_sk_X509_free(ssl->ourCertChain); #endif #if defined(OPENSSL_EXTRA) || defined(WOLFSSL_EXTRA) || defined(HAVE_LIGHTY) - while (ssl->ca_names != NULL) { - WOLFSSL_STACK *next = ssl->ca_names->next; - wolfSSL_X509_NAME_free(ssl->ca_names->data.name); - XFREE(ssl->ca_names, NULL, DYNAMIC_TYPE_OPENSSL); - ssl->ca_names = next; - } + wolfSSL_sk_X509_NAME_pop_free(ssl->ca_names, NULL); + ssl->ca_names = NULL; #endif } @@ -10299,6 +10291,30 @@ static void AddSessionCertToChain(WOLFSSL_X509_CHAIN* chain, #if defined(KEEP_PEER_CERT) || defined(SESSION_CERTS) || \ defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) +static void CopyDecodedName(WOLFSSL_X509_NAME* name, DecodedCert* dCert, int nameType) +{ + if (nameType == SUBJECT) { + XSTRNCPY(name->name, dCert->subject, ASN_NAME_MAX); + name->name[ASN_NAME_MAX - 1] = '\0'; + name->sz = (int)XSTRLEN(name->name) + 1; +#if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) + name->rawLen = min(dCert->subjectRawLen, ASN_NAME_MAX); + XMEMCPY(name->raw, dCert->subjectRaw, name->rawLen); +#endif + } + else { + XSTRNCPY(name->name, dCert->issuer, ASN_NAME_MAX); + name->name[ASN_NAME_MAX - 1] = '\0'; + name->sz = (int)XSTRLEN(name->name) + 1; +#if (defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX)) && defined(WOLFSSL_CERT_EXT) + name->rawLen = min(dCert->issuerRawLen, ASN_NAME_MAX); + if (name->rawLen) { + XMEMCPY(name->raw, dCert->issuerRaw, name->rawLen); + } +#endif + } +} + /* Copy parts X509 needs from Decoded cert, 0 on success */ /* The same DecodedCert cannot be copied to WOLFSSL_X509 twice otherwise the * altNames pointers could be free'd by second x509 still active by first */ @@ -10317,9 +10333,7 @@ int CopyDecodedToX509(WOLFSSL_X509* x509, DecodedCert* dCert) x509->version = dCert->version + 1; - XSTRNCPY(x509->issuer.name, dCert->issuer, ASN_NAME_MAX); - x509->issuer.name[ASN_NAME_MAX - 1] = '\0'; - x509->issuer.sz = (int)XSTRLEN(x509->issuer.name) + 1; + CopyDecodedName(&x509->issuer, dCert, ISSUER); #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) if (dCert->issuerName != NULL) { wolfSSL_X509_set_issuer_name(x509, @@ -10327,10 +10341,7 @@ int CopyDecodedToX509(WOLFSSL_X509* x509, DecodedCert* dCert) x509->issuer.x509 = x509; } #endif /* OPENSSL_EXTRA || OPENSSL_EXTRA_X509_SMALL */ - - XSTRNCPY(x509->subject.name, dCert->subject, ASN_NAME_MAX); - x509->subject.name[ASN_NAME_MAX - 1] = '\0'; - x509->subject.sz = (int)XSTRLEN(x509->subject.name) + 1; + CopyDecodedName(&x509->subject, dCert, SUBJECT); #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) if (dCert->subjectName != NULL) { wolfSSL_X509_set_subject_name(x509, @@ -10338,16 +10349,6 @@ int CopyDecodedToX509(WOLFSSL_X509* x509, DecodedCert* dCert) x509->subject.x509 = x509; } #endif /* OPENSSL_EXTRA || OPENSSL_EXTRA_X509_SMALL */ -#if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) - x509->subject.rawLen = min(dCert->subjectRawLen, sizeof(x509->subject.raw)); - XMEMCPY(x509->subject.raw, dCert->subjectRaw, x509->subject.rawLen); -#ifdef WOLFSSL_CERT_EXT - x509->issuer.rawLen = min(dCert->issuerRawLen, sizeof(x509->issuer.raw)); - if (x509->issuer.rawLen) { - XMEMCPY(x509->issuer.raw, dCert->issuerRaw, x509->issuer.rawLen); - } -#endif -#endif XMEMCPY(x509->serial, dCert->serial, EXTERNAL_SERIAL_SIZE); x509->serialSz = dCert->serialSz; @@ -22936,8 +22937,11 @@ exit_dpk: { word16 len; word32 begin = *inOutIdx; - #ifdef OPENSSL_EXTRA + #if defined(OPENSSL_EXTRA) || defined(OPENSSL_ALL) || \ + defined(WOLFSSL_NGINX) || defined(HAVE_LIGHTY) int ret; + #endif + #ifdef OPENSSL_EXTRA WOLFSSL_X509* x509 = NULL; WOLFSSL_EVP_PKEY* pkey = NULL; #endif @@ -22999,12 +23003,22 @@ exit_dpk: if ((*inOutIdx - begin) + OPAQUE16_LEN > size) return BUFFER_ERROR; + /* DN seq length */ ato16(input + *inOutIdx, &len); *inOutIdx += OPAQUE16_LEN; if ((*inOutIdx - begin) + len > size) return BUFFER_ERROR; + #if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(HAVE_LIGHTY) + if (ssl->ca_names != ssl->ctx->ca_names) + wolfSSL_sk_X509_NAME_pop_free(ssl->ca_names, NULL); + ssl->ca_names = wolfSSL_sk_X509_NAME_new(NULL); + if (ssl->ca_names == NULL) { + return MEMORY_ERROR; + } + #endif + while (len) { word16 dnSz; @@ -23017,6 +23031,37 @@ exit_dpk: if ((*inOutIdx - begin) + dnSz > size) return BUFFER_ERROR; + #if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(HAVE_LIGHTY) + { + /* Use a DecodedCert struct to get access to GetName to + * parse DN name */ + DecodedCert cert; + WOLFSSL_X509_NAME* name; + + InitDecodedCert(&cert, input + *inOutIdx, dnSz, ssl->heap); + + if ((ret = GetName(&cert, SUBJECT, dnSz)) != 0) { + FreeDecodedCert(&cert); + return ret; + } + + if ((name = wolfSSL_X509_NAME_new()) == NULL) { + FreeDecodedCert(&cert); + return MEMORY_ERROR; + } + + CopyDecodedName(name, &cert, SUBJECT); + + if (wolfSSL_sk_X509_NAME_push(ssl->ca_names, name) + == WOLFSSL_FAILURE) { + FreeDecodedCert(&cert); + return MEMORY_ERROR; + } + + FreeDecodedCert(&cert); + } + #endif + *inOutIdx += dnSz; len -= OPAQUE16_LEN + dnSz; } diff --git a/src/ssl.c b/src/ssl.c index 0bda331d8..48e82bf93 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -16037,8 +16037,10 @@ int wolfSSL_set_compression(WOLFSSL* ssl) { WOLFSSL_ENTER("wolfSSL_CTX_set_client_CA_list"); #if defined(OPENSSL_EXTRA) || defined(WOLFSSL_EXTRA) - if (ctx != NULL) + if (ctx != NULL) { + wolfSSL_sk_X509_NAME_pop_free(ctx->ca_names, NULL); ctx->ca_names = names; + } #else (void)ctx; (void)names; @@ -16050,8 +16052,11 @@ int wolfSSL_set_compression(WOLFSSL* ssl) { WOLFSSL_ENTER("wolfSSL_set_client_CA_list"); #if defined(OPENSSL_EXTRA) || defined(WOLFSSL_EXTRA) - if (ssl != NULL) + if (ssl != NULL) { + if (ssl->ca_names != ssl->ctx->ca_names) + wolfSSL_sk_X509_NAME_pop_free(ssl->ca_names, NULL); ssl->ca_names = names; + } #else (void)ssl; (void)names; @@ -16061,7 +16066,6 @@ int wolfSSL_set_compression(WOLFSSL* ssl) /* returns the CA's set on server side or the CA's sent from server when * on client side */ -#if defined(SESSION_CERTS) && defined(OPENSSL_ALL) WOLF_STACK_OF(WOLFSSL_X509_NAME)* wolfSSL_get_client_CA_list( const WOLFSSL* ssl) { @@ -16072,10 +16076,14 @@ int wolfSSL_set_compression(WOLFSSL* ssl) return NULL; } +#ifdef SESSION_CERTS /* return list of CAs sent from the server */ if (ssl->options.side == WOLFSSL_CLIENT_END) { WOLF_STACK_OF(WOLFSSL_X509)* sk; + if (ssl->ca_names != NULL) + return ssl->ca_names; + sk = wolfSSL_get_peer_cert_chain(ssl); if (sk != NULL) { WOLF_STACK_OF(WOLFSSL_X509_NAME)* ret; @@ -16086,26 +16094,38 @@ int wolfSSL_set_compression(WOLFSSL* ssl) x509 = wolfSSL_sk_X509_pop(sk); if (x509 != NULL) { if (wolfSSL_X509_get_isCA(x509)) { - if (wolfSSL_sk_X509_NAME_push(ret, - wolfSSL_X509_get_subject_name(x509)) != 0) { - WOLFSSL_MSG("Error pushing X509 name to stack"); + WOLFSSL_X509_NAME* name = wolfSSL_X509_NAME_dup( + wolfSSL_X509_get_subject_name(x509)); + + if (name != NULL) { /* continue on to try other certificates and * do not fail out here */ + if (wolfSSL_sk_X509_NAME_push(ret, + name) != WOLFSSL_SUCCESS) { + WOLFSSL_MSG("Error pushing X509 " + "name to stack"); + wolfSSL_X509_NAME_free(name); + } + } + else { + WOLFSSL_MSG("Error copying X509 name"); } } wolfSSL_X509_free(x509); } } while (x509 != NULL); - wolfSSL_sk_X509_free(sk); + /* Save return value to free later */ + ((WOLFSSL*)ssl)->ca_names = ret; return ret; } return NULL; } - else { + else +#endif /* SESSION_CERTS */ + { return SSL_CA_NAMES(ssl); } } -#endif /* SESSION_CERTS */ #ifdef OPENSSL_EXTRA @@ -19048,7 +19068,9 @@ WOLF_STACK_OF(WOLFSSL_X509)* wolfSSL_get_peer_cert_chain(const WOLFSSL* ssl) if (ssl == NULL) return NULL; - if (ssl->peerCertChain == NULL) + /* Try to populate if NULL or empty */ + if (ssl->peerCertChain == NULL || + wolfSSL_sk_X509_num(ssl->peerCertChain) == 0) wolfSSL_set_peer_cert_chain((WOLFSSL*) ssl); return ssl->peerCertChain; } @@ -19113,7 +19135,10 @@ WOLF_STACK_OF(WOLFSSL_X509)* wolfSSL_set_peer_cert_chain(WOLFSSL* ssl) if ((ssl == NULL) || (ssl->session.chain.count == 0)) return NULL; - sk = wolfSSL_sk_X509_new(); + if (ssl->peerCertChain == NULL) + sk = wolfSSL_sk_X509_new(); + else /* Try to re-use old chain if available */ + sk = ssl->peerCertChain; i = ssl->session.chain.count-1; for (; i >= 0; i--) { x509 = wolfSSL_X509_new(); @@ -45094,10 +45119,7 @@ void* wolfSSL_SESSION_get_ex_data(const WOLFSSL_SESSION* session, int idx) /* Note: This is a huge section of API's - through * wolfSSL_X509_OBJECT_get0_X509_CRL */ -#if defined(OPENSSL_ALL) || (defined(OPENSSL_EXTRA) && \ - (defined(HAVE_STUNNEL) || defined(WOLFSSL_NGINX) || \ - defined(HAVE_LIGHTY) || defined(WOLFSSL_HAPROXY) || \ - defined(WOLFSSL_OPENSSH) || defined(HAVE_SBLIM_SFCB))) +#if defined(OPENSSL_EXTRA) int wolfSSL_SESSION_get_ex_new_index(long idx, void* data, void* cb1, void* cb2, CRYPTO_free_func* cb3) { @@ -45491,7 +45513,7 @@ WOLF_STACK_OF(WOLFSSL_X509_NAME) *wolfSSL_dup_CA_list( WOLFSSL_ENTER("wolfSSL_dup_CA_list"); - copy = wolfSSL_sk_X509_NAME_new(NULL); + copy = wolfSSL_sk_X509_NAME_new(sk->comp); if (copy == NULL) { WOLFSSL_MSG("Memory error"); return NULL; @@ -46099,9 +46121,7 @@ WOLFSSL_X509_CRL *wolfSSL_X509_OBJECT_get0_X509_CRL(WOLFSSL_X509_OBJECT *obj) return NULL; } -#endif /* OPENSSL_ALL || (OPENSSL_EXTRA && (HAVE_STUNNEL || WOLFSSL_NGINX || - * HAVE_LIGHTY || WOLFSSL_HAPROXY || WOLFSSL_OPENSSH || - * HAVE_SBLIM_SFCB)) */ +#endif /* OPENSSL_EXTRA */ #if defined(OPENSSL_EXTRA) diff --git a/tests/api.c b/tests/api.c index 7d97be6e9..522cd4e65 100644 --- a/tests/api.c +++ b/tests/api.c @@ -30780,10 +30780,17 @@ static void test_wolfSSL_CTX_set_client_CA_list(void) printf(testingFmt, "wolfSSL_CTX_set_client_CA_list()"); AssertNotNull(ctx = wolfSSL_CTX_new(wolfSSLv23_server_method())); + /* Send two X501 names in cert request */ names = SSL_load_client_CA_file(cliCertFile); AssertNotNull(names); + ca_list = SSL_load_client_CA_file(caCertFile); + AssertNotNull(ca_list); + AssertIntEQ(sk_X509_NAME_push(names, sk_X509_NAME_value(ca_list, 0)), 1); SSL_CTX_set_client_CA_list(ctx, names); + /* This should only free the stack structure */ + sk_X509_NAME_free(ca_list); AssertNotNull(ca_list = SSL_CTX_get_client_CA_list(ctx)); + AssertIntEQ(sk_X509_NAME_num(ca_list), sk_X509_NAME_num(names)); AssertIntGT((names_len = sk_X509_NAME_num(names)), 0); for (i=0; iport, 0, 0, NULL); + AssertNotNull(ctx_client = wolfSSL_CTX_new(wolfTLSv1_2_client_method())); + AssertIntEQ(WOLFSSL_SUCCESS, + wolfSSL_CTX_load_verify_locations(ctx_client, caCertFile, 0)); + AssertIntEQ(WOLFSSL_SUCCESS, + wolfSSL_CTX_use_certificate_file(ctx_client, cliCertFile, SSL_FILETYPE_PEM)); + AssertIntEQ(WOLFSSL_SUCCESS, + wolfSSL_CTX_use_PrivateKey_file(ctx_client, cliKeyFile, SSL_FILETYPE_PEM)); + + AssertNotNull(ssl_client = wolfSSL_new(ctx_client)); + AssertIntEQ(wolfSSL_set_fd(ssl_client, sockfd), WOLFSSL_SUCCESS); + AssertIntEQ(wolfSSL_connect(ssl_client), WOLFSSL_SUCCESS); + + AssertNotNull(ca_list = SSL_get_client_CA_list(ssl_client)); + /* We are expecting two cert names to be sent */ + AssertIntEQ(sk_X509_NAME_num(ca_list), 2); + + AssertNotNull(names = SSL_CTX_get_client_CA_list(ctx)); + for (i=0; i #include #endif @@ -6542,9 +6543,17 @@ int GetName(DecodedCert* cert, int nameType, int maxIdx) #if (defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL)) && \ !defined(WOLFCRYPT_ONLY) if (nameType == ISSUER) { +#if (defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX)) && defined(WOLFSSL_CERT_EXT) + dName->rawLen = min(cert->issuerRawLen, ASN_NAME_MAX); + XMEMCPY(dName->raw, cert->issuerRaw, dName->rawLen); +#endif cert->issuerName = dName; } else { +#if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) + dName->rawLen = min(cert->subjectRawLen, ASN_NAME_MAX); + XMEMCPY(dName->raw, cert->subjectRaw, dName->rawLen); +#endif cert->subjectName = dName; } #endif @@ -9694,7 +9703,9 @@ int ParseCert(DecodedCert* cert, int type, int verify, void* cm) return ret; } -/* from SSL proper, for locking can't do find here anymore */ +#if !defined(OPENSSL_EXTRA) && !defined(OPENSSL_EXTRA_X509_SMALL) +/* from SSL proper, for locking can't do find here anymore. + * brought in from internal.h if built with compat layer */ #ifdef __cplusplus extern "C" { #endif @@ -9705,6 +9716,7 @@ int ParseCert(DecodedCert* cert, int type, int verify, void* cm) #ifdef __cplusplus } #endif +#endif #if defined(WOLFCRYPT_ONLY) || defined(NO_CERTS) diff --git a/wolfssl/openssl/objects.h b/wolfssl/openssl/objects.h index ba79c48b4..5d6b49ca9 100644 --- a/wolfssl/openssl/objects.h +++ b/wolfssl/openssl/objects.h @@ -24,7 +24,6 @@ #define WOLFSSL_OBJECTS_H_ #include -//#include #ifndef OPENSSL_EXTRA_SSL_GUARD #define OPENSSL_EXTRA_SSL_GUARD #include