wf...@niif.hu writes: > The backport of the patches is fairly straightforward indeed, but up > to now I couldn't reproduce the issue itself.
Hi Security Team, After a couple more rounds with upstream it turns out that while #905332 affected simple parsing of the structure, this one affects verification using an invalid structure. So one has to convince the code to actually attempt that verification, which does not seem easily possible with the (recommended) explicit key trust engine. However, deployments using the PKIX trust engine are still prone to remote DoS, though I can't easily test this. All in all, for security and symmetry I recommend fixing this issue in stretch the same way it was fixed in jessie by Thorsten. Please find the debdiff attached. -- Thanks, Feri
diff -Nru xml-security-c-1.7.3/debian/changelog xml-security-c-1.7.3/debian/changelog --- xml-security-c-1.7.3/debian/changelog 2018-08-03 11:32:52.000000000 +0200 +++ xml-security-c-1.7.3/debian/changelog 2018-12-10 11:45:41.000000000 +0100 @@ -1,3 +1,20 @@ +xml-security-c (1.7.3-4+deb9u2) stretch-security; urgency=high + + * [12dd825] New patches: DSA verification crashes OpenSSL on invalid + combinations of key content. + Particular KeyInfo combinations result in incomplete DSA key structures + that OpenSSL can't handle without crashing. In the case of Shibboleth + SP software this manifests as a crash in the shibd daemon. Exploitation + is believed to be possible only in deployments employing the PKIX trust + engine, which is generally recommended against. + The upstream patches backported from 2.0.2 apply analogous safeguards to + the RSA and ECDSA key handling as well. + Upstream bug: https://issues.apache.org/jira/browse/SANTUARIO-496 + CVE: not assigned + Thanks to Scott Cantor (Closes: #913136) + + -- Ferenc Wágner <wf...@debian.org> Mon, 10 Dec 2018 11:45:41 +0100 + xml-security-c (1.7.3-4+deb9u1) stretch-security; urgency=high * [93b87c6] New patch: Default KeyInfo resolver doesn't check for empty diff -Nru xml-security-c-1.7.3/debian/patches/SANTUARIO-496-DSA-verification-crashes-OpenSSL-on-invalid.patch xml-security-c-1.7.3/debian/patches/SANTUARIO-496-DSA-verification-crashes-OpenSSL-on-invalid.patch --- xml-security-c-1.7.3/debian/patches/SANTUARIO-496-DSA-verification-crashes-OpenSSL-on-invalid.patch 1970-01-01 01:00:00.000000000 +0100 +++ xml-security-c-1.7.3/debian/patches/SANTUARIO-496-DSA-verification-crashes-OpenSSL-on-invalid.patch 2018-12-10 11:44:59.000000000 +0100 @@ -0,0 +1,103 @@ +From: Scott Cantor <scan...@apache.org> +Date: Thu, 11 Oct 2018 15:13:40 +0000 +Subject: SANTUARIO-496 - DSA verification crashes OpenSSL on invalid + combinations of key content + +Backport of +git-svn-id: https://svn.apache.org/repos/asf/santuario/xml-security-cpp/trunk@1843562 13f79535-47bb-0310-9956-ffa450edef68 +--- + xsec/enc/OpenSSL/OpenSSLCryptoKeyDSA.cpp | 12 ++++++++++++ + xsec/enc/OpenSSL/OpenSSLCryptoKeyEC.cpp | 12 ++++++++++++ + xsec/enc/OpenSSL/OpenSSLCryptoKeyRSA.cpp | 12 ++++++++++++ + 3 files changed, 36 insertions(+) + +diff --git a/xsec/enc/OpenSSL/OpenSSLCryptoKeyDSA.cpp b/xsec/enc/OpenSSL/OpenSSLCryptoKeyDSA.cpp +index 57999a2..5bdf133 100644 +--- a/xsec/enc/OpenSSL/OpenSSLCryptoKeyDSA.cpp ++++ b/xsec/enc/OpenSSL/OpenSSLCryptoKeyDSA.cpp +@@ -164,6 +164,12 @@ bool OpenSSLCryptoKeyDSA::verifyBase64Signature(unsigned char * hashBuf, + "OpenSSL:DSA - Attempt to validate signature with empty key"); + } + ++ XSECCryptoKey::KeyType keyType = getKeyType(); ++ if (keyType != KEY_DSA_PAIR && keyType != KEY_DSA_PUBLIC) { ++ throw XSECCryptoException(XSECCryptoException::DSAError, ++ "OpenSSL:DSA - Attempt to validate signature without public key"); ++ } ++ + char* cleanedBase64Signature; + unsigned int cleanedBase64SignatureLen = 0; + +@@ -264,6 +270,12 @@ unsigned int OpenSSLCryptoKeyDSA::signBase64Signature(unsigned char * hashBuf, + "OpenSSL:DSA - Attempt to sign data with empty key"); + } + ++ KeyType keyType = getKeyType(); ++ if (keyType != KEY_DSA_PAIR && keyType != KEY_DSA_PRIVATE) { ++ throw XSECCryptoException(XSECCryptoException::DSAError, ++ "OpenSSL:DSA - Attempt to sign data without private key"); ++ } ++ + DSA_SIG * dsa_sig; + + dsa_sig = DSA_do_sign(hashBuf, hashLen, mp_dsaKey); +diff --git a/xsec/enc/OpenSSL/OpenSSLCryptoKeyEC.cpp b/xsec/enc/OpenSSL/OpenSSLCryptoKeyEC.cpp +index 3233343..09ba69e 100644 +--- a/xsec/enc/OpenSSL/OpenSSLCryptoKeyEC.cpp ++++ b/xsec/enc/OpenSSL/OpenSSLCryptoKeyEC.cpp +@@ -151,6 +151,12 @@ bool OpenSSLCryptoKeyEC::verifyBase64SignatureDSA(unsigned char * hashBuf, + "OpenSSL:EC - Attempt to validate signature with empty key"); + } + ++ KeyType keyType = getKeyType(); ++ if (keyType != KEY_EC_PAIR && keyType != KEY_EC_PUBLIC) { ++ throw XSECCryptoException(XSECCryptoException::ECError, ++ "OpenSSL:EC - Attempt to validate signature without public key"); ++ } ++ + char * cleanedBase64Signature; + unsigned int cleanedBase64SignatureLen = 0; + +@@ -225,6 +231,12 @@ unsigned int OpenSSLCryptoKeyEC::signBase64SignatureDSA(unsigned char * hashBuf, + "OpenSSL:EC - Attempt to sign data with empty key"); + } + ++ KeyType keyType = getKeyType(); ++ if (keyType != KEY_EC_PAIR && keyType != KEY_EC_PRIVATE) { ++ throw XSECCryptoException(XSECCryptoException::ECError, ++ "OpenSSL:EC - Attempt to sign data without private key"); ++ } ++ + ECDSA_SIG * dsa_sig; + + dsa_sig = ECDSA_do_sign(hashBuf, hashLen, mp_ecKey); +diff --git a/xsec/enc/OpenSSL/OpenSSLCryptoKeyRSA.cpp b/xsec/enc/OpenSSL/OpenSSLCryptoKeyRSA.cpp +index e21b001..c0d7b2b 100644 +--- a/xsec/enc/OpenSSL/OpenSSLCryptoKeyRSA.cpp ++++ b/xsec/enc/OpenSSL/OpenSSLCryptoKeyRSA.cpp +@@ -416,6 +416,12 @@ bool OpenSSLCryptoKeyRSA::verifySHA1PKCS1Base64Signature(const unsigned char * h + "OpenSSL:RSA - Attempt to validate signature with empty key"); + } + ++ XSECCryptoKey::KeyType keyType = getKeyType(); ++ if (keyType != KEY_RSA_PAIR && keyType != KEY_RSA_PUBLIC) { ++ throw XSECCryptoException(XSECCryptoException::RSAError, ++ "OpenSSL:RSA - Attempt to validate signature without public key"); ++ } ++ + char* cleanedBase64Signature; + unsigned int cleanedBase64SignatureLen = 0; + +@@ -534,6 +540,12 @@ unsigned int OpenSSLCryptoKeyRSA::signSHA1PKCS1Base64Signature(unsigned char * h + "OpenSSL:RSA - Attempt to sign data with empty key"); + } + ++ KeyType keyType = getKeyType(); ++ if (keyType != KEY_RSA_PAIR && keyType != KEY_RSA_PRIVATE) { ++ throw XSECCryptoException(XSECCryptoException::RSAError, ++ "OpenSSL:RSA - Attempt to sign data without private key"); ++ } ++ + // Build the buffer to be encrypted by prepending the SHA1 OID to the hash + + unsigned char * encryptBuf; diff -Nru xml-security-c-1.7.3/debian/patches/SANTUARIO-496-Prevent-KeyInfoResolver-returning-NONE-keys.patch xml-security-c-1.7.3/debian/patches/SANTUARIO-496-Prevent-KeyInfoResolver-returning-NONE-keys.patch --- xml-security-c-1.7.3/debian/patches/SANTUARIO-496-Prevent-KeyInfoResolver-returning-NONE-keys.patch 1970-01-01 01:00:00.000000000 +0100 +++ xml-security-c-1.7.3/debian/patches/SANTUARIO-496-Prevent-KeyInfoResolver-returning-NONE-keys.patch 2018-12-10 11:44:10.000000000 +0100 @@ -0,0 +1,65 @@ +From: Scott Cantor <scan...@apache.org> +Date: Thu, 11 Oct 2018 15:39:30 +0000 +Subject: SANTUARIO-496 - Prevent KeyInfoResolver returning NONE keys. + +git-svn-id: https://svn.apache.org/repos/asf/santuario/xml-security-cpp/trunk@1843566 13f79535-47bb-0310-9956-ffa450edef68 +--- + xsec/enc/XSECKeyInfoResolverDefault.cpp | 24 +++++++++++++++++------- + 1 file changed, 17 insertions(+), 7 deletions(-) + +diff --git a/xsec/enc/XSECKeyInfoResolverDefault.cpp b/xsec/enc/XSECKeyInfoResolverDefault.cpp +index c4c81cb..7356fc4 100644 +--- a/xsec/enc/XSECKeyInfoResolverDefault.cpp ++++ b/xsec/enc/XSECKeyInfoResolverDefault.cpp +@@ -127,8 +127,10 @@ XSECCryptoKey * XSECKeyInfoResolverDefault::resolveKey(DSIGKeyInfoList * lst) { + dsa->loadYBase64BigNums(value.rawCharBuffer(), (unsigned int) strlen(value.rawCharBuffer())); + } + +- j_dsa.release(); +- return dsa; ++ if (dsa->getKeyType() != XSECCryptoKey::KEY_NONE) { ++ j_dsa.release(); ++ return dsa; ++ } + } + } + break; +@@ -148,8 +150,10 @@ XSECCryptoKey * XSECKeyInfoResolverDefault::resolveKey(DSIGKeyInfoList * lst) { + value << (*mp_formatter << rsaval->getRSAExponent()); + rsa->loadPublicExponentBase64BigNums(value.rawCharBuffer(), (unsigned int) strlen(value.rawCharBuffer())); + +- j_rsa.release(); +- return rsa; ++ if (rsa->getKeyType() != XSECCryptoKey::KEY_NONE) { ++ j_rsa.release(); ++ return rsa; ++ } + } + + } +@@ -169,8 +173,10 @@ XSECCryptoKey * XSECKeyInfoResolverDefault::resolveKey(DSIGKeyInfoList * lst) { + XSECAutoPtrChar curve(ecval->getECNamedCurve()); + if (curve.get()) { + ec->loadPublicKeyBase64(curve.get(), value.rawCharBuffer(), (unsigned int) strlen(value.rawCharBuffer())); +- j_ec.release(); +- return ec; ++ if (ec->getKeyType() != XSECCryptoKey::KEY_NONE) { ++ j_ec.release(); ++ return ec; ++ } + } + } + } +@@ -184,7 +190,11 @@ XSECCryptoKey * XSECKeyInfoResolverDefault::resolveKey(DSIGKeyInfoList * lst) { + safeBuffer value; + + value << (*mp_formatter << derval->getData()); +- return XSECPlatformUtils::g_cryptoProvider->keyDER(value.rawCharBuffer(), (unsigned int)strlen(value.rawCharBuffer()), true); ++ XSECCryptoKey* key = XSECPlatformUtils::g_cryptoProvider->keyDER(value.rawCharBuffer(), (unsigned int)strlen(value.rawCharBuffer()), true); ++ if (key && key->getKeyType() != XSECCryptoKey::KEY_NONE) { ++ return key; ++ } ++ delete key; + } + } + break; diff -Nru xml-security-c-1.7.3/debian/patches/series xml-security-c-1.7.3/debian/patches/series --- xml-security-c-1.7.3/debian/patches/series 2018-08-03 11:32:52.000000000 +0200 +++ xml-security-c-1.7.3/debian/patches/series 2018-12-10 11:44:10.000000000 +0100 @@ -22,3 +22,5 @@ We-do-not-use-pthreads-threadtest.cpp-is-Windows-onl.patch Only-add-found-packages-to-the-pkg-config-dependenci.patch Default-KeyInfo-resolver-doesn-t-check-for-empty-element-.patch +SANTUARIO-496-DSA-verification-crashes-OpenSSL-on-invalid.patch +SANTUARIO-496-Prevent-KeyInfoResolver-returning-NONE-keys.patch
