From 4b22986e7426cc8b8989bd17157b0aedf4b8ffcf Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 28 Mar 2014 10:10:22 -0700 Subject: [PATCH 1/5] Check for Certificate Sign key usage bit on intermediate CAs. --- src/ssl.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/ssl.c b/src/ssl.c index 2afb77997..0f4f7645a 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -1497,6 +1497,13 @@ int AddCA(CYASSL_CERT_MANAGER* cm, buffer der, int type, int verify) CYASSL_MSG(" Can't add as CA if not actually one"); ret = NOT_CA_ERROR; } + else if (ret == 0 && cert.isCA == 1 && type != CYASSL_USER_CA && + (cert.extKeyUsage & KEYUSE_KEY_CERT_SIGN) == 0) { + /* Intermediate CA certs are required to have the keyCertSign + * extension set. User loaded root certs are not. */ + CYASSL_MSG(" Doesn't have key usage certificate signing"); + ret = NOT_CA_ERROR; + } else if (ret == 0 && AlreadySigner(cm, subjectHash)) { CYASSL_MSG(" Already have this CA, not adding again"); (void)ret; From b5a27b0f411f0b45745d4fbe8e65f569fbc15d7b Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 28 Mar 2014 11:21:07 -0700 Subject: [PATCH 2/5] Add compile flag to disable Cert Sign key usage flag check. --- src/ssl.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 0f4f7645a..17d649863 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -1497,13 +1497,15 @@ int AddCA(CYASSL_CERT_MANAGER* cm, buffer der, int type, int verify) CYASSL_MSG(" Can't add as CA if not actually one"); ret = NOT_CA_ERROR; } - else if (ret == 0 && cert.isCA == 1 && type != CYASSL_USER_CA && + #ifndef ALLOW_INVALID_CERTSIGN + else if (ret == 0 && cert.isCA == 1 && type != CYASSL_USER_CA && (cert.extKeyUsage & KEYUSE_KEY_CERT_SIGN) == 0) { - /* Intermediate CA certs are required to have the keyCertSign - * extension set. User loaded root certs are not. */ - CYASSL_MSG(" Doesn't have key usage certificate signing"); - ret = NOT_CA_ERROR; - } + /* Intermediate CA certs are required to have the keyCertSign + * extension set. User loaded root certs are not. */ + CYASSL_MSG(" Doesn't have key usage certificate signing"); + ret = NOT_CA_ERROR; + } + #endif else if (ret == 0 && AlreadySigner(cm, subjectHash)) { CYASSL_MSG(" Already have this CA, not adding again"); (void)ret; From 1f3bc9263d2b3f91c9a475a5a3a8b592cd8b4424 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 28 Mar 2014 11:25:05 -0700 Subject: [PATCH 3/5] error to have v1 or v2 certificates with extensions --- ctaocrypt/src/asn.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/ctaocrypt/src/asn.c b/ctaocrypt/src/asn.c index 7610152e0..8597fa238 100644 --- a/ctaocrypt/src/asn.c +++ b/ctaocrypt/src/asn.c @@ -3594,13 +3594,18 @@ int ParseCertRelative(DecodedCert* cert, int type, int verify, void* cm) CYASSL_MSG("Parsed Past Key"); - if (cert->srcIdx != cert->sigIndex) { - if (cert->srcIdx < cert->sigIndex) { - /* save extensions */ - cert->extensions = &cert->source[cert->srcIdx]; - cert->extensionsSz = cert->sigIndex - cert->srcIdx; - cert->extensionsIdx = cert->srcIdx; /* for potential later use */ - } + if (cert->srcIdx < cert->sigIndex) { + #ifndef ALLOW_V1_EXTENSIONS + if (cert->version < 2) { + CYASSL_MSG(" v1 and v2 certs not allowed extensions"); + return ASN_VERSION_E; + } + #endif + /* save extensions */ + cert->extensions = &cert->source[cert->srcIdx]; + cert->extensionsSz = cert->sigIndex - cert->srcIdx; + cert->extensionsIdx = cert->srcIdx; /* for potential later use */ + if ((ret = DecodeCertExtensions(cert)) < 0) { if (ret == ASN_CRIT_EXT_E) criticalExt = ret; From e79ce42ef426b876f304bdc67e9fe9fade06b262 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 10 Apr 2014 16:50:14 -0700 Subject: [PATCH 4/5] Added checking of the key usage and extended key usage extensions in the certificates. --- ctaocrypt/src/asn.c | 6 +++++ cyassl/ctaocrypt/asn.h | 1 + cyassl/error-ssl.h | 4 +++ src/internal.c | 59 ++++++++++++++++++++++++++++++++++++++++++ src/ssl.c | 2 ++ 5 files changed, 72 insertions(+) diff --git a/ctaocrypt/src/asn.c b/ctaocrypt/src/asn.c index 8597fa238..205310bf3 100644 --- a/ctaocrypt/src/asn.c +++ b/ctaocrypt/src/asn.c @@ -6493,6 +6493,12 @@ int ParseCRL(DecodedCRL* dcrl, const byte* buff, word32 sz, void* cm) if (ca) { CYASSL_MSG("Found CRL issuer CA"); /* try to confirm/verify signature */ + #ifndef IGNORE_KEY_EXTENSIONS + if ((ca->keyUsage & KEYUSE_CRL_SIGN) == 0) { + CYASSL_MSG("CA cannot sign CRLs"); + return ASN_CRL_NO_SIGNER_E; + } + #endif /* IGNORE_KEY_EXTENSIONS */ if (!ConfirmSignature(buff + dcrl->certBegin, dcrl->sigIndex - dcrl->certBegin, ca->publicKey, ca->pubKeySize, ca->keyOID, diff --git a/cyassl/ctaocrypt/asn.h b/cyassl/ctaocrypt/asn.h index fe961afdb..751739dd7 100644 --- a/cyassl/ctaocrypt/asn.h +++ b/cyassl/ctaocrypt/asn.h @@ -426,6 +426,7 @@ struct DecodedCert { struct Signer { word32 pubKeySize; word32 keyOID; /* key type */ + word16 keyUsage; byte* publicKey; int nameLen; char* name; /* common name */ diff --git a/cyassl/error-ssl.h b/cyassl/error-ssl.h index 71f4b4ffd..134c0e6a7 100644 --- a/cyassl/error-ssl.h +++ b/cyassl/error-ssl.h @@ -115,6 +115,10 @@ enum CyaSSL_ErrorCodes { UNKNOWN_SNI_HOST_NAME_E = -281, /* Unrecognized host name Error */ UNKNOWN_MAX_FRAG_LEN_E = -282, /* Unrecognized max frag len Error */ /* add strings to SetErrorString !!!!! */ + KEYUSE_SIGNATURE_E = -283, /* KeyUse digSignature error */ + KEYUSE_AGREEMENT_E = -284, /* KeyUse keyAgreement error */ + KEYUSE_ENCIPHER_E = -285, /* KeyUse keyEncipher error */ + EXTKEYUSE_AUTH_E = -286, /* ExtKeyUse server|client_auth */ /* begin negotiation parameter errors */ UNSUPPORTED_SUITE = -290, /* unsupported cipher suite */ diff --git a/src/internal.c b/src/internal.c index 044342304..dac18b89f 100644 --- a/src/internal.c +++ b/src/internal.c @@ -3465,6 +3465,49 @@ static int DoCertificate(CYASSL* ssl, byte* input, word32* inOutIdx, } #endif +#ifndef IGNORE_KEY_EXTENSIONS + if (dCert.extKeyUsageSet) { + if ((ssl->specs.kea == rsa_kea) && + (dCert.extKeyUsage & KEYUSE_KEY_ENCIPHER) == 0) { + fatal = 1; + ret = KEYUSE_ENCIPHER_E; + } + if ((ssl->specs.kea == diffie_hellman_kea || + ssl->specs.kea == ecc_diffie_hellman_kea || + ssl->specs.kea == ecc_static_diffie_hellman_kea) && + (dCert.extKeyUsage & KEYUSE_KEY_AGREE) == 0) { + fatal = 1; + ret = KEYUSE_AGREEMENT_E; + } + if ((ssl->specs.sig_algo == rsa_sa_algo || + ssl->specs.sig_algo == ecc_dsa_sa_algo) && + (dCert.extKeyUsage & KEYUSE_DIGITAL_SIG) == 0) { + CYASSL_MSG("KeyUse Digital Sig not set"); + fatal = 1; + ret = KEYUSE_SIGNATURE_E; + } + } + + if (dCert.extExtKeyUsageSet) { + if (ssl->options.side == CYASSL_CLIENT_END) { + if ((dCert.extExtKeyUsage & + (EXTKEYUSE_ANY | EXTKEYUSE_SERVER_AUTH)) == 0) { + CYASSL_MSG("ExtKeyUse Server Auth not set"); + fatal = 1; + ret = EXTKEYUSE_AUTH_E; + } + } + else { + if ((dCert.extExtKeyUsage & + (EXTKEYUSE_ANY | EXTKEYUSE_CLIENT_AUTH)) == 0) { + CYASSL_MSG("ExtKeyUse Client Auth not set"); + fatal = 1; + ret = EXTKEYUSE_AUTH_E; + } + } + } +#endif /* IGNORE_KEY_EXTENSIONS */ + if (fatal) { FreeDecodedCert(&dCert); ssl->error = ret; @@ -6451,6 +6494,22 @@ void SetErrorString(int error, char* str) XSTRNCPY(str, "Unrecognized host name Error", max); break; + case KEYUSE_SIGNATURE_E: + XSTRNCPY(str, "Key Use digitalSignature not set Error", max); + break; + + case KEYUSE_AGREEMENT_E: + XSTRNCPY(str, "Key Use keyAgreement not set Error", max); + break; + + case KEYUSE_ENCIPHER_E: + XSTRNCPY(str, "Key Use keyEncipherment not set Error", max); + break; + + case EXTKEYUSE_AUTH_E: + XSTRNCPY(str, "Ext Key Use server/client auth not set Error", max); + break; + default : XSTRNCPY(str, "unknown error number", max); } diff --git a/src/ssl.c b/src/ssl.c index 17d649863..1bcaa594a 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -1526,6 +1526,8 @@ int AddCA(CYASSL_CERT_MANAGER* cm, buffer der, int type, int verify) cert.extSubjKeyId, SHA_DIGEST_SIZE); #endif XMEMCPY(signer->subjectNameHash, cert.subjectHash, SHA_DIGEST_SIZE); + signer->keyUsage = cert.extKeyUsageSet ? cert.extKeyUsage : 0xFFFF; + /* If Key Usage not set, all uses valid. */ signer->next = NULL; /* in case lock fails */ cert.publicKey = 0; /* don't free here */ From 603192f1533a21d493ba67452303fba96d020b5f Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 10 Apr 2014 23:31:43 -0700 Subject: [PATCH 5/5] Removed an incorrect key use check. --- cyassl/error-ssl.h | 1 - src/internal.c | 11 ----------- 2 files changed, 12 deletions(-) diff --git a/cyassl/error-ssl.h b/cyassl/error-ssl.h index 134c0e6a7..fd34c8896 100644 --- a/cyassl/error-ssl.h +++ b/cyassl/error-ssl.h @@ -116,7 +116,6 @@ enum CyaSSL_ErrorCodes { UNKNOWN_MAX_FRAG_LEN_E = -282, /* Unrecognized max frag len Error */ /* add strings to SetErrorString !!!!! */ KEYUSE_SIGNATURE_E = -283, /* KeyUse digSignature error */ - KEYUSE_AGREEMENT_E = -284, /* KeyUse keyAgreement error */ KEYUSE_ENCIPHER_E = -285, /* KeyUse keyEncipher error */ EXTKEYUSE_AUTH_E = -286, /* ExtKeyUse server|client_auth */ diff --git a/src/internal.c b/src/internal.c index dac18b89f..ba4b754d9 100644 --- a/src/internal.c +++ b/src/internal.c @@ -3472,13 +3472,6 @@ static int DoCertificate(CYASSL* ssl, byte* input, word32* inOutIdx, fatal = 1; ret = KEYUSE_ENCIPHER_E; } - if ((ssl->specs.kea == diffie_hellman_kea || - ssl->specs.kea == ecc_diffie_hellman_kea || - ssl->specs.kea == ecc_static_diffie_hellman_kea) && - (dCert.extKeyUsage & KEYUSE_KEY_AGREE) == 0) { - fatal = 1; - ret = KEYUSE_AGREEMENT_E; - } if ((ssl->specs.sig_algo == rsa_sa_algo || ssl->specs.sig_algo == ecc_dsa_sa_algo) && (dCert.extKeyUsage & KEYUSE_DIGITAL_SIG) == 0) { @@ -6498,10 +6491,6 @@ void SetErrorString(int error, char* str) XSTRNCPY(str, "Key Use digitalSignature not set Error", max); break; - case KEYUSE_AGREEMENT_E: - XSTRNCPY(str, "Key Use keyAgreement not set Error", max); - break; - case KEYUSE_ENCIPHER_E: XSTRNCPY(str, "Key Use keyEncipherment not set Error", max); break;