forked from wolfSSL/wolfssl
-
Notifications
You must be signed in to change notification settings - Fork 0
Custom oid in eku #11
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
Open
anhu
wants to merge
1
commit into
master
Choose a base branch
from
custom_oid_in_EKU
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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 hidden or 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 hidden or 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 |
|---|---|---|
|
|
@@ -22031,6 +22031,7 @@ enum { | |
| * @param [out] extExtKeyUsageCount Number of usages read. | ||
| * @param [out] extExtKeyUsage Usages read. | ||
| * @param [out] extExtKeyUsageSsh SSH usages read. | ||
| * @param [in] unknownCb Callback for unknown EKU OIDs. | ||
| * @return 0 on success. | ||
| * @return ASN_BITSTR_E when the expected BIT_STRING tag is not found. | ||
| * @return ASN_PARSE_E when BER encoded data does not match ASN.1 items or | ||
|
|
@@ -22040,18 +22041,30 @@ enum { | |
| int DecodeExtKeyUsage(const byte* input, word32 sz, | ||
| const byte **extExtKeyUsageSrc, word32 *extExtKeyUsageSz, | ||
| word32 *extExtKeyUsageCount, byte *extExtKeyUsage, | ||
| byte *extExtKeyUsageSsh) | ||
| byte *extExtKeyUsageSsh, | ||
| #ifdef WC_ASN_UNKNOWN_EXT_CB | ||
| wc_UnknownExtKeyUsageCallback unknownCb | ||
| #else | ||
| void *unknownCb | ||
| #endif | ||
| ) | ||
| { | ||
| #ifndef WOLFSSL_ASN_TEMPLATE | ||
| word32 idx = 0, oid; | ||
| int length, ret; | ||
| #ifdef WC_ASN_UNKNOWN_EXT_CB | ||
| word32 oidStartIdx; | ||
| #endif | ||
|
|
||
| WOLFSSL_ENTER("DecodeExtKeyUsage"); | ||
|
|
||
| (void) extExtKeyUsageSrc; | ||
| (void) extExtKeyUsageSz; | ||
| (void) extExtKeyUsageCount; | ||
| (void) extExtKeyUsageSsh; | ||
| #ifndef WC_ASN_UNKNOWN_EXT_CB | ||
| (void) unknownCb; | ||
| #endif | ||
|
|
||
| #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) | ||
| *extExtKeyUsageSrc = NULL; | ||
|
|
@@ -22074,9 +22087,34 @@ int DecodeExtKeyUsage(const byte* input, word32 sz, | |
| #endif | ||
|
|
||
| while (idx < (word32)sz) { | ||
| #ifdef WC_ASN_UNKNOWN_EXT_CB | ||
| oidStartIdx = idx; | ||
| #endif | ||
| ret = GetObjectId(input, &idx, &oid, oidCertKeyUseType, sz); | ||
| if (ret == WC_NO_ERR_TRACE(ASN_UNKNOWN_OID_E)) | ||
| if (ret == WC_NO_ERR_TRACE(ASN_UNKNOWN_OID_E)) { | ||
| #ifdef WC_ASN_UNKNOWN_EXT_CB | ||
| if (unknownCb != NULL) { | ||
| word16 decOid[MAX_OID_SZ]; | ||
| word32 decOidSz = sizeof(decOid); | ||
| /* Skip past the tag and length to get raw OID bytes */ | ||
|
Owner
Author
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. @aisle-analyzer , does this comment make sense? |
||
| word32 oidIdx = oidStartIdx; | ||
| int oidLen; | ||
| byte tag; | ||
| if (GetASNTag(input, &oidIdx, &tag, sz) == 0 && | ||
| tag == ASN_OBJECT_ID && | ||
| GetLength(input, &oidIdx, &oidLen, sz) >= 0) { | ||
| ret = DecodeObjectId(input + oidIdx, (word32)oidLen, | ||
| decOid, &decOidSz); | ||
| if (ret == 0) { | ||
| ret = unknownCb(decOid, decOidSz); | ||
| } | ||
| if (ret != 0) | ||
| return ret; | ||
| } | ||
| } | ||
| #endif | ||
| continue; | ||
| } | ||
| else if (ret < 0) | ||
| return ret; | ||
|
|
||
|
|
@@ -22127,13 +22165,19 @@ int DecodeExtKeyUsage(const byte* input, word32 sz, | |
| word32 idx = 0; | ||
| int length; | ||
| int ret = 0; | ||
| #ifdef WC_ASN_UNKNOWN_EXT_CB | ||
| int isKnownOid; | ||
| #endif | ||
|
|
||
| WOLFSSL_ENTER("DecodeExtKeyUsage"); | ||
|
|
||
| (void) extExtKeyUsageSrc; | ||
| (void) extExtKeyUsageSz; | ||
| (void) extExtKeyUsageCount; | ||
| (void) extExtKeyUsageSsh; | ||
| #ifndef WC_ASN_UNKNOWN_EXT_CB | ||
| (void) unknownCb; | ||
| #endif | ||
|
|
||
| #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) | ||
| *extExtKeyUsageSrc = NULL; | ||
|
|
@@ -22171,9 +22215,29 @@ int DecodeExtKeyUsage(const byte* input, word32 sz, | |
| input, &idx, sz); | ||
| /* Skip unknown OIDs. */ | ||
| if (ret == WC_NO_ERR_TRACE(ASN_UNKNOWN_OID_E)) { | ||
| #ifdef WC_ASN_UNKNOWN_EXT_CB | ||
| if (unknownCb != NULL) { | ||
| word16 decOid[MAX_OID_SZ]; | ||
| word32 decOidSz = sizeof(decOid); | ||
| ret = DecodeObjectId( | ||
| dataASN[KEYPURPOSEIDASN_IDX_OID].data.oid.data, | ||
| dataASN[KEYPURPOSEIDASN_IDX_OID].data.oid.length, | ||
| decOid, &decOidSz); | ||
| if (ret == 0) { | ||
| ret = unknownCb(decOid, decOidSz); | ||
| } | ||
| } | ||
| else { | ||
| ret = 0; | ||
| } | ||
| #else | ||
| ret = 0; | ||
| #endif | ||
| } | ||
| else if (ret == 0) { | ||
| #ifdef WC_ASN_UNKNOWN_EXT_CB | ||
| isKnownOid = 1; | ||
| #endif | ||
| /* Store the bit for the OID. */ | ||
| switch (dataASN[KEYPURPOSEIDASN_IDX_OID].data.oid.sum) { | ||
| case EKU_ANY_OID: | ||
|
|
@@ -22197,7 +22261,28 @@ int DecodeExtKeyUsage(const byte* input, word32 sz, | |
| case EKU_OCSP_SIGN_OID: | ||
| *extExtKeyUsage |= EXTKEYUSE_OCSP_SIGN; | ||
| break; | ||
| #ifdef WC_ASN_UNKNOWN_EXT_CB | ||
| default: | ||
| isKnownOid = 0; | ||
| break; | ||
| #endif | ||
| } | ||
|
|
||
| #ifdef WC_ASN_UNKNOWN_EXT_CB | ||
| /* Handle unknown OIDs that parsed successfully but aren't | ||
| * recognized */ | ||
| if (!isKnownOid && unknownCb != NULL) { | ||
| word16 decOid[MAX_OID_SZ]; | ||
| word32 decOidSz = sizeof(decOid); | ||
| ret = DecodeObjectId( | ||
| dataASN[KEYPURPOSEIDASN_IDX_OID].data.oid.data, | ||
| dataASN[KEYPURPOSEIDASN_IDX_OID].data.oid.length, | ||
| decOid, &decOidSz); | ||
| if (ret == 0) { | ||
| ret = unknownCb(decOid, decOidSz); | ||
| } | ||
| } | ||
| #endif | ||
|
|
||
| #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) | ||
| /* Keep count for WOLFSSL_X509. */ | ||
|
|
@@ -22238,7 +22323,12 @@ static int DecodeExtKeyUsageInternal(const byte* input, word32 sz, | |
| #endif | ||
| &cert->extExtKeyUsage, | ||
| #ifdef WOLFSSL_WOLFSSH | ||
| &cert->extExtKeyUsageSsh | ||
| &cert->extExtKeyUsageSsh, | ||
| #else | ||
| NULL, | ||
| #endif | ||
| #ifdef WC_ASN_UNKNOWN_EXT_CB | ||
| cert->unknownExtKeyUsageCallback | ||
| #else | ||
| NULL | ||
| #endif | ||
|
|
@@ -23663,6 +23753,16 @@ int wc_SetUnknownExtCallbackEx(DecodedCert* cert, | |
| cert->unknownExtCallbackExCtx = ctx; | ||
| return 0; | ||
| } | ||
|
|
||
| int wc_SetUnknownExtKeyUsageCallback(DecodedCert* cert, | ||
| wc_UnknownExtKeyUsageCallback cb) { | ||
| if (cert == NULL) { | ||
| return BAD_FUNC_ARG; | ||
| } | ||
|
|
||
| cert->unknownExtKeyUsageCallback = cb; | ||
| return 0; | ||
| } | ||
| #endif /* WC_ASN_UNKNOWN_EXT_CB */ | ||
|
|
||
| /* | ||
|
|
||
This file contains hidden or 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
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.
Buffer size passed as bytes instead of elements
Medium Severity
decOidSzis initialized tosizeof(decOid)which yields the byte size (MAX_OID_SZ * sizeof(word16)= 64), butDecodeObjectIdinterprets*outSzas the number of elements in the array (documented as "On in, number of elements in array"). ThedecOidarray only holdsMAX_OID_SZ(32) elements. The bounds check insideDecodeObjectId(y >= (int)*outSz) would allow writing up to index 63, causing a stack buffer overflow for OIDs with more than ~31 dotted components. The correct value isMAX_OID_SZorsizeof(decOid)/sizeof(decOid[0]). This follows a pre-existing pattern elsewhere in the file, but each new call site is a new exploitable path reachable via a crafted certificate.Additional Locations (2)
wolfcrypt/src/asn.c#L22220-L22221wolfcrypt/src/asn.c#L22275-L22276