-
Notifications
You must be signed in to change notification settings - Fork 110
Reject non-CA peer intermediate certs #1021
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,7 @@ | |
|
|
||
| #include <wolfssl/ssl.h> | ||
| #include <wolfssl/ocsp.h> | ||
| #include <wolfssl/wolfcrypt/asn.h> | ||
| #include <wolfssl/wolfcrypt/error-crypt.h> | ||
| #include <wolfssl/error-ssl.h> | ||
|
|
||
|
|
@@ -202,6 +203,31 @@ enum { | |
| #define MAX_CHAIN_DEPTH 9 | ||
| #endif | ||
|
|
||
| /* Returns 1 if der is a CA: isCA set and, unless self-signed, keyCertSign | ||
| * set. Already signature-verified by the caller, so parse NO_VERIFY. */ | ||
| static int CertManIntermediateIsCA(WOLFSSH_CERTMAN* cm, | ||
| const unsigned char* der, word32 derSz) | ||
| { | ||
| DecodedCert decoded; | ||
| int isCA = 0; | ||
|
|
||
| wc_InitDecodedCert(&decoded, der, derSz, cm->heap); | ||
| if (wc_ParseCert(&decoded, WOLFSSL_FILETYPE_ASN1, NO_VERIFY, NULL) == 0) { | ||
| isCA = decoded.isCA; | ||
| #ifndef ALLOW_INVALID_CERTSIGN | ||
| if (isCA && !decoded.selfSigned && decoded.extKeyUsageSet && | ||
| (decoded.extKeyUsage & KEYUSE_KEY_CERT_SIGN) == 0) { | ||
| /* If a KeyUsage extension is present, an intermediate CA must | ||
| * assert the keyCertSign bit. */ | ||
| isCA = 0; | ||
| } | ||
| #endif | ||
| } | ||
| wc_FreeDecodedCert(&decoded); | ||
|
|
||
| return isCA; | ||
| } | ||
|
|
||
| /* if handling a chain it is expected to be the leaf cert first followed by | ||
| * intermediates and CA last (CA may be omitted) */ | ||
| int wolfSSH_CERTMAN_VerifyCerts_buffer(WOLFSSH_CERTMAN* cm, | ||
|
|
@@ -301,18 +327,35 @@ int wolfSSH_CERTMAN_VerifyCerts_buffer(WOLFSSH_CERTMAN* cm, | |
| WLOG(WS_LOG_CERTMAN, "ocsp lookup: ocsp revoked"); | ||
| ret = WS_CERT_REVOKED_E; | ||
| } | ||
| else if (ret == OCSP_NEED_URL) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ⚪ [Info] OCSP_NEED_URL softens revocation from hard-fail to soft-fail · Authentication bypass
Fix: Confirm soft-fail is intended; optionally gate it behind a config flag or require a default OCSP responder to keep hard-fail enforcement. |
||
| /* The cert carries no OCSP responder URL and certman has | ||
| * no default responder configured, so OCSP cannot be | ||
| * performed. Treat as not revoked rather than failing the | ||
| * whole verification. */ | ||
|
Comment on lines
+330
to
+334
|
||
| WLOG(WS_LOG_CERTMAN, "ocsp lookup: no responder url, skipping"); | ||
| ret = WS_SUCCESS; | ||
| } | ||
| else { | ||
| WLOG(WS_LOG_CERTMAN, "ocsp lookup: other error (%d)", ret); | ||
| ret = WS_CERT_OTHER_E; | ||
| } | ||
| } | ||
| #endif /* HAVE_OCSP */ | ||
|
|
||
| /* verified successfully, add intermideate as trusted */ | ||
| /* promote the intermediate so the next cert has a signer, but | ||
| * only if it's actually a CA */ | ||
| if (ret == WS_SUCCESS && certIdx > 0) { | ||
| WLOG(WS_LOG_CERTMAN, "adding intermediate cert as trusted"); | ||
| ret = wolfSSH_CERTMAN_LoadRootCA_buffer(cm, certLoc[certIdx], | ||
| certLen[certIdx]); | ||
| if (CertManIntermediateIsCA(cm, certLoc[certIdx], | ||
| certLen[certIdx])) { | ||
| WLOG(WS_LOG_CERTMAN, "adding intermediate cert as trusted"); | ||
| ret = wolfSSH_CERTMAN_LoadRootCA_buffer(cm, | ||
| certLoc[certIdx], certLen[certIdx]); | ||
| } | ||
| else { | ||
| WLOG(WS_LOG_CERTMAN, | ||
| "peer intermediate is not a CA; not promoting"); | ||
| ret = WS_CERT_NO_SIGNER_E; | ||
| } | ||
| } | ||
|
ejohnstown marked this conversation as resolved.
|
||
|
|
||
| if (ret != WS_SUCCESS) { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.