Add support for enabling and disabling CRLs. #6329
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4509,7 +4509,11 @@ int wolfSSL_get_error(WOLFSSL* ssl, int ret) | |
| return WOLFSSL_ERROR_WANT_WRITE; /* convert to OpenSSL type */ | ||
| else if (ssl->error == ZERO_RETURN || ssl->options.shutdownDone) | ||
| return WOLFSSL_ERROR_ZERO_RETURN; /* convert to OpenSSL type */ | ||
| #if defined(WOLFSSL_HAPROXY) | ||
| return GetX509Error(ssl->error); | ||
julek-wolfssl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| #else | ||
| return (ssl->error); | ||
| #endif | ||
| } | ||
|
|
||
|
|
||
|
|
@@ -8152,7 +8156,8 @@ int wolfSSL_CertManagerLoadCRLBuffer(WOLFSSL_CERT_MANAGER* cm, | |
| return BAD_FUNC_ARG; | ||
|
|
||
| if (cm->crl == NULL) { | ||
| if (wolfSSL_CertManagerEnableCRL(cm, WOLFSSL_CRL_CHECK) != | ||
| WOLFSSL_SUCCESS) { | ||
| WOLFSSL_MSG("Enable CRL failed"); | ||
| return WOLFSSL_FATAL_ERROR; | ||
| } | ||
|
|
@@ -8204,11 +8209,21 @@ int wolfSSL_CertManagerEnableCRL(WOLFSSL_CERT_MANAGER* cm, int options) | |
| { | ||
| int ret = WOLFSSL_SUCCESS; | ||
|
|
||
| WOLFSSL_ENTER("wolfSSL_CertManagerEnableCRL"); | ||
| if (cm == NULL) | ||
| return BAD_FUNC_ARG; | ||
| #if defined(OPENSSL_COMPATIBLE_DEFAULTS) | ||
| if (options == 0) { | ||
|
|
||
| /* Turn off doing Leaf CRL check */ | ||
| cm->crlEnabled = 0; | ||
| /* Turn off all checks */ | ||
| cm->crlCheckAll = 0; | ||
| return ret; | ||
| } | ||
| #else | ||
|
There was a problem hiding this comment. If OPENSSL_COMPATIBLE_DEFAULTS is defined, and we don't end up enabling CRL, is there any point in allocating and initializing the cert manager's CRL? Or should the entire function be skipped in this case? I'm not seeing anywhere else in the code where we set crlEnabled. There was a problem hiding this comment. @kareem-wolfssl. Excellent observation! Your feedback has been implemented in the update. |
||
| (void)options; | ||
| #endif | ||
|
|
||
| #ifdef HAVE_CRL | ||
| if (cm->crl == NULL) { | ||
|
|
@@ -8228,10 +8243,15 @@ int wolfSSL_CertManagerEnableCRL(WOLFSSL_CERT_MANAGER* cm, int options) | |
| cm->crl->crlIOCb = EmbedCrlLookup; | ||
| #endif | ||
| } | ||
| #if defined(OPENSSL_COMPATIBLE_DEFAULTS) | ||
| if ((options & WOLFSSL_CRL_CHECKALL) || | ||
| (options & WOLFSSL_CRL_CHECK)) | ||
| #endif | ||
| { | ||
| cm->crlEnabled = 1; | ||
| if (options & WOLFSSL_CRL_CHECKALL) | ||
| cm->crlCheckAll = 1; | ||
| } | ||
| #else | ||
| ret = NOT_COMPILED_IN; | ||
| #endif | ||
|
|
@@ -9431,7 +9451,8 @@ int wolfSSL_CertManagerLoadCRL(WOLFSSL_CERT_MANAGER* cm, const char* path, | |
| return BAD_FUNC_ARG; | ||
|
|
||
| if (cm->crl == NULL) { | ||
| if (wolfSSL_CertManagerEnableCRL(cm, WOLFSSL_CRL_CHECK) | ||
| != WOLFSSL_SUCCESS) { | ||
| WOLFSSL_MSG("Enable CRL failed"); | ||
| return WOLFSSL_FATAL_ERROR; | ||
| } | ||
|
|
@@ -9448,7 +9469,8 @@ int wolfSSL_CertManagerLoadCRLFile(WOLFSSL_CERT_MANAGER* cm, const char* file, | |
| return BAD_FUNC_ARG; | ||
|
|
||
| if (cm->crl == NULL) { | ||
| if (wolfSSL_CertManagerEnableCRL(cm, WOLFSSL_CRL_CHECK) | ||
| != WOLFSSL_SUCCESS) { | ||
| WOLFSSL_MSG("Enable CRL failed"); | ||
| return WOLFSSL_FATAL_ERROR; | ||
| } | ||
|
|
@@ -14494,12 +14516,17 @@ void SetupSession(WOLFSSL* ssl) | |
|
|
||
| WOLFSSL_ENTER("SetupSession"); | ||
|
|
||
| if (!IsAtLeastTLSv1_3(ssl->version) && ssl->arrays != NULL) { | ||
| /* Make sure the session ID is available when the user calls any | ||
| * get_session API */ | ||
| if (!session->haveAltSessionID) { | ||
| XMEMCPY(session->sessionID, ssl->arrays->sessionID, ID_LEN); | ||
| session->sessionIDSz = ssl->arrays->sessionIDSz; | ||
| } | ||
| else { | ||
| XMEMCPY(session->sessionID, session->altSessionID, ID_LEN); | ||
| session->sessionIDSz = ID_LEN; | ||
| } | ||
| } | ||
| session->side = (byte)ssl->options.side; | ||
| if (!IsAtLeastTLSv1_3(ssl->version) && ssl->arrays != NULL) | ||
|
|
@@ -14904,7 +14931,7 @@ int wolfSSL_GetSessionFromCache(WOLFSSL* ssl, WOLFSSL_SESSION* output) | |
| if (SslSessionCacheOff(ssl, ssl->session)) | ||
| return WOLFSSL_FAILURE; | ||
|
|
||
| if (ssl->options.haveSessionId == 0 && !ssl->session->haveAltSessionID) | ||
| return WOLFSSL_FAILURE; | ||
|
|
||
| #ifdef HAVE_SESSION_TICKET | ||
|
|
@@ -14913,7 +14940,8 @@ int wolfSSL_GetSessionFromCache(WOLFSSL* ssl, WOLFSSL_SESSION* output) | |
| #endif | ||
|
|
||
| XMEMSET(bogusID, 0, sizeof(bogusID)); | ||
| if (!IsAtLeastTLSv1_3(ssl->version) && ssl->arrays != NULL | ||
| && !ssl->session->haveAltSessionID) | ||
| id = ssl->arrays->sessionID; | ||
| else if (ssl->session->haveAltSessionID) { | ||
| id = ssl->session->altSessionID; | ||
|
|
@@ -23116,8 +23144,9 @@ int wolfSSL_ERR_GET_REASON(unsigned long err) | |
| #if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY) | ||
| /* Nginx looks for this error to know to stop parsing certificates. | ||
| * Same for HAProxy. */ | ||
| if (err == ((ERR_LIB_PEM << 24) | PEM_R_NO_START_LINE) || | ||
| ((err & 0xFFFFFFL) == -ASN_NO_PEM_HEADER) || | ||
| ((err & 0xFFFL) == PEM_R_NO_START_LINE )) | ||
| return PEM_R_NO_START_LINE; | ||
| if (err == ((ERR_LIB_SSL << 24) | -SSL_R_HTTP_REQUEST)) | ||
| return SSL_R_HTTP_REQUEST; | ||
|
|
@@ -31248,6 +31277,9 @@ WOLFSSL_CTX* wolfSSL_set_SSL_CTX(WOLFSSL* ssl, WOLFSSL_CTX* ctx) | |
| #endif | ||
| if (ssl->ctx) { | ||
| wolfSSL_CTX_free(ssl->ctx); | ||
| #if defined(WOLFSSL_HAPROXY) | ||
| wolfSSL_CTX_free(ssl->initial_ctx); | ||
| #endif | ||
| } | ||
| ssl->ctx = ctx; | ||
|
|
||
|
|
@@ -31450,6 +31482,12 @@ const byte* wolfSSL_SESSION_get_id(const WOLFSSL_SESSION* sess, | |
| WOLFSSL_MSG("Bad func args. Please provide idLen"); | ||
| return NULL; | ||
| } | ||
| #ifdef HAVE_SESSION_TICKET | ||
| if (sess->haveAltSessionID) { | ||
| *idLen = ID_LEN; | ||
| return sess->altSessionID; | ||
| } | ||
| #endif | ||
| *idLen = sess->sessionIDSz; | ||
| return sess->sessionID; | ||
| } | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we support a way to use
initial_ctxwith a macro besidesWOLFSSL_HAPROXY?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dgarske, not in this PR. More comfortable with the current changes since it has been tested against Haproxy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems shortsighted to limit that area of code to HAPROXY only. Could you instead create a new macro like
WOLFSSL_TICKET_USE_INITIAL_CTXand set this macro in settings.h forWOLFSSL_HAPROXY? At least giving other OSP projects the opportunity to use it?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the use case for
initial_ctxis quite rare, and I'm skeptical about its use in other OSP projects.Currently,
#define SSL_TICKET_CTX(ssl) ssl->initial_ctx->ticketEncCtx,initial_ctx, andssl->initial_ctxare gated by WOLFSSL_HAPROXY. Are you suggesting alterations to the gating for all of these?If that's the case, I can replace the
WOLFSSL_HAPROXYgating withWOLFSSL_USE_INITIAL_CTX, which would provide a bit more flexibility.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, looking for flexibility around the use of the
initial_ctxfeature. Its a fairly easy refactor. If you think HAPROXY is the only project that will ever use this feature then we can take the PR as-is.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for clarifying. Currently, I'm inclined to leave it as-is and consider the easy refactor only when necessary. None of our supported OSP projects require it presently, and if they had, we would have run into issues earlier. I can't predict the needs of future OSP projects. Although introducing a new setting is straightforward, it implies the added responsibility of maintenance and instructing non-configure autotool users to add one more setting to their user_settings.h file.