Santuario patch coming shortly
Ferenc Wágner
wferi at niif.hu
Thu Aug 2 14:51:19 BST 2018
"Cantor, Scott" <cantor.2 at osu.edu> writes:
> I have a 2.0.1 coming to fix a DOS bug that impacts the SP, I've
> already committed the fix and am testing now.
Hi Scott,
Thanks for the heads-up. I squashed and backported your patches,
dropping some disconnected whitespace and (void)->() prototype changes
(as far as I know, they are equivalent in C++) for easier review by the
Debian Security Team. Does this look sane to you?
Author: Scott Cantor <scantor at apache.org>
Date: Wed Aug 1 13:02:21 2018 +0000
SANTUARIO-491 - Default KeyInfo resolver doesn't check for empty element content.
[...]
diff --git a/xsec/dsig/DSIGKeyInfoValue.hpp b/xsec/dsig/DSIGKeyInfoValue.hpp
index f652679b..d6da5d56 100644
--- a/xsec/dsig/DSIGKeyInfoValue.hpp
+++ b/xsec/dsig/DSIGKeyInfoValue.hpp
@@ -117,7 +117,7 @@ public:
* @returns a pointer to the DSA P string value.
*/
- const XMLCh * getDSAP(void) const {return mp_PTextNode->getNodeValue();}
+ const XMLCh * getDSAP() const {return mp_PTextNode ? mp_PTextNode->getNodeValue() : NULL;}
/**
* \brief Get Q value
@@ -125,7 +125,7 @@ public:
* @returns a pointer to the DSA Q string value.
*/
- const XMLCh * getDSAQ(void) const {return mp_QTextNode->getNodeValue();}
+ const XMLCh * getDSAQ() const {return mp_QTextNode ? mp_QTextNode->getNodeValue() : NULL;}
/**
* \brief Get G value
@@ -133,7 +133,7 @@ public:
* @returns a pointer to the DSA G string value.
*/
- const XMLCh * getDSAG(void) const {return mp_GTextNode->getNodeValue();}
+ const XMLCh * getDSAG() const {return mp_GTextNode ? mp_GTextNode->getNodeValue() : NULL;}
/**
* \brief Get Y value
@@ -141,7 +141,7 @@ public:
* @returns a pointer to the DSA Y string value.
*/
- const XMLCh * getDSAY(void) const {return mp_YTextNode->getNodeValue();}
+ const XMLCh * getDSAY() const {return mp_YTextNode ? mp_YTextNode->getNodeValue() : NULL;}
/**
* \brief Get Modulus
diff --git a/xsec/enc/XSECKeyInfoResolverDefault.cpp b/xsec/enc/XSECKeyInfoResolverDefault.cpp
index 52cdfe35..c4c81cb2 100644
--- a/xsec/enc/XSECKeyInfoResolverDefault.cpp
+++ b/xsec/enc/XSECKeyInfoResolverDefault.cpp
@@ -79,13 +79,11 @@ XSECCryptoKey * XSECKeyInfoResolverDefault::resolveKey(DSIGKeyInfoList * lst) {
case (DSIGKeyInfo::KEYINFO_X509) :
{
ret = NULL;
- const XMLCh * x509Str;
- XSECCryptoX509 * x509 = XSECPlatformUtils::g_cryptoProvider->X509();
- Janitor<XSECCryptoX509> j_x509(x509);
-
- x509Str = ((DSIGKeyInfoX509 *) lst->item(i))->getCertificateItem(0);
+ const XMLCh* x509Str = ((const DSIGKeyInfoX509 *) lst->item(i))->getCertificateItem(0);
- if (x509Str != 0) {
+ if (x509Str) {
+ XSECCryptoX509 * x509 = XSECPlatformUtils::g_cryptoProvider->X509();
+ Janitor<XSECCryptoX509> j_x509(x509);
// The crypto interface classes work UTF-8
safeBuffer transX509;
@@ -104,66 +102,90 @@ XSECCryptoKey * XSECKeyInfoResolverDefault::resolveKey(DSIGKeyInfoList * lst) {
case (DSIGKeyInfo::KEYINFO_VALUE_DSA) :
{
- XSECCryptoKeyDSA * dsa = XSECPlatformUtils::g_cryptoProvider->keyDSA();
- Janitor<XSECCryptoKeyDSA> j_dsa(dsa);
-
- safeBuffer value;
-
- value << (*mp_formatter << ((DSIGKeyInfoValue *) lst->item(i))->getDSAP());
- dsa->loadPBase64BigNums(value.rawCharBuffer(), (unsigned int) strlen(value.rawCharBuffer()));
- value << (*mp_formatter << ((DSIGKeyInfoValue *) lst->item(i))->getDSAQ());
- dsa->loadQBase64BigNums(value.rawCharBuffer(), (unsigned int) strlen(value.rawCharBuffer()));
- value << (*mp_formatter << ((DSIGKeyInfoValue *) lst->item(i))->getDSAG());
- dsa->loadGBase64BigNums(value.rawCharBuffer(), (unsigned int) strlen(value.rawCharBuffer()));
- value << (*mp_formatter << ((DSIGKeyInfoValue *) lst->item(i))->getDSAY());
- dsa->loadYBase64BigNums(value.rawCharBuffer(), (unsigned int) strlen(value.rawCharBuffer()));
-
- j_dsa.release();
- return dsa;
+ const DSIGKeyInfoValue* dsaval = (const DSIGKeyInfoValue *) lst->item(i);
+ if (dsaval->getDSAP() || dsaval->getDSAQ() || dsaval->getDSAG() || dsaval->getDSAY()) {
+
+ XSECCryptoKeyDSA * dsa = XSECPlatformUtils::g_cryptoProvider->keyDSA();
+ Janitor<XSECCryptoKeyDSA> j_dsa(dsa);
+
+ safeBuffer value;
+
+ if (dsaval->getDSAP()) {
+ value << (*mp_formatter << dsaval->getDSAP());
+ dsa->loadPBase64BigNums(value.rawCharBuffer(), (unsigned int) strlen(value.rawCharBuffer()));
+ }
+ if (dsaval->getDSAQ()) {
+ value << (*mp_formatter << dsaval->getDSAQ());
+ dsa->loadQBase64BigNums(value.rawCharBuffer(), (unsigned int) strlen(value.rawCharBuffer()));
+ }
+ if (dsaval->getDSAG()) {
+ value << (*mp_formatter << dsaval->getDSAG());
+ dsa->loadGBase64BigNums(value.rawCharBuffer(), (unsigned int) strlen(value.rawCharBuffer()));
+ }
+ if (dsaval->getDSAY()) {
+ value << (*mp_formatter << dsaval->getDSAY());
+ dsa->loadYBase64BigNums(value.rawCharBuffer(), (unsigned int) strlen(value.rawCharBuffer()));
+ }
+
+ j_dsa.release();
+ return dsa;
+ }
}
break;
case (DSIGKeyInfo::KEYINFO_VALUE_RSA) :
{
+ const DSIGKeyInfoValue* rsaval = (const DSIGKeyInfoValue *) lst->item(i);
+ if (rsaval->getRSAModulus() && rsaval->getRSAExponent()) {
- XSECCryptoKeyRSA * rsa = XSECPlatformUtils::g_cryptoProvider->keyRSA();
- Janitor<XSECCryptoKeyRSA> j_rsa(rsa);
+ XSECCryptoKeyRSA* rsa = XSECPlatformUtils::g_cryptoProvider->keyRSA();
+ Janitor<XSECCryptoKeyRSA> j_rsa(rsa);
- safeBuffer value;
+ safeBuffer value;
- value << (*mp_formatter << ((DSIGKeyInfoValue *) lst->item(i))->getRSAModulus());
- rsa->loadPublicModulusBase64BigNums(value.rawCharBuffer(), (unsigned int) strlen(value.rawCharBuffer()));
- value << (*mp_formatter << ((DSIGKeyInfoValue *) lst->item(i))->getRSAExponent());
- rsa->loadPublicExponentBase64BigNums(value.rawCharBuffer(), (unsigned int) strlen(value.rawCharBuffer()));
+ value << (*mp_formatter << rsaval->getRSAModulus());
+ rsa->loadPublicModulusBase64BigNums(value.rawCharBuffer(), (unsigned int) strlen(value.rawCharBuffer()));
+ value << (*mp_formatter << rsaval->getRSAExponent());
+ rsa->loadPublicExponentBase64BigNums(value.rawCharBuffer(), (unsigned int) strlen(value.rawCharBuffer()));
- j_rsa.release();
- return rsa;
+ j_rsa.release();
+ return rsa;
+ }
}
break;
case (DSIGKeyInfo::KEYINFO_VALUE_EC) :
{
+ const DSIGKeyInfoValue* ecval = (const DSIGKeyInfoValue *) lst->item(i);
+ if (ecval->getECPublicKey() && ecval->getECNamedCurve()) {
- XSECCryptoKeyEC* ec = XSECPlatformUtils::g_cryptoProvider->keyEC();
- Janitor<XSECCryptoKeyEC> j_ec(ec);
+ XSECCryptoKeyEC* ec = XSECPlatformUtils::g_cryptoProvider->keyEC();
+ Janitor<XSECCryptoKeyEC> j_ec(ec);
- safeBuffer value;
- value << (*mp_formatter << ((DSIGKeyInfoValue *) lst->item(i))->getECPublicKey());
- XSECAutoPtrChar curve(((DSIGKeyInfoValue *) lst->item(i))->getECNamedCurve());
- if (curve.get()) {
- ec->loadPublicKeyBase64(curve.get(), value.rawCharBuffer(), (unsigned int) strlen(value.rawCharBuffer()));
- j_ec.release();
- return ec;
+ safeBuffer value;
+
+ value << (*mp_formatter << ecval->getECPublicKey());
+ XSECAutoPtrChar curve(ecval->getECNamedCurve());
+ if (curve.get()) {
+ ec->loadPublicKeyBase64(curve.get(), value.rawCharBuffer(), (unsigned int) strlen(value.rawCharBuffer()));
+ j_ec.release();
+ return ec;
+ }
}
}
break;
case (DSIGKeyInfo::KEYINFO_DERENCODED) :
{
- safeBuffer value;
- value << (*mp_formatter << ((DSIGKeyInfoDEREncoded *) lst->item(i))->getData());
- return XSECPlatformUtils::g_cryptoProvider->keyDER(value.rawCharBuffer(), (unsigned int)strlen(value.rawCharBuffer()), true);
+ const DSIGKeyInfoDEREncoded* derval = (const DSIGKeyInfoDEREncoded *) lst->item(i);
+ if (derval->getData()) {
+
+ safeBuffer value;
+
+ value << (*mp_formatter << derval->getData());
+ return XSECPlatformUtils::g_cryptoProvider->keyDER(value.rawCharBuffer(), (unsigned int)strlen(value.rawCharBuffer()), true);
+ }
}
break;
There are changes in a header file as well, so some of the SP stack will
need to be recompiled to include the fix, I guess. Or all of it, maybe?
> Fair warning since I expect to get grief from all quarters, but I am
> *not* going to handle this as a security issue, CVE, etc. on the
> Apache side, I flat out do not have time for that and people will
> either live with it or they'll get a response from me they will
> happily file with the rest of their grievances.
:)
> I will do an advisory for the SP itself since that is a) all I have
> time to worry about, and b) much simpler for me to do.
Are you okay with us getting a separate CVE for this Santuario issue?
Could you use the same or another one for the SP?
--
Thanks,
Feri
More information about the Pkg-shibboleth-devel
mailing list