Bug#913136: security update for xml-security-c in Stretch

Ferenc Wágner wagner.ferenc at kifu.gov.hu
Mon Dec 10 11:23:04 GMT 2018


wferi at 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

-------------- next part --------------
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 <wferi at 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 <scantor at 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 <scantor at 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


More information about the Pkg-shibboleth-devel mailing list