diff --git a/src/mscng/app.c b/src/mscng/app.c index bc654bbd4..728a9612a 100755 --- a/src/mscng/app.c +++ b/src/mscng/app.c @@ -469,7 +469,7 @@ xmlSecMSCngAppPkcs12LoadMemory(const xmlSecByte* data, xmlSecSize dataSize, cons goto cleanup; } - DWORD flags = CRYPT_EXPORTABLE; + DWORD flags = CRYPT_EXPORTABLE | PKCS12_ALWAYS_CNG_KSP; if (!xmlSecImportGetPersistKey()) { flags |= PKCS12_NO_PERSIST_KEY; } diff --git a/src/mscng/ciphers.c b/src/mscng/ciphers.c index 718da159c..dd3ad39aa 100644 --- a/src/mscng/ciphers.c +++ b/src/mscng/ciphers.c @@ -528,6 +528,7 @@ static int xmlSecMSCngGCMBlockCipherCtxInit(xmlSecMSCngBlockCipherCtxPtr ctx, xmlSecMSCngNtError("BCryptGetProperty", cipherName, status); return(-1); } + xmlSecAssert2(bytesRead == sizeof(authTagLengths), -1); if(ctx->authInfo.pbMacContext == NULL) { ctx->authInfo.pbMacContext = xmlMalloc(authTagLengths.dwMaxLength); @@ -675,6 +676,9 @@ xmlSecMSCngCBCBlockCipherCtxUpdate(xmlSecMSCngBlockCipherCtxPtr ctx, inBlocks = (inSize - 1) / blockSize; } inSize = inBlocks * blockSize; + if(inSize == 0) { + return(0); + } /* we write out the input size plus maybe one block */ if((inSize > XMLSEC_SIZE_MAX - blockSize) || (outSize > XMLSEC_SIZE_MAX - inSize - blockSize)) { @@ -814,6 +818,11 @@ xmlSecMSCngGCMBlockCipherCtxUpdate(xmlSecMSCngBlockCipherCtxPtr ctx, XMLSEC_SAFE_CAST_SIZE_TO_ULONG(inSize, inLen, return(-1), cipherName); outSize = xmlSecBufferGetSize(out); + if(outSize > XMLSEC_SIZE_MAX - inSize) { + xmlSecInternalError3("xmlSecBufferSetMaxSize", cipherName, + "outSize=" XMLSEC_SIZE_FMT "; inSize=" XMLSEC_SIZE_FMT, outSize, inSize); + return(-1); + } ret = xmlSecBufferSetMaxSize(out, outSize + inSize); if(ret < 0) { xmlSecInternalError2("xmlSecBufferSetMaxSize", cipherName, @@ -1203,7 +1212,7 @@ xmlSecMSCngGCMBlockCipherCtxFinal(xmlSecMSCngBlockCipherCtxPtr ctx, /* check if we really have decrypted the numbers of bytes that we * requested */ if(dwCLen != dwInSize) { - xmlSecInternalError3("BCryptEncrypt", cipherName, + xmlSecInternalError3("BCryptDecrypt", cipherName, "in-size=%lu; out-size=%lu", dwInSize, dwCLen); return(-1); } diff --git a/src/mscng/crypto.c b/src/mscng/crypto.c index 220311861..90d1b8a7e 100755 --- a/src/mscng/crypto.c +++ b/src/mscng/crypto.c @@ -87,11 +87,6 @@ xmlSecCryptoGetFunctions_mscng(void) { gXmlSecMSCngFunctions->keyDataEcdsaGetKlass = xmlSecMSCngKeyDataEcdsaGetKlass; #endif /* XMLSEC_NO_ECDSA */ -#ifdef XMLSEC_MSCNG_TODO - gXmlSecMSCngFunctions->keyDataGost2001GetKlass = xmlSecMSCngKeyDataGost2001GetKlass; - gXmlSecMSCngFunctions->keyDataGostR3410_2012GetKlass = xmlSecMSCngKeyDataGostR3410_2012GetKlass; -#endif /* XMLSEC_NO_GOST */ - #ifndef XMLSEC_NO_HMAC gXmlSecMSCngFunctions->keyDataHmacGetKlass = xmlSecMSCngKeyDataHmacGetKlass; #endif /* XMLSEC_NO_HMAC */ @@ -146,10 +141,6 @@ xmlSecCryptoGetFunctions_mscng(void) { gXmlSecMSCngFunctions->transformDsaSha1GetKlass = xmlSecMSCngTransformDsaSha1GetKlass; #endif /* XMLSEC_NO_SHA1 */ -#ifdef XMLSEC_MSCNG_TODO - gXmlSecMSCngFunctions->transformDsaSha256GetKlass = xmlSecMSCngTransformDsaSha256GetKlass; -#endif /* XMLSEC_NO_SHA256 */ - #endif /* XMLSEC_NO_DSA */ /******************************* ECDSA ********************************/ @@ -159,10 +150,6 @@ xmlSecCryptoGetFunctions_mscng(void) { gXmlSecMSCngFunctions->transformEcdsaSha1GetKlass = xmlSecMSCngTransformEcdsaSha1GetKlass; #endif /* XMLSEC_NO_SHA1 */ -#ifdef XMLSEC_MSCNG_TODO - gXmlSecMSCngFunctions->transformEcdsaSha224GetKlass = xmlSecMSCngTransformEcdsaSha224GetKlass; -#endif /* XMLSEC_NO_SHA224 */ - #ifndef XMLSEC_NO_SHA256 gXmlSecMSCngFunctions->transformEcdsaSha256GetKlass = xmlSecMSCngTransformEcdsaSha256GetKlass; #endif /* XMLSEC_NO_SHA256 */ @@ -177,19 +164,6 @@ xmlSecCryptoGetFunctions_mscng(void) { #endif /* XMLSEC_NO_ECDSA */ - /******************************* GOST ********************************/ -#ifdef XMLSEC_MSCNG_TODO - gXmlSecMSCngFunctions->transformGost2001GostR3411_94GetKlass = xmlSecMSCngTransformGost2001GostR3411_94GetKlass; - gXmlSecMSCngFunctions->transformGostR3410_2012GostR3411_2012_256GetKlass = xmlSecMSCngTransformGostR3410_2012GostR3411_2012_256GetKlass; - gXmlSecMSCngFunctions->transformGostR3410_2012GostR3411_2012_512GetKlass = xmlSecMSCngTransformGostR3410_2012GostR3411_2012_512GetKlass; -#endif /* XMLSEC_NO_GOST */ - -#ifdef XMLSEC_MSCNG_TODO - gXmlSecMSCngFunctions->transformGostR3411_94GetKlass = xmlSecMSCngTransformGostR3411_94GetKlass; - gXmlSecMSCngFunctions->transformGostR3411_2012_256GetKlass = xmlSecMSCngTransformGostR3411_2012_256GetKlass; - gXmlSecMSCngFunctions->transformGostR3411_2012_512GetKlass = xmlSecMSCngTransformGostR3411_2012_512GetKlass; -#endif /* XMLSEC_NO_GOST */ - /******************************* HMAC ********************************/ #ifndef XMLSEC_NO_HMAC @@ -213,21 +187,8 @@ xmlSecCryptoGetFunctions_mscng(void) { gXmlSecMSCngFunctions->transformHmacSha512GetKlass = xmlSecMSCngTransformHmacSha512GetKlass; #endif /* XMLSEC_NO_SHA512 */ -#ifdef XMLSEC_MSCNG_TODO - gXmlSecMSCngFunctions->transformHmacRipemd160GetKlass = xmlSecMSCngTransformHmacRipemd160GetKlass; -#endif /* XMLSEC_NO_RIPEMD160 */ - -#ifdef XMLSEC_MSCNG_TODO - gXmlSecMSCngFunctions->transformHmacMd5GetKlass = xmlSecMSCngTransformHmacMd5GetKlass; -#endif /* XMLSEC_NO_MD5 */ - #endif /* XMLSEC_NO_HMAC */ - /******************************* RIPEMD160 ********************************/ -#ifdef XMLSEC_MSCNG_TODO - gXmlSecMSCngFunctions->transformRipemd160GetKlass = xmlSecMSCngTransformRipemd160GetKlass; -#endif /* XMLSEC_NO_RIPEMD160 */ - /******************************* RSA ********************************/ #ifndef XMLSEC_NO_RSA diff --git a/src/mscng/kt_rsa.c b/src/mscng/kt_rsa.c index 764ab1442..6a0f27037 100644 --- a/src/mscng/kt_rsa.c +++ b/src/mscng/kt_rsa.c @@ -468,6 +468,7 @@ xmlSecMSCngRsaOaepNodeRead(xmlSecTransformPtr transform, xmlNodePtr node, ctx = xmlSecMSCngRsaPkcs1OaepGetCtx(transform); xmlSecAssert2(ctx != NULL, -1); + xmlSecAssert2(xmlSecBufferGetSize(&(ctx->oaepParams)) == 0, -1); ret = xmlSecTransformRsaOaepReadParams(node, &(ctx->oaepParams), &algorithm); if (ret < 0) { diff --git a/src/mscng/x509.c b/src/mscng/x509.c index a0513e767..791a9aac1 100755 --- a/src/mscng/x509.c +++ b/src/mscng/x509.c @@ -76,6 +76,8 @@ static int xmlSecMSCngKeyDataX509Duplicate(xmlSecKeyDataPtr dst, xmlSecKeyDataPtr src) { PCCERT_CONTEXT srcCert = NULL; PCCERT_CONTEXT dstCert; + PCCRL_CONTEXT srcCrl = NULL; + PCCRL_CONTEXT dstCrl; xmlSecMSCngX509DataCtxPtr srcCtx; int ret; @@ -83,12 +85,15 @@ xmlSecMSCngKeyDataX509Duplicate(xmlSecKeyDataPtr dst, xmlSecKeyDataPtr src) { xmlSecAssert2(xmlSecKeyDataCheckId(src, xmlSecMSCngKeyDataX509Id), -1); srcCtx = xmlSecMSCngX509DataGetCtx(src); - /* duplicate the certificate store */ + /* duplicate the certificate store; CertEnumCertificatesInStore frees the + * previous certificate context (see + * https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-certenumcertificatesinstore) */ while((srcCert = CertEnumCertificatesInStore(srcCtx->hMemStore, srcCert)) != NULL) { dstCert = CertDuplicateCertificateContext(srcCert); if(dstCert == NULL) { xmlSecMSCngLastError("CertDuplicateCertificateContext", xmlSecKeyDataGetName(dst)); + CertFreeCertificateContext(srcCert); return(-1); } @@ -96,11 +101,34 @@ xmlSecMSCngKeyDataX509Duplicate(xmlSecKeyDataPtr dst, xmlSecKeyDataPtr src) { if(ret < 0) { xmlSecInternalError("xmlSecMSCngKeyDataX509AdoptCert", xmlSecKeyDataGetName(dst)); + CertFreeCertificateContext(srcCert); CertFreeCertificateContext(dstCert); return(-1); } } + /* duplicate the CRLs; CertEnumCRLsInStore frees the previous CRL context + * (see + * https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-certenumcrlsinstore) */ + while((srcCrl = CertEnumCRLsInStore(srcCtx->hMemStore, srcCrl)) != NULL) { + dstCrl = CertDuplicateCRLContext(srcCrl); + if(dstCrl == NULL) { + xmlSecMSCngLastError("CertDuplicateCRLContext", + xmlSecKeyDataGetName(dst)); + CertFreeCRLContext(srcCrl); + return(-1); + } + + ret = xmlSecMSCngKeyDataX509AdoptCrl(dst, dstCrl); + if(ret < 0) { + xmlSecInternalError("xmlSecMSCngKeyDataX509AdoptCrl", + xmlSecKeyDataGetName(dst)); + CertFreeCRLContext(srcCrl); + CertFreeCRLContext(dstCrl); + return(-1); + } + } + if(srcCtx->cert != NULL) { /* have a key certificate, duplicate that */ dstCert = CertDuplicateCertificateContext(srcCtx->cert); @@ -254,6 +282,8 @@ xmlSecMSCngKeyDataX509AdoptCrl(xmlSecKeyDataPtr data, PCCRL_CONTEXT crl) { return(-1); } + /* this just decrements the refcount, so won't free */ + CertFreeCRLContext(crl); return(0); } diff --git a/src/mscng/x509vfy.c b/src/mscng/x509vfy.c index eddc3caca..3e4176faa 100755 --- a/src/mscng/x509vfy.c +++ b/src/mscng/x509vfy.c @@ -668,32 +668,37 @@ xmlSecMSCngX509StoreContainsCert(HCERTSTORE store, CERT_NAME_BLOB* name, xmlSecAssert2(name != NULL, -1); xmlSecAssert2(cert != NULL, -1); - storeCert = CertFindCertificateInStore(store, - X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, - 0, - CERT_FIND_SUBJECT_NAME, - name, - NULL); - if (storeCert == NULL) { - return (0); - } + /* enumerate all certificates with the same subject: a valid second cert + * with the same subject (e.g. CA key rotation) must not be rejected just + * because the first matching cert does not verify; + * CertFindCertificateInStore frees the previous certificate context (see + * https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-certfindcertificateinstore) */ + while (TRUE) { + storeCert = CertFindCertificateInStore(store, + X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, + 0, + CERT_FIND_SUBJECT_NAME, + name, + storeCert); + if (storeCert == NULL) { + return (0); + } - ret = xmlSecMSCngX509StoreVerifySubject(cert, storeCert); - if (ret < 0) { - xmlSecInternalError("xmlSecMSCngX509StoreVerifySubject", NULL); - CertFreeCertificateContext(storeCert); - return(-1); - } else if (ret == 0) { - xmlSecOtherError(XMLSEC_ERRORS_R_CERT_VERIFY_FAILED, - NULL, - "xmlSecMSCngX509StoreVerifySubject"); + ret = xmlSecMSCngX509StoreVerifySubject(cert, storeCert); + if (ret < 0) { + xmlSecInternalError("xmlSecMSCngX509StoreVerifySubject", NULL); + continue; + } else if (ret == 0) { + xmlSecOtherError(XMLSEC_ERRORS_R_CERT_VERIFY_FAILED, + NULL, + "xmlSecMSCngX509StoreVerifySubject"); + continue; + } + + /* success */ CertFreeCertificateContext(storeCert); - return(-1); + return(1); } - - /* success */ - CertFreeCertificateContext(storeCert); - return(1); } static int diff --git a/src/mscrypto/kt_rsa.c b/src/mscrypto/kt_rsa.c index fba153979..8385aa525 100644 --- a/src/mscrypto/kt_rsa.c +++ b/src/mscrypto/kt_rsa.c @@ -239,6 +239,7 @@ xmlSecMSCryptoRsaPkcs1OaepProcess(xmlSecTransformPtr transform) { xmlSecSize keySize; int ret; HCRYPTKEY hKey = 0; + HCRYPTKEY hTmpKey = 0; DWORD dwInLen; DWORD dwBufLen; DWORD dwOutLen; @@ -313,11 +314,11 @@ xmlSecMSCryptoRsaPkcs1OaepProcess(xmlSecTransformPtr transform) { outBuf = xmlSecBufferGetData(out); xmlSecAssert2(outBuf != NULL, -1); - /* set OAEP parameter for the key - * - * aleksey: I don't understand how this would work in multi-threaded - * environment or when key can be re-used multiple times - */ + /* Set the OAEP parameter on a temporary duplicate key, when available, + * so the original key is not modified. The raw OAEP label is passed to + * KP_OAEP_PARAMS (a CAPI/wincrypt parameter that expects the label L, + * not its hash H(L); the provider computes H(L) internally). + * https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-cryptsetkeyparam */ if(xmlSecTransformCheckId(transform, xmlSecMSCryptoTransformRsaOaepId) && xmlSecBufferGetSize(&(ctx->oaepParams)) > 0) { xmlSecSize oaepParamsSize; CRYPT_DATA_BLOB oaepParams; @@ -327,17 +328,33 @@ xmlSecMSCryptoRsaPkcs1OaepProcess(xmlSecTransformPtr transform) { oaepParamsSize = xmlSecBufferGetSize(&(ctx->oaepParams)); XMLSEC_SAFE_CAST_SIZE_TO_ULONG(oaepParamsSize, oaepParams.cbData, return(-1), xmlSecTransformGetName(transform)); - if (!CryptSetKeyParam(hKey, KP_OAEP_PARAMS, (const BYTE*)&oaepParams, 0)) { + if (!CryptDuplicateKey(hKey, NULL, 0, &hTmpKey)) { + xmlSecMSCryptoError("CryptDuplicateKey", xmlSecTransformGetName(transform)); + return (-1); + } + if (!CryptSetKeyParam(hTmpKey, KP_OAEP_PARAMS, (const BYTE*)&oaepParams, 0)) { xmlSecMSCryptoError("CryptSetKeyParam", xmlSecTransformGetName(transform)); + if (hTmpKey != 0) { + CryptDestroyKey(hTmpKey); + hTmpKey = 0; + } return (-1); } } /* encrypt */ - if (!CryptEncrypt(hKey, 0, TRUE, ctx->dwFlags, outBuf, &dwInLen, dwBufLen)) { + if (!CryptEncrypt(((hTmpKey != 0) ? hTmpKey : hKey), 0, TRUE, ctx->dwFlags, outBuf, &dwInLen, dwBufLen)) { xmlSecMSCryptoError("CryptEncrypt", xmlSecTransformGetName(transform)); + if (hTmpKey != 0) { + CryptDestroyKey(hTmpKey); + hTmpKey = 0; + } return (-1); } + if (hTmpKey != 0) { + CryptDestroyKey(hTmpKey); + hTmpKey = 0; + } /* The output of CryptEncrypt is in little-endian format, so we have to convert to * big-endian first. @@ -359,11 +376,11 @@ xmlSecMSCryptoRsaPkcs1OaepProcess(xmlSecTransformPtr transform) { return (-1); } - /* set OAEP parameter for the key - * - * aleksey: I don't understand how this would work in multi-threaded - * environment or when key can be re-used multiple times - */ + /* Set the OAEP parameter on a temporary duplicate key, when available, + * so the original key is not modified. The raw OAEP label is passed to + * KP_OAEP_PARAMS (a CAPI/wincrypt parameter that expects the label L, + * not its hash H(L); the provider computes H(L) internally). + * https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-cryptsetkeyparam */ if(xmlSecTransformCheckId(transform, xmlSecMSCryptoTransformRsaOaepId) && xmlSecBufferGetSize(&(ctx->oaepParams)) > 0) { xmlSecSize oaepParamsSize; CRYPT_DATA_BLOB oaepParams; @@ -373,20 +390,36 @@ xmlSecMSCryptoRsaPkcs1OaepProcess(xmlSecTransformPtr transform) { oaepParamsSize = xmlSecBufferGetSize(&(ctx->oaepParams)); XMLSEC_SAFE_CAST_SIZE_TO_ULONG(oaepParamsSize, oaepParams.cbData, { CryptDestroyKey(hKey); return(-1); }, xmlSecTransformGetName(transform)); - if (!CryptSetKeyParam(hKey, KP_OAEP_PARAMS, (const BYTE*)&oaepParams, 0)) { + if (!CryptDuplicateKey(hKey, NULL, 0, &hTmpKey)) { + xmlSecMSCryptoError("CryptDuplicateKey", xmlSecTransformGetName(transform)); + CryptDestroyKey(hKey); + return (-1); + } + if (!CryptSetKeyParam(hTmpKey, KP_OAEP_PARAMS, (const BYTE*)&oaepParams, 0)) { xmlSecMSCryptoError("CryptSetKeyParam", xmlSecTransformGetName(transform)); + if (hTmpKey != 0) { + CryptDestroyKey(hTmpKey); + hTmpKey = 0; + } CryptDestroyKey(hKey); return (-1); } } /* decrypt */ - if (!CryptDecrypt(hKey, 0, TRUE, ctx->dwFlags, outBuf, &dwOutLen)) { + if (!CryptDecrypt(((hTmpKey != 0) ? hTmpKey : hKey), 0, TRUE, ctx->dwFlags, outBuf, &dwOutLen)) { xmlSecMSCryptoError("CryptDecrypt", xmlSecTransformGetName(transform)); + if (hTmpKey != 0) { + CryptDestroyKey(hTmpKey); + hTmpKey = 0; + } CryptDestroyKey(hKey); return(-1); } - + if (hTmpKey != 0) { + CryptDestroyKey(hTmpKey); + hTmpKey = 0; + } /* hKey comes from CryptGetUserKey and must be destroyed by the caller */ CryptDestroyKey(hKey); hKey = 0; diff --git a/src/mscrypto/xmlsec-mingw.h b/src/mscrypto/xmlsec-mingw.h index e53503483..279709e38 100644 --- a/src/mscrypto/xmlsec-mingw.h +++ b/src/mscrypto/xmlsec-mingw.h @@ -149,7 +149,7 @@ /*structures/types*/ -typedef struct _PUBKEY { +typedef struct _DSSPUBKEY { DWORD magic; DWORD bitlen; } DSSPUBKEY;