Juan Lang : crypt32: Check key usage during chain validation.
Alexandre Julliard
julliard at winehq.org
Wed Oct 21 13:14:08 CDT 2009
Module: wine
Branch: master
Commit: 7fa618aa8e4351ff9e284fa8cf8ac6cc869b7f29
URL: http://source.winehq.org/git/wine.git/?a=commit;h=7fa618aa8e4351ff9e284fa8cf8ac6cc869b7f29
Author: Juan Lang <juan.lang at gmail.com>
Date: Tue Oct 20 18:25:06 2009 -0700
crypt32: Check key usage during chain validation.
---
dlls/crypt32/chain.c | 125 +++++++++++++++++++++++++++++++++++++++-----
dlls/crypt32/tests/chain.c | 6 +-
2 files changed, 114 insertions(+), 17 deletions(-)
diff --git a/dlls/crypt32/chain.c b/dlls/crypt32/chain.c
index 1dc7a74..0490723 100644
--- a/dlls/crypt32/chain.c
+++ b/dlls/crypt32/chain.c
@@ -815,6 +815,106 @@ static void dump_element(PCCERT_CONTEXT cert)
dump_extension(&cert->pCertInfo->rgExtension[i]);
}
+static void trace_usage(const CRYPT_BIT_BLOB *usage)
+{
+#define trace_usage_bit(bits, bit) \
+ if ((bits) & (bit)) TRACE_(chain)("%s\n", #bit)
+ if (usage->cbData)
+ {
+ trace_usage_bit(usage->pbData[0], CERT_DIGITAL_SIGNATURE_KEY_USAGE);
+ trace_usage_bit(usage->pbData[0], CERT_NON_REPUDIATION_KEY_USAGE);
+ trace_usage_bit(usage->pbData[0], CERT_KEY_ENCIPHERMENT_KEY_USAGE);
+ trace_usage_bit(usage->pbData[0], CERT_DATA_ENCIPHERMENT_KEY_USAGE);
+ trace_usage_bit(usage->pbData[0], CERT_KEY_AGREEMENT_KEY_USAGE);
+ trace_usage_bit(usage->pbData[0], CERT_KEY_CERT_SIGN_KEY_USAGE);
+ trace_usage_bit(usage->pbData[0], CERT_CRL_SIGN_KEY_USAGE);
+ trace_usage_bit(usage->pbData[0], CERT_ENCIPHER_ONLY_KEY_USAGE);
+ }
+#undef trace_usage_bit
+ if (usage->cbData > 1 && usage->pbData[1] & CERT_DECIPHER_ONLY_KEY_USAGE)
+ TRACE_(chain)("CERT_DECIPHER_ONLY_KEY_USAGE\n");
+}
+
+static BOOL CRYPT_KeyUsageValid(PCCERT_CONTEXT cert, BOOL isRoot, BOOL isCA,
+ DWORD index)
+{
+ PCERT_EXTENSION ext;
+ BOOL ret;
+ BYTE usageBits = 0;
+
+ ext = CertFindExtension(szOID_KEY_USAGE, cert->pCertInfo->cExtension,
+ cert->pCertInfo->rgExtension);
+ if (ext)
+ {
+ CRYPT_BIT_BLOB usage;
+ DWORD size = sizeof(usage);
+
+ ret = CryptDecodeObjectEx(cert->dwCertEncodingType, X509_BITS,
+ ext->Value.pbData, ext->Value.cbData, CRYPT_DECODE_NOCOPY_FLAG, NULL,
+ &usage, &size);
+ if (!ret)
+ return FALSE;
+ else if (usage.cbData > 2)
+ {
+ /* The key usage extension only defines 9 bits => no more than 2
+ * bytes are needed to encode all known usages.
+ */
+ return FALSE;
+ }
+ else
+ {
+ /* The only bit relevant to chain validation is the keyCertSign
+ * bit, which is always in the least significant byte of the
+ * key usage bits.
+ */
+ usageBits = usage.pbData[usage.cbData - 1];
+ if (TRACE_ON(chain))
+ trace_usage(&usage);
+ }
+ }
+ if (isCA)
+ {
+ if (!ext)
+ {
+ /* MS appears to violate RFC 3280, section 4.2.1.3 (Key Usage)
+ * here. Quoting the RFC:
+ * "This [key usage] extension MUST appear in certificates that
+ * contain public keys that are used to validate digital signatures
+ * on other public key certificates or CRLs."
+ * Most of the test chains' certs do not contain key usage
+ * extensions, yet are allowed to be CA certs. This appears to
+ * be common usage too: the root CA in a chain often does not have
+ * the key usage extension. We are a little more restrictive:
+ * root certs, which commonly do not have any extensions, are
+ * allowed to sign certificates without the key usage extension.
+ */
+ WARN_(chain)("no key usage extension on a CA cert\n");
+ ret = isRoot;
+ }
+ else
+ {
+ if (!(usageBits & CERT_KEY_CERT_SIGN_KEY_USAGE))
+ {
+ WARN_(chain)("keyCertSign not asserted on a CA cert\n");
+ ret = FALSE;
+ }
+ else
+ ret = TRUE;
+ }
+ }
+ else
+ {
+ if (ext && (usageBits & CERT_KEY_CERT_SIGN_KEY_USAGE))
+ {
+ WARN_(chain)("keyCertSign asserted on a non-CA cert\n");
+ ret = FALSE;
+ }
+ else
+ ret = TRUE;
+ }
+ return ret;
+}
+
static BOOL CRYPT_CriticalExtensionsSupported(PCCERT_CONTEXT cert)
{
BOOL ret = TRUE;
@@ -833,13 +933,7 @@ static BOOL CRYPT_CriticalExtensionsSupported(PCCERT_CONTEXT cert)
else if (!strcmp(oid, szOID_NAME_CONSTRAINTS))
ret = TRUE;
else if (!strcmp(oid, szOID_KEY_USAGE))
- {
- static int warned;
-
- if (!warned++)
- FIXME("key usage extension unsupported, ignoring\n");
ret = TRUE;
- }
else if (!strcmp(oid, szOID_SUBJECT_ALT_NAME))
ret = TRUE;
else
@@ -865,21 +959,21 @@ static void CRYPT_CheckSimpleChain(PCertificateChainEngine engine,
chain->cElement, debugstr_w(filetime_to_str(time)));
for (i = chain->cElement - 1; i >= 0; i--)
{
+ BOOL isRoot;
+
if (TRACE_ON(chain))
dump_element(chain->rgpElement[i]->pCertContext);
if (CertVerifyTimeValidity(time,
chain->rgpElement[i]->pCertContext->pCertInfo) != 0)
chain->rgpElement[i]->TrustStatus.dwErrorStatus |=
CERT_TRUST_IS_NOT_TIME_VALID;
+ if (i == chain->cElement - 1)
+ isRoot = CRYPT_IsCertificateSelfSigned(
+ chain->rgpElement[i]->pCertContext);
+ else
+ isRoot = FALSE;
if (i != 0)
{
- BOOL isRoot;
-
- if (i == chain->cElement - 1)
- isRoot = CRYPT_IsCertificateSelfSigned(
- chain->rgpElement[i]->pCertContext);
- else
- isRoot = FALSE;
/* Check the signature of the cert this issued */
if (!CryptVerifyCertificateSignatureEx(0, X509_ASN_ENCODING,
CRYPT_VERIFY_CERT_SIGN_SUBJECT_CERT,
@@ -914,6 +1008,10 @@ static void CRYPT_CheckSimpleChain(PCertificateChainEngine engine,
chain->rgpElement[i]->TrustStatus.dwErrorStatus |=
CERT_TRUST_INVALID_BASIC_CONSTRAINTS;
}
+ if (!CRYPT_KeyUsageValid(chain->rgpElement[i]->pCertContext, isRoot,
+ constraints.fCA, i))
+ chain->rgpElement[i]->TrustStatus.dwErrorStatus |=
+ CERT_TRUST_IS_NOT_VALID_FOR_USAGE;
if (CRYPT_IsSimpleChainCyclic(chain))
{
/* If the chain is cyclic, then the path length constraints
@@ -924,7 +1022,6 @@ static void CRYPT_CheckSimpleChain(PCertificateChainEngine engine,
CERT_TRUST_IS_PARTIAL_CHAIN |
CERT_TRUST_INVALID_BASIC_CONSTRAINTS;
}
- /* FIXME: check valid usages */
/* Check whether every critical extension is supported */
if (!CRYPT_CriticalExtensionsSupported(
chain->rgpElement[i]->pCertContext))
diff --git a/dlls/crypt32/tests/chain.c b/dlls/crypt32/tests/chain.c
index 44fcacb..d29c86a 100644
--- a/dlls/crypt32/tests/chain.c
+++ b/dlls/crypt32/tests/chain.c
@@ -1782,7 +1782,7 @@ static ChainCheck chainCheck[] = {
{ { CERT_TRUST_IS_NOT_TIME_NESTED, CERT_TRUST_HAS_PREFERRED_ISSUER },
{ CERT_TRUST_IS_UNTRUSTED_ROOT | CERT_TRUST_IS_NOT_VALID_FOR_USAGE, 0 },
1, simpleStatus15 },
- TODO_ERROR },
+ 0 },
/* Windows XP incorrectly does not complain that the end cert's key usage is
* invalid, so ignore that error.
*/
@@ -1791,7 +1791,7 @@ static ChainCheck chainCheck[] = {
CERT_TRUST_HAS_PREFERRED_ISSUER },
{ CERT_TRUST_IS_UNTRUSTED_ROOT | CERT_TRUST_IS_NOT_VALID_FOR_USAGE, 0 },
1, simpleStatus16 },
- TODO_ERROR },
+ 0 },
{ { sizeof(chain17) / sizeof(chain17[0]), chain17 },
{ { CERT_TRUST_IS_NOT_TIME_NESTED, CERT_TRUST_HAS_PREFERRED_ISSUER },
{ CERT_TRUST_IS_UNTRUSTED_ROOT, 0 }, 1, simpleStatus17 },
@@ -1800,7 +1800,7 @@ static ChainCheck chainCheck[] = {
{ { CERT_TRUST_IS_NOT_TIME_NESTED, CERT_TRUST_HAS_PREFERRED_ISSUER },
{ CERT_TRUST_IS_UNTRUSTED_ROOT | CERT_TRUST_IS_NOT_VALID_FOR_USAGE, 0 },
1, simpleStatus18 },
- TODO_ERROR },
+ 0 },
{ { sizeof(selfSignedChain) / sizeof(selfSignedChain[0]), selfSignedChain },
{ { 0, CERT_TRUST_HAS_PREFERRED_ISSUER },
{ CERT_TRUST_IS_NOT_TIME_VALID | CERT_TRUST_IS_UNTRUSTED_ROOT, 0 },
More information about the wine-cvs
mailing list