From a6ec5ba5d5a9ce4f3e127fdc455eea31051390c2 Mon Sep 17 00:00:00 2001 From: Scott Cantor Date: Fri, 17 Apr 2009 16:03:10 +0000 Subject: [PATCH] Revert to string-based return values, add linefeed option. --- xmltooling/security/SecurityHelper.h | 19 +++-- xmltooling/security/impl/CredentialCriteria.cpp | 2 +- .../security/impl/FilesystemCredentialResolver.cpp | 2 +- xmltooling/security/impl/SecurityHelper.cpp | 98 ++++++++++------------ xmltoolingtest/SecurityHelperTest.h | 27 +++--- 5 files changed, 69 insertions(+), 79 deletions(-) diff --git a/xmltooling/security/SecurityHelper.h b/xmltooling/security/SecurityHelper.h index 70ccc5d..e09e139 100644 --- a/xmltooling/security/SecurityHelper.h +++ b/xmltooling/security/SecurityHelper.h @@ -125,31 +125,34 @@ namespace xmltooling { * @param key2 second key to compare * @return true iff the keys match */ - static bool matches(const XSECCryptoKey* key1, const XSECCryptoKey* key2); + static bool matches(const XSECCryptoKey& key1, const XSECCryptoKey& key2); /** * Returns the base64-encoded DER encoding of a public key in SubjectPublicKeyInfo format. * - * @param key the credential containing the key to encode - * @return the base64 encoded key value in a malloc'd string + * @param key the credential containing the key to encode + * @param nowrap if true, any linefeeds will be stripped from the result + * @return the base64 encoded key value */ - static char* getDEREncoding(const Credential& cred); + static std::string getDEREncoding(const Credential& cred, bool nowrap=true); /** * Returns the base64-encoded DER encoding of a public key in SubjectPublicKeyInfo format. * * @param key the key to encode - * @return the base64 encoded key value in a malloc'd string + * @param nowrap if true, any linefeeds will be stripped from the result + * @return the base64 encoded key value */ - static char* getDEREncoding(const XSECCryptoKey& key); + static std::string getDEREncoding(const XSECCryptoKey& key, bool nowrap=true); /** * Returns the base64-encoded DER encoding of a certifiate's public key in SubjectPublicKeyInfo format. * * @param cert the certificate's key to encode - * @return the base64 encoded key value in a malloc'd string + * @param nowrap if true, any linefeeds will be stripped from the result + * @return the base64 encoded key value */ - static char* getDEREncoding(const XSECCryptoX509& cert); + static std::string getDEREncoding(const XSECCryptoX509& cert, bool nowrap=true); }; }; diff --git a/xmltooling/security/impl/CredentialCriteria.cpp b/xmltooling/security/impl/CredentialCriteria.cpp index 9842df3..6187771 100644 --- a/xmltooling/security/impl/CredentialCriteria.cpp +++ b/xmltooling/security/impl/CredentialCriteria.cpp @@ -81,5 +81,5 @@ bool CredentialCriteria::matches(const Credential& credential) const if (!key2) return true; // no key here, so we can't test it - return SecurityHelper::matches(key1, key2); + return SecurityHelper::matches(*key1, *key2); } diff --git a/xmltooling/security/impl/FilesystemCredentialResolver.cpp b/xmltooling/security/impl/FilesystemCredentialResolver.cpp index d995180..2ae0edd 100644 --- a/xmltooling/security/impl/FilesystemCredentialResolver.cpp +++ b/xmltooling/security/impl/FilesystemCredentialResolver.cpp @@ -558,7 +558,7 @@ Credential* FilesystemCredentialResolver::getCredential() // First, verify that the key and certificate match. if (m_key.key && !m_certs.empty()) { auto_ptr temp(m_certs.front().certs.front()->clonePublicKey()); - if (!SecurityHelper::matches(m_key.key, temp.get())) + if (!SecurityHelper::matches(*m_key.key, *temp.get())) throw XMLSecurityException("FilesystemCredentialResolver given mismatched key/certificate, check for consistency."); } diff --git a/xmltooling/security/impl/SecurityHelper.cpp b/xmltooling/security/impl/SecurityHelper.cpp index 4e8b5ab..6370079 100644 --- a/xmltooling/security/impl/SecurityHelper.cpp +++ b/xmltooling/security/impl/SecurityHelper.cpp @@ -436,57 +436,57 @@ vector::size_type SecurityHelper::loadCRLsFromURL( return loadCRLsFromFile(crls, backing, format); } -bool SecurityHelper::matches(const XSECCryptoKey* key1, const XSECCryptoKey* key2) +bool SecurityHelper::matches(const XSECCryptoKey& key1, const XSECCryptoKey& key2) { - if (key1->getProviderName()!=DSIGConstants::s_unicodeStrPROVOpenSSL || - key2->getProviderName()!=DSIGConstants::s_unicodeStrPROVOpenSSL) { + if (key1.getProviderName()!=DSIGConstants::s_unicodeStrPROVOpenSSL || + key2.getProviderName()!=DSIGConstants::s_unicodeStrPROVOpenSSL) { Category::getInstance(XMLTOOLING_LOGCAT".SecurityHelper").warn("comparison of non-OpenSSL keys not supported"); return false; } // If one key is public or both, just compare the public key half. - if (key1->getKeyType()==XSECCryptoKey::KEY_RSA_PUBLIC || key1->getKeyType()==XSECCryptoKey::KEY_RSA_PAIR) { - if (key2->getKeyType()!=XSECCryptoKey::KEY_RSA_PUBLIC && key2->getKeyType()!=XSECCryptoKey::KEY_RSA_PAIR) + if (key1.getKeyType()==XSECCryptoKey::KEY_RSA_PUBLIC || key1.getKeyType()==XSECCryptoKey::KEY_RSA_PAIR) { + if (key2.getKeyType()!=XSECCryptoKey::KEY_RSA_PUBLIC && key2.getKeyType()!=XSECCryptoKey::KEY_RSA_PAIR) return false; - const RSA* rsa1 = static_cast(key1)->getOpenSSLRSA(); - const RSA* rsa2 = static_cast(key2)->getOpenSSLRSA(); - return (BN_cmp(rsa1->n,rsa2->n) == 0 && BN_cmp(rsa1->e,rsa2->e) == 0); + const RSA* rsa1 = static_cast(key1).getOpenSSLRSA(); + const RSA* rsa2 = static_cast(key2).getOpenSSLRSA(); + return (rsa1 && rsa2 && BN_cmp(rsa1->n,rsa2->n) == 0 && BN_cmp(rsa1->e,rsa2->e) == 0); } // For a private key, compare the private half. - if (key1->getKeyType()==XSECCryptoKey::KEY_RSA_PRIVATE) { - if (key2->getKeyType()!=XSECCryptoKey::KEY_RSA_PRIVATE && key2->getKeyType()!=XSECCryptoKey::KEY_RSA_PAIR) + if (key1.getKeyType()==XSECCryptoKey::KEY_RSA_PRIVATE) { + if (key2.getKeyType()!=XSECCryptoKey::KEY_RSA_PRIVATE && key2.getKeyType()!=XSECCryptoKey::KEY_RSA_PAIR) return false; - const RSA* rsa1 = static_cast(key1)->getOpenSSLRSA(); - const RSA* rsa2 = static_cast(key2)->getOpenSSLRSA(); - return (BN_cmp(rsa1->n,rsa2->n) == 0 && BN_cmp(rsa1->d,rsa2->d) == 0); + const RSA* rsa1 = static_cast(key1).getOpenSSLRSA(); + const RSA* rsa2 = static_cast(key2).getOpenSSLRSA(); + return (rsa1 && rsa2 && BN_cmp(rsa1->n,rsa2->n) == 0 && BN_cmp(rsa1->d,rsa2->d) == 0); } // If one key is public or both, just compare the public key half. - if (key1->getKeyType()==XSECCryptoKey::KEY_DSA_PUBLIC || key1->getKeyType()==XSECCryptoKey::KEY_DSA_PAIR) { - if (key2->getKeyType()!=XSECCryptoKey::KEY_DSA_PUBLIC && key2->getKeyType()!=XSECCryptoKey::KEY_DSA_PAIR) + if (key1.getKeyType()==XSECCryptoKey::KEY_DSA_PUBLIC || key1.getKeyType()==XSECCryptoKey::KEY_DSA_PAIR) { + if (key2.getKeyType()!=XSECCryptoKey::KEY_DSA_PUBLIC && key2.getKeyType()!=XSECCryptoKey::KEY_DSA_PAIR) return false; - const DSA* dsa1 = static_cast(key1)->getOpenSSLDSA(); - const DSA* dsa2 = static_cast(key2)->getOpenSSLDSA(); - return (BN_cmp(dsa1->pub_key,dsa2->pub_key) == 0); + const DSA* dsa1 = static_cast(key1).getOpenSSLDSA(); + const DSA* dsa2 = static_cast(key2).getOpenSSLDSA(); + return (dsa1 && dsa2 && BN_cmp(dsa1->pub_key,dsa2->pub_key) == 0); } // For a private key, compare the private half. - if (key1->getKeyType()==XSECCryptoKey::KEY_DSA_PRIVATE) { - if (key2->getKeyType()!=XSECCryptoKey::KEY_DSA_PRIVATE && key2->getKeyType()!=XSECCryptoKey::KEY_DSA_PAIR) + if (key1.getKeyType()==XSECCryptoKey::KEY_DSA_PRIVATE) { + if (key2.getKeyType()!=XSECCryptoKey::KEY_DSA_PRIVATE && key2.getKeyType()!=XSECCryptoKey::KEY_DSA_PAIR) return false; - const DSA* dsa1 = static_cast(key1)->getOpenSSLDSA(); - const DSA* dsa2 = static_cast(key2)->getOpenSSLDSA(); - return (BN_cmp(dsa1->priv_key,dsa2->priv_key) == 0); + const DSA* dsa1 = static_cast(key1).getOpenSSLDSA(); + const DSA* dsa2 = static_cast(key2).getOpenSSLDSA(); + return (dsa1 && dsa2 && BN_cmp(dsa1->priv_key,dsa2->priv_key) == 0); } Category::getInstance(XMLTOOLING_LOGCAT".SecurityHelper").warn("unsupported key type for comparison"); return false; } -char* SecurityHelper::getDEREncoding(const XSECCryptoKey& key) +string SecurityHelper::getDEREncoding(const XSECCryptoKey& key, bool nowrap) { - char* ret=NULL; + string ret; if (key.getProviderName()!=DSIGConstants::s_unicodeStrPROVOpenSSL) { Category::getInstance(XMLTOOLING_LOGCAT".SecurityHelper").warn("encoding of non-OpenSSL keys not supported"); @@ -500,20 +500,16 @@ char* SecurityHelper::getDEREncoding(const XSECCryptoKey& key) return ret; } BIO* base64 = BIO_new(BIO_f_base64()); - BIO_set_flags(base64, BIO_FLAGS_BASE64_NO_NL); + if (nowrap) + BIO_set_flags(base64, BIO_FLAGS_BASE64_NO_NL); BIO* mem = BIO_new(BIO_s_mem()); BIO_push(base64, mem); i2d_RSA_PUBKEY_bio(base64, const_cast(rsa)); BIO_flush(base64); BUF_MEM* bptr=NULL; BIO_get_mem_ptr(base64, &bptr); - if (bptr && bptr->length > 0) { - ret = (char*)malloc(sizeof(char)*(bptr->length+1)); - if (ret) { - strncpy(ret, bptr->data, bptr->length); - ret[bptr->length]=0; - } - } + if (bptr && bptr->length > 0) + ret.append(bptr->data, bptr->length); BIO_free_all(base64); } else if (key.getKeyType() == XSECCryptoKey::KEY_DSA_PUBLIC || key.getKeyType() == XSECCryptoKey::KEY_DSA_PAIR) { @@ -523,20 +519,16 @@ char* SecurityHelper::getDEREncoding(const XSECCryptoKey& key) return ret; } BIO* base64 = BIO_new(BIO_f_base64()); - BIO_set_flags(base64, BIO_FLAGS_BASE64_NO_NL); + if (nowrap) + BIO_set_flags(base64, BIO_FLAGS_BASE64_NO_NL); BIO* mem = BIO_new(BIO_s_mem()); BIO_push(base64, mem); i2d_DSA_PUBKEY_bio(base64, const_cast(dsa)); BIO_flush(base64); BUF_MEM* bptr=NULL; BIO_get_mem_ptr(base64, &bptr); - if (bptr && bptr->length > 0) { - ret = (char*)malloc(sizeof(char)*(bptr->length+1)); - if (ret) { - strncpy(ret, bptr->data, bptr->length); - ret[bptr->length]=0; - } - } + if (bptr && bptr->length > 0) + ret.append(bptr->data, bptr->length); BIO_free_all(base64); } else { @@ -545,9 +537,9 @@ char* SecurityHelper::getDEREncoding(const XSECCryptoKey& key) return ret; } -char* SecurityHelper::getDEREncoding(const XSECCryptoX509& cert) +string SecurityHelper::getDEREncoding(const XSECCryptoX509& cert, bool nowrap) { - char* ret=NULL; + string ret; if (cert.getProviderName()!=DSIGConstants::s_unicodeStrPROVOpenSSL) { Category::getInstance(XMLTOOLING_LOGCAT".SecurityHelper").warn("encoding of non-OpenSSL keys not supported"); @@ -558,7 +550,8 @@ char* SecurityHelper::getDEREncoding(const XSECCryptoX509& cert) EVP_PKEY* key = X509_get_pubkey(const_cast(x)); BIO* base64 = BIO_new(BIO_f_base64()); - BIO_set_flags(base64, BIO_FLAGS_BASE64_NO_NL); + if (nowrap) + BIO_set_flags(base64, BIO_FLAGS_BASE64_NO_NL); BIO* mem = BIO_new(BIO_s_mem()); BIO_push(base64, mem); i2d_PUBKEY_bio(base64, key); @@ -566,23 +559,18 @@ char* SecurityHelper::getDEREncoding(const XSECCryptoX509& cert) BIO_flush(base64); BUF_MEM* bptr=NULL; BIO_get_mem_ptr(base64, &bptr); - if (bptr && bptr->length > 0) { - ret = (char*)malloc(sizeof(char)*(bptr->length+1)); - if (ret) { - strncpy(ret, bptr->data, bptr->length); - ret[bptr->length]=0; - } - } + if (bptr && bptr->length > 0) + ret.append(bptr->data, bptr->length); BIO_free_all(base64); return ret; } -char* SecurityHelper::getDEREncoding(const Credential& cred) +string SecurityHelper::getDEREncoding(const Credential& cred, bool nowrap) { const X509Credential* x509 = dynamic_cast(&cred); if (x509 && !x509->getEntityCertificateChain().empty()) - return getDEREncoding(*(x509->getEntityCertificateChain().front())); + return getDEREncoding(*(x509->getEntityCertificateChain().front()), nowrap); else if (cred.getPublicKey()) - return getDEREncoding(*(cred.getPublicKey())); - return NULL; + return getDEREncoding(*(cred.getPublicKey()), nowrap); + return ""; } diff --git a/xmltoolingtest/SecurityHelperTest.h b/xmltoolingtest/SecurityHelperTest.h index 01d1b4e..70e451b 100644 --- a/xmltoolingtest/SecurityHelperTest.h +++ b/xmltoolingtest/SecurityHelperTest.h @@ -42,12 +42,12 @@ public: pathname = data_path + "test.pfx"; auto_ptr key3(SecurityHelper::loadKeyFromFile(pathname.c_str(), NULL, "password")); - TSM_ASSERT("PEM/DER keys did not match", SecurityHelper::matches(key1.get(), key2.get())); - TSM_ASSERT("DER/PKCS12 keys did not match", SecurityHelper::matches(key2.get(), key3.get())); + TSM_ASSERT("PEM/DER keys did not match", SecurityHelper::matches(*key1.get(), *key2.get())); + TSM_ASSERT("DER/PKCS12 keys did not match", SecurityHelper::matches(*key2.get(), *key3.get())); pathname = data_path + "key2.pem"; auto_ptr key4(SecurityHelper::loadKeyFromFile(pathname.c_str())); - TSM_ASSERT("Different keys matched", !SecurityHelper::matches(key3.get(), key4.get())); + TSM_ASSERT("Different keys matched", !SecurityHelper::matches(*key3.get(), *key4.get())); } void testKeysFromURLs() { @@ -61,8 +61,8 @@ public: auto_ptr t3(getTransport("https://spaces.internet2.edu/download/attachments/5305/test.pfx")); auto_ptr key3(SecurityHelper::loadKeyFromURL(*t3.get(), pathname.c_str(), NULL, "password")); - TSM_ASSERT("PEM/DER keys did not match", SecurityHelper::matches(key1.get(), key2.get())); - TSM_ASSERT("DER/PKCS12 keys did not match", SecurityHelper::matches(key2.get(), key3.get())); + TSM_ASSERT("PEM/DER keys did not match", SecurityHelper::matches(*key1.get(), *key2.get())); + TSM_ASSERT("DER/PKCS12 keys did not match", SecurityHelper::matches(*key2.get(), *key3.get())); } void testCertificatesFromFiles() { @@ -79,14 +79,13 @@ public: auto_ptr key2(certs[1]->clonePublicKey()); auto_ptr key3(certs[2]->clonePublicKey()); - TSM_ASSERT("PEM/DER keys did not match", SecurityHelper::matches(key1.get(), key2.get())); - TSM_ASSERT("DER/PKCS12 keys did not match", SecurityHelper::matches(key2.get(), key3.get())); + TSM_ASSERT("PEM/DER keys did not match", SecurityHelper::matches(*key1.get(), *key2.get())); + TSM_ASSERT("DER/PKCS12 keys did not match", SecurityHelper::matches(*key2.get(), *key3.get())); - char* enc1 = SecurityHelper::getDEREncoding(*certs[2]); - char* enc2 = SecurityHelper::getDEREncoding(*key1.get()); - TSM_ASSERT("Certificate and its key produced different DER encodings", !strcmp(enc1, enc2)); - if (enc1) free(enc1); - if (enc2) free(enc2); + TSM_ASSERT_EQUALS( + "Certificate and its key produced different DER encodings", + SecurityHelper::getDEREncoding(*certs[2]), SecurityHelper::getDEREncoding(*key1.get()) + ); for_each(certs.begin(), certs.end(), xmltooling::cleanup()); certs.clear(); @@ -109,8 +108,8 @@ public: auto_ptr key2(certs[0]->clonePublicKey()); auto_ptr key3(certs[0]->clonePublicKey()); - TSM_ASSERT("PEM/DER keys did not match", SecurityHelper::matches(key1.get(), key2.get())); - TSM_ASSERT("DER/PKCS12 keys did not match", SecurityHelper::matches(key2.get(), key3.get())); + TSM_ASSERT("PEM/DER keys did not match", SecurityHelper::matches(*key1.get(), *key2.get())); + TSM_ASSERT("DER/PKCS12 keys did not match", SecurityHelper::matches(*key2.get(), *key3.get())); for_each(certs.begin(), certs.end(), xmltooling::cleanup()); certs.clear(); -- 2.1.4