From 647e007eeaf8e7a6061bd5bbaaf1b07606d4168e Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Fri, 23 Jul 2021 20:46:40 +0200 Subject: [PATCH 1/6] Implement `wolfSSL_set_client_CA_list` and add 'HIGH' cipher suite --- src/internal.c | 14 +++++++++++--- src/ssl.c | 22 +++++++++++++++++----- tests/api.c | 22 ++++++++++++++++++++-- wolfssl/internal.h | 6 ++++++ wolfssl/openssl/ssl.h | 1 + wolfssl/ssl.h | 6 ++++-- 6 files changed, 59 insertions(+), 12 deletions(-) diff --git a/src/internal.c b/src/internal.c index a0afd3d6b..7d4a53299 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7075,6 +7075,14 @@ void SSL_ResourceFree(WOLFSSL* ssl) wolfSSL_sk_X509_free(ssl->peerCertChain); 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; + } +#endif } /* Free any handshake resources no longer needed */ @@ -18688,7 +18696,7 @@ int SendCertificateRequest(WOLFSSL* ssl) #if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(HAVE_LIGHTY) /* Certificate Authorities */ - names = ssl->ctx->ca_names; + names = SSL_CA_NAMES(ssl); while (names != NULL) { byte seq[MAX_SEQ_SZ]; WOLFSSL_X509_NAME* name = names->data.name; @@ -18759,7 +18767,7 @@ int SendCertificateRequest(WOLFSSL* ssl) c16toa((word16)dnLen, &output[i]); /* auth's */ i += REQ_HEADER_SZ; #if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(HAVE_LIGHTY) - names = ssl->ctx->ca_names; + names = SSL_CA_NAMES(ssl); while (names != NULL) { byte seq[MAX_SEQ_SZ]; WOLFSSL_X509_NAME* name = names->data.name; @@ -21105,7 +21113,7 @@ int SetCipherList(WOLFSSL_CTX* ctx, Suites* suites, const char* list) } if (next[0] == 0 || XSTRNCMP(next, "ALL", 3) == 0 || - XSTRNCMP(next, "DEFAULT", 7) == 0) + XSTRNCMP(next, "DEFAULT", 7) == 0 || XSTRNCMP(next, "HIGH", 4) == 0) return 1; /* wolfSSL default */ do { diff --git a/src/ssl.c b/src/ssl.c index 34b407e90..0bda331d8 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -16045,6 +16045,19 @@ int wolfSSL_set_compression(WOLFSSL* ssl) #endif } + void wolfSSL_set_client_CA_list(WOLFSSL* ssl, + WOLF_STACK_OF(WOLFSSL_X509_NAME)* names) + { + WOLFSSL_ENTER("wolfSSL_set_client_CA_list"); + #if defined(OPENSSL_EXTRA) || defined(WOLFSSL_EXTRA) + if (ssl != NULL) + ssl->ca_names = names; + #else + (void)ssl; + (void)names; + #endif + } + /* returns the CA's set on server side or the CA's sent from server when * on client side */ @@ -16089,8 +16102,7 @@ int wolfSSL_set_compression(WOLFSSL* ssl) return NULL; } else { - /* currently only can be set in the CTX */ - return ssl->ctx->ca_names; + return SSL_CA_NAMES(ssl); } } #endif /* SESSION_CERTS */ @@ -16154,14 +16166,14 @@ int wolfSSL_set_compression(WOLFSSL* ssl) #if defined(OPENSSL_EXTRA) || defined(WOLFSSL_EXTRA) WOLF_STACK_OF(WOLFSSL_X509_NAME)* wolfSSL_CTX_get_client_CA_list( - const WOLFSSL_CTX *s) + const WOLFSSL_CTX *ctx) { WOLFSSL_ENTER("wolfSSL_CTX_get_client_CA_list"); - if (s == NULL) + if (ctx == NULL) return NULL; - return s->ca_names; + return ctx->ca_names; } #endif diff --git a/tests/api.c b/tests/api.c index dd2e8d364..7d97be6e9 100644 --- a/tests/api.c +++ b/tests/api.c @@ -30772,16 +30772,17 @@ static void test_wolfSSL_CTX_set_client_CA_list(void) #if defined(OPENSSL_ALL) && !defined(NO_RSA) && !defined(NO_CERTS) && \ !defined(NO_WOLFSSL_CLIENT) && !defined(NO_BIO) WOLFSSL_CTX* ctx; + WOLFSSL* ssl; X509_NAME* name = NULL; STACK_OF(X509_NAME)* names = NULL; STACK_OF(X509_NAME)* ca_list = NULL; int i, names_len; printf(testingFmt, "wolfSSL_CTX_set_client_CA_list()"); - AssertNotNull(ctx = wolfSSL_CTX_new(wolfSSLv23_client_method())); + AssertNotNull(ctx = wolfSSL_CTX_new(wolfSSLv23_server_method())); names = SSL_load_client_CA_file(cliCertFile); AssertNotNull(names); - SSL_CTX_set_client_CA_list(ctx,names); + SSL_CTX_set_client_CA_list(ctx, names); AssertNotNull(ca_list = SSL_CTX_get_client_CA_list(ctx)); AssertIntGT((names_len = sk_X509_NAME_num(names)), 0); @@ -30790,6 +30791,23 @@ static void test_wolfSSL_CTX_set_client_CA_list(void) AssertIntEQ(sk_X509_NAME_find(names, name), i); } + /* Needed to be able to create ssl object */ + AssertTrue(SSL_CTX_use_certificate_file(ctx, svrCertFile, SSL_FILETYPE_PEM)); + AssertTrue(SSL_CTX_use_PrivateKey_file(ctx, svrKeyFile, SSL_FILETYPE_PEM)); + AssertNotNull(ssl = wolfSSL_new(ctx)); + /* laod again as old names are responsibility of ctx to free */ + names = SSL_load_client_CA_file(cliCertFile); + AssertNotNull(names); + SSL_set_client_CA_list(ssl, names); + AssertNotNull(ca_list = SSL_get_client_CA_list(ssl)); + + AssertIntGT((names_len = sk_X509_NAME_num(names)), 0); + for (i=0; ictx->cm #endif +#define SSL_CA_NAMES(ssl) (ssl->ca_names != NULL ? ssl->ca_names : \ + ssl->ctx->ca_names) + WOLFSSL_LOCAL int SSL_CTX_RefCount(WOLFSSL_CTX* ctx, int incr); WOLFSSL_LOCAL int SetSSL_CTX(WOLFSSL*, WOLFSSL_CTX*, int); WOLFSSL_LOCAL int InitSSL(WOLFSSL*, WOLFSSL_CTX*, int); diff --git a/wolfssl/openssl/ssl.h b/wolfssl/openssl/ssl.h index c6d6bcb30..f8404c910 100644 --- a/wolfssl/openssl/ssl.h +++ b/wolfssl/openssl/ssl.h @@ -835,6 +835,7 @@ wolfSSL_X509_STORE_set_verify_cb((WOLFSSL_X509_STORE *)(s), (WOLFSSL_X509_STORE_ #define SSL_set1_verify_cert_store wolfSSL_set1_verify_cert_store #define SSL_CTX_get_cert_store(x) wolfSSL_CTX_get_cert_store ((WOLFSSL_CTX*) (x)) #define SSL_get_client_CA_list wolfSSL_get_client_CA_list +#define SSL_set_client_CA_list wolfSSL_set_client_CA_list #define SSL_get_ex_data_X509_STORE_CTX_idx wolfSSL_get_ex_data_X509_STORE_CTX_idx #define SSL_get_ex_data wolfSSL_get_ex_data diff --git a/wolfssl/ssl.h b/wolfssl/ssl.h index 8c7a609a4..9146df804 100644 --- a/wolfssl/ssl.h +++ b/wolfssl/ssl.h @@ -1807,11 +1807,13 @@ WOLFSSL_API void wolfSSL_ASN1_TIME_free(WOLFSSL_ASN1_TIME* t); WOLFSSL_API WOLF_STACK_OF(WOLFSSL_X509_NAME)* wolfSSL_load_client_CA_file(const char*); WOLFSSL_API WOLF_STACK_OF(WOLFSSL_X509_NAME)* wolfSSL_CTX_get_client_CA_list( - const WOLFSSL_CTX *s); + const WOLFSSL_CTX *ctx); /* deprecated function name */ #define wolfSSL_SSL_CTX_get_client_CA_list wolfSSL_CTX_get_client_CA_list -WOLFSSL_API void wolfSSL_CTX_set_client_CA_list(WOLFSSL_CTX*, +WOLFSSL_API void wolfSSL_CTX_set_client_CA_list(WOLFSSL_CTX*, + WOLF_STACK_OF(WOLFSSL_X509_NAME)*); +WOLFSSL_API void wolfSSL_set_client_CA_list(WOLFSSL*, WOLF_STACK_OF(WOLFSSL_X509_NAME)*); WOLFSSL_API WOLF_STACK_OF(WOLFSSL_X509_NAME)* wolfSSL_get_client_CA_list( const WOLFSSL* ssl); From d4391bd99789223306c35c9429459167ca77bb63 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Wed, 28 Jul 2021 13:09:09 +0200 Subject: [PATCH 2/6] 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 From 62cab15c648c1d618c658332bc59276e7baa6eb6 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Thu, 29 Jul 2021 15:12:58 +0200 Subject: [PATCH 3/6] Reorganize wolfSSL_sk_X509_NAME_* Make the `wolfSSL_sk_X509_NAME_*` API's available in OPENSSL_EXTRA for use with `client_CA_list` API's. --- src/ssl.c | 572 +++++++++++++++++++++++++++++------------------------- 1 file changed, 310 insertions(+), 262 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 48e82bf93..ebc4f5a3e 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -16190,91 +16190,30 @@ int wolfSSL_set_compression(WOLFSSL* ssl) { WOLFSSL_ENTER("wolfSSL_CTX_get_client_CA_list"); - if (ctx == NULL) + if (ctx == NULL) { + WOLFSSL_MSG("Bad argument passed to wolfSSL_CTX_get_client_CA_list"); return NULL; + } return ctx->ca_names; } -#endif -#if defined(OPENSSL_EXTRA) || defined(HAVE_WEBSERVER) -#ifndef NO_BIO - #if !defined(NO_RSA) && !defined(NO_CERTS) - WOLF_STACK_OF(WOLFSSL_X509_NAME)* wolfSSL_load_client_CA_file(const char* fname) + + /* returns the CA's set on server side or the CA's sent from server when + * on client side */ + WOLF_STACK_OF(WOLFSSL_X509_NAME)* wolfSSL_get_client_CA_list( + const WOLFSSL* ssl) { - /* The webserver build is using this to load a CA into the server - * for client authentication as an option. Have this return NULL in - * that case. If OPENSSL_EXTRA is enabled, go ahead and include - * the function. */ - #ifdef OPENSSL_EXTRA - WOLFSSL_STACK *list = NULL; - WOLFSSL_STACK *node; - WOLFSSL_BIO* bio; - WOLFSSL_X509 *cert = NULL; - WOLFSSL_X509_NAME *subjectName = NULL; - unsigned long err; + WOLFSSL_ENTER("wolfSSL_get_client_CA_list"); - WOLFSSL_ENTER("wolfSSL_load_client_CA_file"); - - bio = wolfSSL_BIO_new_file(fname, "rb"); - if (bio == NULL) + if (ssl == NULL) { + WOLFSSL_MSG("Bad argument passed to wolfSSL_get_client_CA_list"); return NULL; - - /* Read each certificate in the chain out of the file. */ - while (wolfSSL_PEM_read_bio_X509(bio, &cert, NULL, NULL) != NULL) { - subjectName = wolfSSL_X509_get_subject_name(cert); - if (subjectName == NULL) - break; - - node = wolfSSL_sk_new_node(NULL); - if (node == NULL) - break; - node->type = STACK_TYPE_X509_NAME; - - /* Need a persistent copy of the subject name. */ - node->data.name = wolfSSL_X509_NAME_dup(subjectName); - if (node->data.name != NULL) { - /* - * Original cert will be freed so make sure not to try to access - * it in the future. - */ - node->data.name->x509 = NULL; - } - - /* Put node on the front of the list. */ - node->num = (list == NULL) ? 1 : list->num + 1; - node->next = list; - list = node; - - wolfSSL_X509_free(cert); - cert = NULL; } - err = wolfSSL_ERR_peek_last_error(); - - if (ERR_GET_LIB(err) == ERR_LIB_PEM && - ERR_GET_REASON(err) == PEM_R_NO_START_LINE) { - /* - * wolfSSL_PEM_read_bio_X509 pushes an ASN_NO_PEM_HEADER error - * to the error queue on file end. This should not be left - * for the caller to find so we clear the last error. - */ - wc_RemoveErrorNode(-1); - } - - wolfSSL_X509_free(cert); - wolfSSL_BIO_free(bio); - return list; - #else - (void)fname; - return NULL; - #endif + return SSL_CA_NAMES(ssl); } - #endif -#endif /* !NO_BIO */ -#endif /* OPENSSL_EXTRA || HAVE_WEBSERVER */ -#ifdef OPENSSL_EXTRA #if !defined(NO_RSA) && !defined(NO_CERTS) int wolfSSL_CTX_add_client_CA(WOLFSSL_CTX* ctx, WOLFSSL_X509* x509) { @@ -16324,6 +16263,99 @@ int wolfSSL_set_compression(WOLFSSL* ssl) } #endif + #ifndef NO_BIO + #if !defined(NO_RSA) && !defined(NO_CERTS) + WOLF_STACK_OF(WOLFSSL_X509_NAME)* wolfSSL_load_client_CA_file(const char* fname) + { + /* The webserver build is using this to load a CA into the server + * for client authentication as an option. Have this return NULL in + * that case. If OPENSSL_EXTRA is enabled, go ahead and include + * the function. */ + #ifdef OPENSSL_EXTRA + WOLFSSL_STACK *list = NULL; + WOLFSSL_STACK *node; + WOLFSSL_BIO* bio; + WOLFSSL_X509 *cert = NULL; + WOLFSSL_X509_NAME *subjectName = NULL; + unsigned long err; + + WOLFSSL_ENTER("wolfSSL_load_client_CA_file"); + + bio = wolfSSL_BIO_new_file(fname, "rb"); + if (bio == NULL) + return NULL; + + /* Read each certificate in the chain out of the file. */ + while (wolfSSL_PEM_read_bio_X509(bio, &cert, NULL, NULL) != NULL) { + subjectName = wolfSSL_X509_get_subject_name(cert); + if (subjectName == NULL) + break; + + node = wolfSSL_sk_new_node(NULL); + if (node == NULL) + break; + node->type = STACK_TYPE_X509_NAME; + + /* Need a persistent copy of the subject name. */ + node->data.name = wolfSSL_X509_NAME_dup(subjectName); + if (node->data.name != NULL) { + /* + * Original cert will be freed so make sure not to try to access + * it in the future. + */ + node->data.name->x509 = NULL; + } + + /* Put node on the front of the list. */ + node->num = (list == NULL) ? 1 : list->num + 1; + node->next = list; + list = node; + + wolfSSL_X509_free(cert); + cert = NULL; + } + + err = wolfSSL_ERR_peek_last_error(); + + if (ERR_GET_LIB(err) == ERR_LIB_PEM && + ERR_GET_REASON(err) == PEM_R_NO_START_LINE) { + /* + * wolfSSL_PEM_read_bio_X509 pushes an ASN_NO_PEM_HEADER error + * to the error queue on file end. This should not be left + * for the caller to find so we clear the last error. + */ + wc_RemoveErrorNode(-1); + } + + wolfSSL_X509_free(cert); + wolfSSL_BIO_free(bio); + return list; + #else + (void)fname; + return NULL; + #endif + } + #endif + #endif /* !NO_BIO */ +#endif /* OPENSSL_EXTRA || WOLFSSL_EXTRA || HAVE_WEBSERVER */ + + +#if defined(OPENSSL_ALL) || defined(OPENSSL_EXTRA) || \ + defined(WOLFSSL_NGINX) || defined (WOLFSSL_HAPROXY) + /* registers client cert callback, called during handshake if server + requests client auth but user has not loaded client cert/key */ + void wolfSSL_CTX_set_client_cert_cb(WOLFSSL_CTX *ctx, client_cert_cb cb) + { + WOLFSSL_ENTER("wolfSSL_CTX_set_client_cert_cb"); + + if (ctx != NULL) { + ctx->CBClientCert = cb; + } + } +#endif /* OPENSSL_ALL || OPENSSL_EXTRA || WOLFSSL_NGINX || WOLFSSL_HAPROXY */ + +#ifdef OPENSSL_EXTRA + #ifndef NO_WOLFSSL_STUB int wolfSSL_CTX_set_default_verify_paths(WOLFSSL_CTX* ctx) { @@ -45117,9 +45149,210 @@ void* wolfSSL_SESSION_get_ex_data(const WOLFSSL_SESSION* session, int idx) } #endif /* OPENSSL_EXTRA || WOLFSSL_WPAS_SMALL || FORTRESS */ +#if defined(OPENSSL_EXTRA) || defined(HAVE_STUNNEL) || defined(WOLFSSL_NGINX) || \ + defined(HAVE_LIGHTY) || defined(WOLFSSL_HAPROXY) || \ + defined(WOLFSSL_OPENSSH) || defined(HAVE_SBLIM_SFCB) + +WOLF_STACK_OF(WOLFSSL_X509_NAME)* wolfSSL_sk_X509_NAME_new(wolf_sk_compare_cb cb) +{ + WOLFSSL_STACK* sk; + (void)cb; + + WOLFSSL_ENTER("wolfSSL_sk_X509_NAME_new"); + + sk = wolfSSL_sk_new_node(NULL); + if (sk != NULL) { + sk->type = STACK_TYPE_X509_NAME; +#ifdef OPENSSL_ALL + sk->comp = cb; +#endif + } + + return sk; +} + +int wolfSSL_sk_X509_NAME_num(const WOLF_STACK_OF(WOLFSSL_X509_NAME) *sk) +{ + WOLFSSL_ENTER("wolfSSL_sk_X509_NAME_num"); + + if (sk == NULL) + return BAD_FUNC_ARG; + + return (int)sk->num; +} + +/* Getter function for WOLFSSL_X509_NAME pointer + * + * sk is the stack to retrieve pointer from + * i is the index value in stack + * + * returns a pointer to a WOLFSSL_X509_NAME structure on success and NULL on + * fail + */ +WOLFSSL_X509_NAME* wolfSSL_sk_X509_NAME_value(const STACK_OF(WOLFSSL_X509_NAME)* sk, + int i) +{ + WOLFSSL_ENTER("wolfSSL_sk_X509_NAME_value"); + + for (; sk != NULL && i > 0; i--) { + sk = sk->next; + } + + if (i != 0 || sk == NULL) + return NULL; + + return sk->data.name; +} + +WOLFSSL_X509_NAME* wolfSSL_sk_X509_NAME_pop(WOLF_STACK_OF(WOLFSSL_X509_NAME)* sk) +{ + WOLFSSL_STACK* node; + WOLFSSL_X509_NAME* name; + + if (sk == NULL) { + return NULL; + } + + node = sk->next; + name = sk->data.name; + + if (node != NULL) { /* update sk and remove node from stack */ + sk->data.name = node->data.name; + sk->next = node->next; + XFREE(node, NULL, DYNAMIC_TYPE_OPENSSL); + } + else { /* last x509 in stack */ + sk->data.name = NULL; + } + + if (sk->num > 0) { + sk->num -= 1; + } + + return name; +} + +void wolfSSL_sk_X509_NAME_pop_free(WOLF_STACK_OF(WOLFSSL_X509_NAME)* sk, + void (*f) (WOLFSSL_X509_NAME*)) +{ + WOLFSSL_STACK* node; + WOLFSSL_ENTER("wolfSSL_sk_X509_NAME_pop_free"); + + if (sk == NULL) + return; + + node = sk->next; + while (node && sk->num > 1) { + WOLFSSL_STACK* tmp = node; + node = node->next; + if (f) + f(tmp->data.name); + else + wolfSSL_X509_NAME_free(tmp->data.name); + tmp->data.name = NULL; + XFREE(tmp, NULL, DYNAMIC_TYPE_OPENSSL); + sk->num -= 1; + } + + /* free head of stack */ + if (sk->num == 1) { + if (f) + f(sk->data.name); + else + wolfSSL_X509_NAME_free(sk->data.name); + sk->data.name = NULL; + } + + XFREE(sk, sk->heap, DYNAMIC_TYPE_OPENSSL); +} + +/* Free only the sk structure, NOT X509_NAME members */ +void wolfSSL_sk_X509_NAME_free(WOLF_STACK_OF(WOLFSSL_X509_NAME)* sk) +{ + WOLFSSL_STACK* node; + WOLFSSL_ENTER("wolfSSL_sk_X509_NAME_free"); + + if (sk == NULL) + return; + + node = sk->next; + while (sk->num > 1) { + WOLFSSL_STACK* tmp = node; + node = node->next; + XFREE(tmp, NULL, DYNAMIC_TYPE_OPENSSL); + sk->num -= 1; + } + + XFREE(sk, sk->heap, DYNAMIC_TYPE_OPENSSL); +} + +int wolfSSL_sk_X509_NAME_push(WOLF_STACK_OF(WOLFSSL_X509_NAME)* sk, + WOLFSSL_X509_NAME* name) +{ + WOLFSSL_STACK* node; + + WOLFSSL_ENTER("wolfSSL_sk_X509_NAME_push"); + + if (sk == NULL || name == NULL) { + return WOLFSSL_FAILURE; + } + + /* no previous values in stack */ + if (sk->data.name == NULL) { + sk->data.name = name; + sk->num += 1; + return WOLFSSL_SUCCESS; + } + + /* stack already has value(s) create a new node and add more */ + node = (WOLFSSL_STACK*)XMALLOC(sizeof(WOLFSSL_STACK), NULL, + DYNAMIC_TYPE_OPENSSL); + if (node == NULL) { + WOLFSSL_MSG("Memory error"); + return WOLFSSL_FAILURE; + } + XMEMSET(node, 0, sizeof(WOLFSSL_STACK)); + + /* push new obj onto head of stack */ + node->data.name = sk->data.name; + node->next = sk->next; + sk->type = STACK_TYPE_X509_NAME; + sk->next = node; + sk->data.name = name; + sk->num += 1; + + return WOLFSSL_SUCCESS; +} + +/* return index of found, or negative to indicate not found */ +int wolfSSL_sk_X509_NAME_find(const WOLF_STACK_OF(WOLFSSL_X509_NAME) *sk, + WOLFSSL_X509_NAME *name) +{ + int i; + + WOLFSSL_ENTER("wolfSSL_sk_X509_NAME_find"); + + if (sk == NULL) + return BAD_FUNC_ARG; + + for (i = 0; sk; i++, sk = sk->next) { + if (wolfSSL_X509_NAME_cmp(sk->data.name, name) == 0) { + return i; + } + } + return -1; +} + +#endif /* OPENSSL_EXTRA || HAVE_STUNNEL || WOLFSSL_NGINX || + HAVE_LIGHTY || WOLFSSL_HAPROXY || + WOLFSSL_OPENSSH || HAVE_SBLIM_SFCB */ + /* Note: This is a huge section of API's - through * wolfSSL_X509_OBJECT_get0_X509_CRL */ -#if defined(OPENSSL_EXTRA) +#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))) int wolfSSL_SESSION_get_ex_new_index(long idx, void* data, void* cb1, void* cb2, CRYPTO_free_func* cb3) { @@ -45486,21 +45719,6 @@ int wolfSSL_sk_X509_INFO_push(WOLF_STACK_OF(WOLFSSL_X509_INFO)* sk, return WOLFSSL_SUCCESS; } -WOLF_STACK_OF(WOLFSSL_X509_NAME)* wolfSSL_sk_X509_NAME_new(wolf_sk_compare_cb cb) -{ - WOLFSSL_STACK* sk; - - WOLFSSL_ENTER("wolfSSL_sk_X509_NAME_new"); - - sk = wolfSSL_sk_new_node(NULL); - if (sk != NULL) { - sk->type = STACK_TYPE_X509_NAME; - sk->comp = cb; - } - - return sk; -} - /* Creates a duplicate of WOLF_STACK_OF(WOLFSSL_X509_NAME). * Returns a new WOLF_STACK_OF(WOLFSSL_X509_NAME) or NULL on failure */ WOLF_STACK_OF(WOLFSSL_X509_NAME) *wolfSSL_dup_CA_list( @@ -45531,63 +45749,6 @@ WOLF_STACK_OF(WOLFSSL_X509_NAME) *wolfSSL_dup_CA_list( return copy; } -int wolfSSL_sk_X509_NAME_push(WOLF_STACK_OF(WOLFSSL_X509_NAME)* sk, - WOLFSSL_X509_NAME* name) -{ - WOLFSSL_STACK* node; - - WOLFSSL_ENTER("wolfSSL_sk_X509_NAME_push"); - - if (sk == NULL || name == NULL) { - return WOLFSSL_FAILURE; - } - - /* no previous values in stack */ - if (sk->data.name == NULL) { - sk->data.name = name; - sk->num += 1; - return WOLFSSL_SUCCESS; - } - - /* stack already has value(s) create a new node and add more */ - node = (WOLFSSL_STACK*)XMALLOC(sizeof(WOLFSSL_STACK), NULL, - DYNAMIC_TYPE_OPENSSL); - if (node == NULL) { - WOLFSSL_MSG("Memory error"); - return WOLFSSL_FAILURE; - } - XMEMSET(node, 0, sizeof(WOLFSSL_STACK)); - - /* push new obj onto head of stack */ - node->data.name = sk->data.name; - node->next = sk->next; - sk->type = STACK_TYPE_X509_NAME; - sk->next = node; - sk->data.name = name; - sk->num += 1; - - return WOLFSSL_SUCCESS; -} - -/* return index of found, or negative to indicate not found */ -int wolfSSL_sk_X509_NAME_find(const WOLF_STACK_OF(WOLFSSL_X509_NAME) *sk, - WOLFSSL_X509_NAME *name) -{ - int i; - - WOLFSSL_ENTER("wolfSSL_sk_X509_NAME_find"); - - if (sk == NULL) - return BAD_FUNC_ARG; - - for (i = 0; sk; i++, sk = sk->next) { - if (wolfSSL_X509_NAME_cmp(sk->data.name, name) == 0) { - return i; - } - } - return -1; -} - void* wolfSSL_sk_X509_OBJECT_value(WOLF_STACK_OF(WOLFSSL_X509_OBJECT)* sk, int i) { WOLFSSL_ENTER("wolfSSL_sk_X509_OBJECT_value"); @@ -45622,121 +45783,6 @@ int wolfSSL_sk_X509_NAME_set_cmp_func(WOLF_STACK_OF(WOLFSSL_X509_NAME)* sk, } #endif /* OPENSSL_ALL */ -int wolfSSL_sk_X509_NAME_num(const WOLF_STACK_OF(WOLFSSL_X509_NAME) *sk) -{ - WOLFSSL_ENTER("wolfSSL_sk_X509_NAME_num"); - - if (sk == NULL) - return BAD_FUNC_ARG; - - return (int)sk->num; -} - -/* Getter function for WOLFSSL_X509_NAME pointer - * - * sk is the stack to retrieve pointer from - * i is the index value in stack - * - * returns a pointer to a WOLFSSL_X509_NAME structure on success and NULL on - * fail - */ -WOLFSSL_X509_NAME* wolfSSL_sk_X509_NAME_value(const STACK_OF(WOLFSSL_X509_NAME)* sk, - int i) -{ - WOLFSSL_ENTER("wolfSSL_sk_X509_NAME_value"); - - for (; sk != NULL && i > 0; i--) { - sk = sk->next; - } - - if (i != 0 || sk == NULL) - return NULL; - - return sk->data.name; -} - -WOLFSSL_X509_NAME* wolfSSL_sk_X509_NAME_pop(WOLF_STACK_OF(WOLFSSL_X509_NAME)* sk) -{ - WOLFSSL_STACK* node; - WOLFSSL_X509_NAME* name; - - if (sk == NULL) { - return NULL; - } - - node = sk->next; - name = sk->data.name; - - if (node != NULL) { /* update sk and remove node from stack */ - sk->data.name = node->data.name; - sk->next = node->next; - XFREE(node, NULL, DYNAMIC_TYPE_OPENSSL); - } - else { /* last x509 in stack */ - sk->data.name = NULL; - } - - if (sk->num > 0) { - sk->num -= 1; - } - - return name; -} - -void wolfSSL_sk_X509_NAME_pop_free(WOLF_STACK_OF(WOLFSSL_X509_NAME)* sk, - void (*f) (WOLFSSL_X509_NAME*)) -{ - WOLFSSL_STACK* node; - WOLFSSL_ENTER("wolfSSL_sk_X509_NAME_pop_free"); - - if (sk == NULL) - return; - - node = sk->next; - while (node && sk->num > 1) { - WOLFSSL_STACK* tmp = node; - node = node->next; - if (f) - f(tmp->data.name); - else - wolfSSL_X509_NAME_free(tmp->data.name); - tmp->data.name = NULL; - XFREE(tmp, NULL, DYNAMIC_TYPE_OPENSSL); - sk->num -= 1; - } - - /* free head of stack */ - if (sk->num == 1) { - if (f) - f(sk->data.name); - else - wolfSSL_X509_NAME_free(sk->data.name); - sk->data.name = NULL; - } - - XFREE(sk, sk->heap, DYNAMIC_TYPE_OPENSSL); -} - -/* Free only the sk structure, NOT X509_NAME members */ -void wolfSSL_sk_X509_NAME_free(WOLF_STACK_OF(WOLFSSL_X509_NAME)* sk) -{ - WOLFSSL_STACK* node; - WOLFSSL_ENTER("wolfSSL_sk_X509_NAME_free"); - - if (sk == NULL) - return; - - node = sk->next; - while (sk->num > 1) { - WOLFSSL_STACK* tmp = node; - node = node->next; - XFREE(tmp, NULL, DYNAMIC_TYPE_OPENSSL); - sk->num -= 1; - } - - XFREE(sk, sk->heap, DYNAMIC_TYPE_OPENSSL); -} - #ifndef NO_BIO #if defined(WOLFSSL_APACHE_HTTPD) || defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) @@ -46121,7 +46167,9 @@ WOLFSSL_X509_CRL *wolfSSL_X509_OBJECT_get0_X509_CRL(WOLFSSL_X509_OBJECT *obj) return NULL; } -#endif /* OPENSSL_EXTRA */ +#endif /* OPENSSL_ALL || (OPENSSL_EXTRA && (HAVE_STUNNEL || WOLFSSL_NGINX || + * HAVE_LIGHTY || WOLFSSL_HAPROXY || WOLFSSL_OPENSSH || + * HAVE_SBLIM_SFCB)) */ #if defined(OPENSSL_EXTRA) From 72f1d0adac93f6fe01a95c6b0ad76842dbbd01ef Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Thu, 29 Jul 2021 15:14:04 +0200 Subject: [PATCH 4/6] Refactor `client_CA` API to use `wolfSSL_sk_X509_NAME_*` API --- src/ssl.c | 123 +++++++++++++++++++++++++----------------------------- 1 file changed, 56 insertions(+), 67 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index ebc4f5a3e..007ab92dc 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -16031,36 +16031,26 @@ int wolfSSL_set_compression(WOLFSSL* ssl) #endif /* !NO_BIO */ #endif /* OPENSSL_EXTRA */ -#if defined(OPENSSL_EXTRA) || defined(WOLFSSL_EXTRA) || defined(HAVE_WEBSERVER) +#if defined(OPENSSL_EXTRA) || defined(WOLFSSL_EXTRA) void wolfSSL_CTX_set_client_CA_list(WOLFSSL_CTX* ctx, WOLF_STACK_OF(WOLFSSL_X509_NAME)* names) { WOLFSSL_ENTER("wolfSSL_CTX_set_client_CA_list"); - #if defined(OPENSSL_EXTRA) || defined(WOLFSSL_EXTRA) if (ctx != NULL) { wolfSSL_sk_X509_NAME_pop_free(ctx->ca_names, NULL); ctx->ca_names = names; } - #else - (void)ctx; - (void)names; - #endif } void wolfSSL_set_client_CA_list(WOLFSSL* ssl, WOLF_STACK_OF(WOLFSSL_X509_NAME)* names) { WOLFSSL_ENTER("wolfSSL_set_client_CA_list"); - #if defined(OPENSSL_EXTRA) || defined(WOLFSSL_EXTRA) 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; - #endif } @@ -16198,7 +16188,6 @@ int wolfSSL_set_compression(WOLFSSL* ssl) return ctx->ca_names; } - /* returns the CA's set on server side or the CA's sent from server when * on client side */ WOLF_STACK_OF(WOLFSSL_X509_NAME)* wolfSSL_get_client_CA_list( @@ -16217,48 +16206,35 @@ int wolfSSL_set_compression(WOLFSSL* ssl) #if !defined(NO_RSA) && !defined(NO_CERTS) int wolfSSL_CTX_add_client_CA(WOLFSSL_CTX* ctx, WOLFSSL_X509* x509) { - WOLFSSL_STACK *node = NULL; - WOLFSSL_X509_NAME *subjectName = NULL; + WOLFSSL_X509_NAME *nameCopy = NULL; WOLFSSL_ENTER("wolfSSL_CTX_add_client_CA"); if (ctx == NULL || x509 == NULL){ WOLFSSL_MSG("Bad argument"); - return SSL_FAILURE; + return WOLFSSL_FAILURE; } - subjectName = wolfSSL_X509_get_subject_name(x509); - if (subjectName == NULL){ - WOLFSSL_MSG("invalid x509 data"); - return SSL_FAILURE; + if (ctx->ca_names == NULL) { + ctx->ca_names = wolfSSL_sk_X509_NAME_new(NULL); + if (ctx->ca_names == NULL) { + WOLFSSL_MSG("wolfSSL_sk_X509_NAME_new error"); + return WOLFSSL_FAILURE; + } } - /* Alloc stack struct */ - node = (WOLF_STACK_OF(WOLFSSL_X509_NAME)*)XMALLOC( - sizeof(WOLF_STACK_OF(WOLFSSL_X509_NAME)), - NULL, DYNAMIC_TYPE_OPENSSL); - if (node == NULL){ - WOLFSSL_MSG("memory allocation error"); - return SSL_FAILURE; + nameCopy = wolfSSL_X509_NAME_dup(wolfSSL_X509_get_subject_name(x509)); + if (nameCopy == NULL) { + WOLFSSL_MSG("wolfSSL_X509_NAME_dup error"); + return WOLFSSL_FAILURE; } - XMEMSET(node, 0, sizeof(WOLF_STACK_OF(WOLFSSL_X509_NAME))); - /* Alloc and copy WOLFSSL_X509_NAME */ - node->data.name = (WOLFSSL_X509_NAME*)XMALLOC( - sizeof(WOLFSSL_X509_NAME), - NULL, DYNAMIC_TYPE_OPENSSL); - if (node->data.name == NULL) { - XFREE(node, NULL, DYNAMIC_TYPE_OPENSSL); - WOLFSSL_MSG("memory allocation error"); - return SSL_FAILURE; + if (wolfSSL_sk_X509_NAME_push(ctx->ca_names, nameCopy) != WOLFSSL_SUCCESS) { + WOLFSSL_MSG("wolfSSL_sk_X509_NAME_push error"); + wolfSSL_X509_NAME_free(nameCopy); + return WOLFSSL_FAILURE; } - XMEMCPY(node->data.name, subjectName, sizeof(WOLFSSL_X509_NAME)); - XMEMSET(subjectName, 0, sizeof(WOLFSSL_X509_NAME)); - /* push new node onto head of stack */ - node->num = (ctx->ca_names == NULL) ? 1 : ctx->ca_names->num + 1; - node->next = ctx->ca_names; - ctx->ca_names = node; return WOLFSSL_SUCCESS; } #endif @@ -16273,43 +16249,49 @@ int wolfSSL_set_compression(WOLFSSL* ssl) * the function. */ #ifdef OPENSSL_EXTRA WOLFSSL_STACK *list = NULL; - WOLFSSL_STACK *node; - WOLFSSL_BIO* bio; + WOLFSSL_BIO* bio = NULL; WOLFSSL_X509 *cert = NULL; - WOLFSSL_X509_NAME *subjectName = NULL; - unsigned long err; + WOLFSSL_X509_NAME *nameCopy = NULL; + unsigned long err = WOLFSSL_FAILURE; WOLFSSL_ENTER("wolfSSL_load_client_CA_file"); bio = wolfSSL_BIO_new_file(fname, "rb"); - if (bio == NULL) - return NULL; + if (bio == NULL) { + WOLFSSL_MSG("wolfSSL_BIO_new_file error"); + goto cleanup; + } + + list = wolfSSL_sk_X509_NAME_new(NULL); + if (list == NULL) { + WOLFSSL_MSG("wolfSSL_sk_X509_NAME_new error"); + goto cleanup; + } /* Read each certificate in the chain out of the file. */ while (wolfSSL_PEM_read_bio_X509(bio, &cert, NULL, NULL) != NULL) { - subjectName = wolfSSL_X509_get_subject_name(cert); - if (subjectName == NULL) - break; - - node = wolfSSL_sk_new_node(NULL); - if (node == NULL) - break; - node->type = STACK_TYPE_X509_NAME; - /* Need a persistent copy of the subject name. */ - node->data.name = wolfSSL_X509_NAME_dup(subjectName); - if (node->data.name != NULL) { - /* - * Original cert will be freed so make sure not to try to access - * it in the future. - */ - node->data.name->x509 = NULL; + nameCopy = wolfSSL_X509_NAME_dup( + wolfSSL_X509_get_subject_name(cert)); + if (nameCopy == NULL) { + WOLFSSL_MSG("wolfSSL_X509_NAME_dup error"); + goto cleanup; } + /* + * Original cert will be freed so make sure not to try to access + * it in the future. + */ + nameCopy->x509 = NULL; - /* Put node on the front of the list. */ - node->num = (list == NULL) ? 1 : list->num + 1; - node->next = list; - list = node; + if (wolfSSL_sk_X509_NAME_push(list, nameCopy) != + WOLFSSL_SUCCESS) { + WOLFSSL_MSG("wolfSSL_sk_X509_NAME_push error"); + /* Do free in loop because nameCopy is now responsibility + * of list to free and adding jumps to cleanup after this + * might result in a double free. */ + wolfSSL_X509_NAME_free(nameCopy); + goto cleanup; + } wolfSSL_X509_free(cert); cert = NULL; @@ -16327,8 +16309,15 @@ int wolfSSL_set_compression(WOLFSSL* ssl) wc_RemoveErrorNode(-1); } + err = WOLFSSL_SUCCESS; +cleanup: wolfSSL_X509_free(cert); wolfSSL_BIO_free(bio); + if (err != WOLFSSL_SUCCESS) { + /* We failed so return NULL */ + wolfSSL_sk_X509_NAME_pop_free(list, NULL); + list = NULL; + } return list; #else (void)fname; From 6a5f40d69870b93b6705c1d40eeb732efa106c41 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Mon, 9 Aug 2021 17:19:54 +0200 Subject: [PATCH 5/6] Code review fixes. --- src/internal.c | 11 ++++++++--- src/ssl.c | 16 ++++++++-------- wolfcrypt/src/asn.c | 3 ++- wolfssl/ssl.h | 6 ++++-- 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/internal.c b/src/internal.c index 831a213bd..f29dc5103 100644 --- a/src/internal.c +++ b/src/internal.c @@ -2195,8 +2195,11 @@ void SSL_CtxResourceFree(WOLFSSL_CTX* ctx) ctx->x509_store.objs = NULL; } #endif - #if defined(OPENSSL_EXTRA) || defined(WOLFSSL_EXTRA) || defined(HAVE_LIGHTY) + #if defined(OPENSSL_EXTRA) || defined(HAVE_WEBSERVER) || \ + defined(WOLFSSL_WPAS_SMALL) wolfSSL_X509_STORE_free(ctx->x509_store_pt); + #endif + #if defined(OPENSSL_EXTRA) || defined(WOLFSSL_EXTRA) || defined(HAVE_LIGHTY) wolfSSL_sk_X509_NAME_pop_free(ctx->ca_names, NULL); ctx->ca_names = NULL; #endif @@ -10297,7 +10300,7 @@ static void CopyDecodedName(WOLFSSL_X509_NAME* name, DecodedCert* dCert, int nam 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) +#if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(HAVE_LIGHTY) name->rawLen = min(dCert->subjectRawLen, ASN_NAME_MAX); XMEMCPY(name->raw, dCert->subjectRaw, name->rawLen); #endif @@ -10306,7 +10309,8 @@ static void CopyDecodedName(WOLFSSL_X509_NAME* name, DecodedCert* dCert, int nam 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) +#if (defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(HAVE_LIGHTY)) \ + && (defined(HAVE_PKCS7) || defined(WOLFSSL_CERT_EXT)) name->rawLen = min(dCert->issuerRawLen, ASN_NAME_MAX); if (name->rawLen) { XMEMCPY(name->raw, dCert->issuerRaw, name->rawLen); @@ -23055,6 +23059,7 @@ exit_dpk: if (wolfSSL_sk_X509_NAME_push(ssl->ca_names, name) == WOLFSSL_FAILURE) { FreeDecodedCert(&cert); + wolfSSL_X509_NAME_free(name); return MEMORY_ERROR; } diff --git a/src/ssl.c b/src/ssl.c index 007ab92dc..09f32b26d 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -16203,7 +16203,7 @@ int wolfSSL_set_compression(WOLFSSL* ssl) return SSL_CA_NAMES(ssl); } - #if !defined(NO_RSA) && !defined(NO_CERTS) + #if !defined(NO_CERTS) int wolfSSL_CTX_add_client_CA(WOLFSSL_CTX* ctx, WOLFSSL_X509* x509) { WOLFSSL_X509_NAME *nameCopy = NULL; @@ -16326,7 +16326,7 @@ cleanup: } #endif #endif /* !NO_BIO */ -#endif /* OPENSSL_EXTRA || WOLFSSL_EXTRA || HAVE_WEBSERVER */ +#endif /* OPENSSL_EXTRA || WOLFSSL_EXTRA */ #if defined(OPENSSL_ALL) || defined(OPENSSL_EXTRA) || \ @@ -19156,10 +19156,7 @@ WOLF_STACK_OF(WOLFSSL_X509)* wolfSSL_set_peer_cert_chain(WOLFSSL* ssl) if ((ssl == NULL) || (ssl->session.chain.count == 0)) return NULL; - if (ssl->peerCertChain == NULL) - sk = wolfSSL_sk_X509_new(); - else /* Try to re-use old chain if available */ - sk = ssl->peerCertChain; + sk = wolfSSL_sk_X509_new(); i = ssl->session.chain.count-1; for (; i >= 0; i--) { x509 = wolfSSL_X509_new(); @@ -19199,6 +19196,8 @@ WOLF_STACK_OF(WOLFSSL_X509)* wolfSSL_set_peer_cert_chain(WOLFSSL* ssl) wolfSSL_sk_X509_shift(sk); } #endif + if (ssl->peerCertChain != NULL) + wolfSSL_sk_X509_free(ssl->peerCertChain); /* This is Free'd when ssl is Free'd */ ssl->peerCertChain = sk; return sk; @@ -58242,7 +58241,8 @@ int wolfSSL_X509_STORE_CTX_get1_issuer(WOLFSSL_X509 **issuer, * START OF X509_STORE APIs ******************************************************************************/ -#if defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL) +#if defined(OPENSSL_EXTRA) || defined(HAVE_WEBSERVER) || \ + defined(WOLFSSL_WPAS_SMALL) WOLFSSL_X509_STORE* wolfSSL_X509_STORE_new(void) { WOLFSSL_X509_STORE* store = NULL; @@ -58435,7 +58435,7 @@ int wolfSSL_X509_STORE_set_ex_data_with_cleanup( #endif /* HAVE_EX_DATA_CLEANUP_HOOKS */ -#endif /* OPENSSL_EXTRA || WOLFSSL_WPAS_SMALL */ +#endif /* OPENSSL_EXTRA || HAVE_WEBSERVER || WOLFSSL_WPAS_SMALL */ #ifdef OPENSSL_EXTRA diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 9331b0c35..447575c26 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -6543,7 +6543,8 @@ 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) +#if (defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(HAVE_LIGHTY)) && \ + (defined(HAVE_PKCS7) || defined(WOLFSSL_CERT_EXT)) dName->rawLen = min(cert->issuerRawLen, ASN_NAME_MAX); XMEMCPY(dName->raw, cert->issuerRaw, dName->rawLen); #endif diff --git a/wolfssl/ssl.h b/wolfssl/ssl.h index 9146df804..412fbba27 100644 --- a/wolfssl/ssl.h +++ b/wolfssl/ssl.h @@ -561,7 +561,8 @@ struct WOLFSSL_X509_STORE { int cache; /* stunnel dereference */ WOLFSSL_CERT_MANAGER* cm; WOLFSSL_X509_LOOKUP lookup; -#if defined(OPENSSL_EXTRA) || defined(HAVE_WEBSERVER) || defined(WOLFSSL_WPAS_SMALL) +#if defined(OPENSSL_EXTRA) || defined(HAVE_WEBSERVER) || \ + defined(WOLFSSL_WPAS_SMALL) int isDynamic; WOLFSSL_X509_VERIFY_PARAM* param; /* certificate validation parameter */ #endif @@ -574,7 +575,8 @@ struct WOLFSSL_X509_STORE { #ifdef HAVE_EX_DATA WOLFSSL_CRYPTO_EX_DATA ex_data; #endif -#ifdef HAVE_CRL +#if (defined(OPENSSL_EXTRA) || defined(HAVE_WEBSERVER) || \ + defined(WOLFSSL_WPAS_SMALL)) && defined(HAVE_CRL) WOLFSSL_X509_CRL *crl; /* points to cm->crl */ #endif #ifndef SINGLE_THREADED From 0f6e5640931eb9099306d74b963cf0b8229ff16a Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Sat, 14 Aug 2021 00:35:55 +0200 Subject: [PATCH 6/6] Rebase fixes --- src/ssl.c | 80 ------------------------------------------- wolfssl/openssl/ssl.h | 4 --- 2 files changed, 84 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 09f32b26d..a12c3ad96 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -16053,71 +16053,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 */ - WOLF_STACK_OF(WOLFSSL_X509_NAME)* wolfSSL_get_client_CA_list( - const WOLFSSL* ssl) - { - WOLFSSL_ENTER("wolfSSL_get_client_CA_list"); - - if (ssl == NULL) { - WOLFSSL_MSG("Bad argument passed to wolfSSL_get_client_CA_list"); - 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; - WOLFSSL_X509* x509; - - ret = wolfSSL_sk_X509_NAME_new(NULL); - do { - x509 = wolfSSL_sk_X509_pop(sk); - if (x509 != NULL) { - if (wolfSSL_X509_get_isCA(x509)) { - 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); - /* Save return value to free later */ - ((WOLFSSL*)ssl)->ca_names = ret; - return ret; - } - return NULL; - } - else -#endif /* SESSION_CERTS */ - { - return SSL_CA_NAMES(ssl); - } - } - - #ifdef OPENSSL_EXTRA /* registers client cert callback, called during handshake if server requests client auth but user has not loaded client cert/key */ @@ -16328,21 +16263,6 @@ cleanup: #endif /* !NO_BIO */ #endif /* OPENSSL_EXTRA || WOLFSSL_EXTRA */ - -#if defined(OPENSSL_ALL) || defined(OPENSSL_EXTRA) || \ - defined(WOLFSSL_NGINX) || defined (WOLFSSL_HAPROXY) - /* registers client cert callback, called during handshake if server - requests client auth but user has not loaded client cert/key */ - void wolfSSL_CTX_set_client_cert_cb(WOLFSSL_CTX *ctx, client_cert_cb cb) - { - WOLFSSL_ENTER("wolfSSL_CTX_set_client_cert_cb"); - - if (ctx != NULL) { - ctx->CBClientCert = cb; - } - } -#endif /* OPENSSL_ALL || OPENSSL_EXTRA || WOLFSSL_NGINX || WOLFSSL_HAPROXY */ - #ifdef OPENSSL_EXTRA #ifndef NO_WOLFSSL_STUB diff --git a/wolfssl/openssl/ssl.h b/wolfssl/openssl/ssl.h index f8404c910..3b0bb6fa9 100644 --- a/wolfssl/openssl/ssl.h +++ b/wolfssl/openssl/ssl.h @@ -839,10 +839,6 @@ wolfSSL_X509_STORE_set_verify_cb((WOLFSSL_X509_STORE *)(s), (WOLFSSL_X509_STORE_ #define SSL_get_ex_data_X509_STORE_CTX_idx wolfSSL_get_ex_data_X509_STORE_CTX_idx #define SSL_get_ex_data wolfSSL_get_ex_data -#ifndef WOLFSSL_NO_STUB -#define SSL_set_client_CA_list(...) -#endif /* WOLFSSL_NO_STUB */ - #define SSL_CTX_set_default_passwd_cb_userdata wolfSSL_CTX_set_default_passwd_cb_userdata #define SSL_CTX_set_default_passwd_cb wolfSSL_CTX_set_default_passwd_cb