Fixed several bugs in CAPI/Schannel code on Windows

This patch includes the following fixes: 1) Correctly hex encode SHA1 hashes when generating certstore: URIs 2) Use the newly parsed certificate store reference, not the old value 3) Need to call findCertificateInStore() when finding the selected TLS certificate in Schannel code. Without that "sha1:XXXX" URIs don't work Also minor optimization of string operations. License: This patch is BSD-licensed, see Documentation/Licenses/BSD-simplified.txt for details.
author: Alexey Melnikov <alexey.melnikov@isode.com> 2012-03-07 19:16:54 (GMT)
committer: Kevin Smith <git@kismith.co.uk> 2012-03-08 15:18:38 (GMT)
commit: 0833f7da453db9cd0fc3a78c793e7532663ab86b (patch)
tree: 0f0dab24760aa0ad5a77a3f5a4b3a25af3420a2a
parent: 2ba2c34e59c4ec5da9254b6c6f4669e0ee3c3225 (diff)
download: swift-contrib-0833f7da453db9cd0fc3a78c793e7532663ab86b.zip
swift-contrib-0833f7da453db9cd0fc3a78c793e7532663ab86b.tar.bz2
4 files changed, 11 insertions, 14 deletions
diff --git a/Swift/QtUI/CAPICertificateSelector.cpp b/Swift/QtUI/CAPICertificateSelector.cpp
index e7948ef..0d4768c 100644
--- a/Swift/QtUI/CAPICertificateSelector.cpp
+++ b/Swift/QtUI/CAPICertificateSelector.cpp
@@ -1,77 +1,80 @@
 /*
  * Copyright (c) 2012 Isode Limited, London, England.
  * Licensed under the simplified BSD license.
  * See Documentation/Licenses/BSD-simplified.txt for more information.
  */
 
 #include <string>
 
 #include "CAPICertificateSelector.h"
 
 #define SECURITY_WIN32
 #include <Windows.h>
 #include <WinCrypt.h>
 #include <cryptuiapi.h>
 
 #include <Swiften/StringCodecs/Hexify.h>
 #include <boost/algorithm/string.hpp>
 
 namespace Swift {
 
 #define cert_dlg_title L"TLS Client Certificate Selection"
 #define cert_dlg_prompt L"Select a certificate to use for authentication"
 /////Hmm, maybe we should not exlude the "location" column
 #define exclude_columns	 CRYPTUI_SELECT_LOCATION_COLUMN \
 			|CRYPTUI_SELECT_INTENDEDUSE_COLUMN
 
 // Size of the SHA1 hash
 #define SHA1_HASH_LEN                20
 
 static std::string getCertUri(PCCERT_CONTEXT cert, const char * cert_store_name) {
 	DWORD cbHash = SHA1_HASH_LEN;
 	BYTE aHash[SHA1_HASH_LEN];
-	std::string ret = std::string("certstore:") + cert_store_name + ":" + "sha1:";
+	std::string ret("certstore:");
+
+	ret += cert_store_name;
+	ret += ":sha1:";
 
 	if (CertGetCertificateContextProperty(cert,
 		 CERT_HASH_PROP_ID,
 		 aHash,
 		 &cbHash) == FALSE ) {
 		return "";
 	}
 
-	ByteArray byteArray = createByteArray((char *)(&aHash[0]));
+	ByteArray byteArray = createByteArray((char *)(&aHash[0]), cbHash);
 	ret += Hexify::hexify(byteArray);
 
 	return ret;
 }
 
 std::string selectCAPICertificate() {
 
 	const char * cert_store_name = "MY";
 	PCCERT_CONTEXT cert;
 	DWORD store_flags;
 	HCERTSTORE hstore;
 	HWND hwnd;
 
 	store_flags = CERT_STORE_OPEN_EXISTING_FLAG |
 		      CERT_STORE_READONLY_FLAG |
 		      CERT_SYSTEM_STORE_CURRENT_USER;
 
 	hstore = CertOpenStore(CERT_STORE_PROV_SYSTEM_A, 0, 0, store_flags, cert_store_name);
 	if (!hstore) {
 		return "";
 	}
 
 
 ////Does this handle need to be freed as well?
 	hwnd = GetForegroundWindow();
 	if (!hwnd) {
 		hwnd = GetActiveWindow();
 	}
 
 	/* Call Windows dialog to select a suitable certificate */
 	cert = CryptUIDlgSelectCertificateFromStore(hstore,
 						  hwnd,
 						  cert_dlg_title,
 						  cert_dlg_prompt,
 						  exclude_columns,
diff --git a/Swiften/TLS/CAPICertificate.cpp b/Swiften/TLS/CAPICertificate.cpp
index a6725c9..0dc3009 100644
--- a/Swiften/TLS/CAPICertificate.cpp
+++ b/Swiften/TLS/CAPICertificate.cpp
@@ -6,71 +6,71 @@
 #pragma once
 
 #include <Swiften/TLS/CAPICertificate.h>
 #include <Swiften/StringCodecs/Hexify.h>
 
 #include <boost/algorithm/string/predicate.hpp>
 
 // Size of the SHA1 hash
 #define SHA1_HASH_LEN                20
 
 
 namespace Swift {
 CAPICertificate::CAPICertificate(const std::string& capiUri)
 			  		 : valid_(false), uri_(capiUri), certStoreHandle_(0), certStore_(), certName_() {
 	setUri(capiUri);
 }
 
 CAPICertificate::~CAPICertificate() {
 	if (certStoreHandle_) {
 		CertCloseStore(certStoreHandle_, 0);
 	}
 }
 
 bool CAPICertificate::isNull() const {
 	return uri_.empty() || !valid_;
 }
 
 const std::string& CAPICertificate::getCertStoreName() const {
 	return certStore_;
 }
 
 const std::string& CAPICertificate::getCertName() const {
 	return certName_;
 }
 
-static PCCERT_CONTEXT findCertificateInStore (HCERTSTORE certStoreHandle, const std::string &certName) {
+PCCERT_CONTEXT findCertificateInStore (HCERTSTORE certStoreHandle, const std::string &certName) {
 	PCCERT_CONTEXT pCertContext = NULL;
 
 	if (!boost::iequals(certName.substr(0, 5), "sha1:")) {
 
 		// Find client certificate. Note that this sample just searches for a
 		// certificate that contains the user name somewhere in the subject name.
 		pCertContext = CertFindCertificateInStore(certStoreHandle,
 				X509_ASN_ENCODING,
 				0,				// dwFindFlags
 				CERT_FIND_SUBJECT_STR_A,
 				certName.c_str(),		// *pvFindPara
 				NULL );				// pPrevCertContext
 		return pCertContext;
 	}
 
 
 	std::string hexstring = certName.substr(5);
 	ByteArray byteArray = Hexify::unhexify(hexstring);
 	CRYPT_HASH_BLOB HashBlob;
 
 	if (byteArray.size() != SHA1_HASH_LEN) {
 		return NULL;
 	}
 	HashBlob.cbData = SHA1_HASH_LEN;
 	HashBlob.pbData = static_cast<BYTE *>(vecptr(byteArray));
 
 	// Find client certificate. Note that this sample just searches for a
 	// certificate that contains the user name somewhere in the subject name.
 	pCertContext = CertFindCertificateInStore(certStoreHandle,
 			X509_ASN_ENCODING,
 			0,				// dwFindFlags
 			CERT_FIND_HASH,
 			&HashBlob,
 			NULL );				// pPrevCertContext
 
@@ -81,71 +81,71 @@ static PCCERT_CONTEXT findCertificateInStore (HCERTSTORE certStoreHandle, const
 void CAPICertificate::setUri (const std::string& capiUri) {
 
 	valid_ = false;
 
 	/* Syntax: "certstore:" <cert_store> ":" <hash> ":" <hash_of_cert> */
 
 	if (!boost::iequals(capiUri.substr(0, 10), "certstore:")) {
 		return;
 	}
 
 	/* Substring of subject: uses "storename" */
 	std::string capi_identity = capiUri.substr(10);
 	std::string new_certStore_name;
 	size_t pos = capi_identity.find_first_of (':');
 
 	if (pos == std::string::npos) {
 		/* Using the default certificate store */
 		new_certStore_name = "MY";
 		certName_ = capi_identity;
 	}
 	else {
 		new_certStore_name = capi_identity.substr(0, pos);
 		certName_ = capi_identity.substr(pos + 1);
 	}
 
 	PCCERT_CONTEXT pCertContext = NULL;
 
 	if (certStoreHandle_ != NULL) {
 		if (new_certStore_name != certStore_) {
 			CertCloseStore(certStoreHandle_, 0);
 			certStoreHandle_ = NULL;
 		}
 	}
 
 	if (certStoreHandle_ == NULL) {
-		certStoreHandle_ = CertOpenSystemStore(0, certStore_.c_str());
+		certStoreHandle_ = CertOpenSystemStore(0, new_certStore_name.c_str());
 		if (!certStoreHandle_) {
 			return;
 		}
 	}
 
 	certStore_ = new_certStore_name;
 
 	pCertContext = findCertificateInStore (certStoreHandle_, certName_);
 
 	if (!pCertContext) {
 		return;
 	}
 
 
 	/* Now verify that we can have access to the corresponding private key */
 
 	DWORD len;
 	CRYPT_KEY_PROV_INFO *pinfo;
 	HCRYPTPROV hprov;
 	HCRYPTKEY key;
 
 	if (!CertGetCertificateContextProperty(pCertContext,
 			CERT_KEY_PROV_INFO_PROP_ID,
 			NULL,
 			&len)) {
 		CertFreeCertificateContext(pCertContext);
 		return;
 	}
 
 	pinfo = static_cast<CRYPT_KEY_PROV_INFO *>(malloc(len));
 	if (!pinfo) {
 		CertFreeCertificateContext(pCertContext);
 		return;
 	}
 
diff --git a/Swiften/TLS/CAPICertificate.h b/Swiften/TLS/CAPICertificate.h
index d9e2704..4204a6b 100644
--- a/Swiften/TLS/CAPICertificate.h
+++ b/Swiften/TLS/CAPICertificate.h
@@ -7,36 +7,39 @@
 #pragma once
 
 #include <Swiften/Base/SafeByteArray.h>
 #include <Swiften/TLS/CertificateWithKey.h>
 
 #define SECURITY_WIN32
 #include <Windows.h>
 #include <WinCrypt.h>
 
 namespace Swift {
 	class CAPICertificate : public Swift::CertificateWithKey {
 		public:
 			CAPICertificate(const std::string& capiUri);
 
 			virtual ~CAPICertificate();
 
 			virtual bool isNull() const;
 
 			const std::string& getCertStoreName() const;
 
 			const std::string& getCertName() const;
 
 		private:
 			void setUri (const std::string& capiUri);
 
 		private:
 			bool valid_;
 			std::string uri_;
 
 			HCERTSTORE certStoreHandle_;
 
 			/* Parsed components of the uri_ */
 			std::string certStore_;
 			std::string certName_;
 	};
+
+PCCERT_CONTEXT findCertificateInStore (HCERTSTORE certStoreHandle, const std::string &certName);
+
 }
diff --git a/Swiften/TLS/Schannel/SchannelContext.cpp b/Swiften/TLS/Schannel/SchannelContext.cpp
index ddbebcb..b2fea65 100644
--- a/Swiften/TLS/Schannel/SchannelContext.cpp
+++ b/Swiften/TLS/Schannel/SchannelContext.cpp
@@ -37,96 +37,87 @@ SchannelContext::SchannelContext()
 SchannelContext::~SchannelContext()
 {
 	if (m_my_cert_store) CertCloseStore(m_my_cert_store, 0);
 }
 
 //------------------------------------------------------------------------
 
 void SchannelContext::determineStreamSizes()
 {
 	QueryContextAttributes(m_ctxtHandle, SECPKG_ATTR_STREAM_SIZES, &m_streamSizes);
 }
 
 //------------------------------------------------------------------------
 
 void SchannelContext::connect() 
 {
 	PCCERT_CONTEXT   pCertContext = NULL;
 
 	m_state = Connecting;
 
 	// If a user name is specified, then attempt to find a client
 	// certificate. Otherwise, just create a NULL credential.
 	if (!m_cert_name.empty())
 	{
 		if (m_my_cert_store == NULL)
 		{
 			m_my_cert_store = CertOpenSystemStore(0, m_cert_store_name.c_str());
 			if (!m_my_cert_store)
 			{
 /////			printf( "**** Error 0x%x returned by CertOpenSystemStore\n", GetLastError() );
 				indicateError();
 				return;
 			}
 		}
 
-		// Find client certificate. Note that this sample just searches for a 
-		// certificate that contains the user name somewhere in the subject name.
-		pCertContext = CertFindCertificateInStore( m_my_cert_store,
-			X509_ASN_ENCODING,
-			0,				// dwFindFlags
-			CERT_FIND_SUBJECT_STR_A,
-			m_cert_name.c_str(),		// *pvFindPara
-			NULL );				// pPrevCertContext
-
+		pCertContext = findCertificateInStore( m_my_cert_store, m_cert_name );
 		if (pCertContext == NULL)
 		{
 /////		printf("**** Error 0x%x returned by CertFindCertificateInStore\n", GetLastError());
 			indicateError();
 			return;
 		}
 	}
 
 	// We use an empty list for client certificates
 	PCCERT_CONTEXT clientCerts[1] = {0};
 
 	SCHANNEL_CRED sc = {0};
 	sc.dwVersion = SCHANNEL_CRED_VERSION;
 
 /////SSL3?
 	sc.grbitEnabledProtocols = SP_PROT_SSL3_CLIENT | SP_PROT_TLS1_CLIENT | SP_PROT_TLS1_1_CLIENT | SP_PROT_TLS1_2_CLIENT;
-/////Check SCH_CRED_REVOCATION_CHECK_CHAIN
 	sc.dwFlags = SCH_CRED_AUTO_CRED_VALIDATION | SCH_CRED_REVOCATION_CHECK_CHAIN;
 
 	if (pCertContext)
 	{
 		sc.cCreds = 1;
 		sc.paCred = &pCertContext;
 		sc.dwFlags |= SCH_CRED_NO_DEFAULT_CREDS;
 	}
 	else
 	{
 		sc.cCreds = 0; // Let Crypto API find the appropriate certificate for us
 		sc.paCred = clientCerts;
 		sc.dwFlags |= SCH_CRED_USE_DEFAULT_CREDS;
 	}
 
 	// Swiften performs the server name check for us
 	sc.dwFlags |= SCH_CRED_NO_SERVERNAME_CHECK;
 
 	SECURITY_STATUS status = AcquireCredentialsHandle(
 		NULL,
 		UNISP_NAME,
 		SECPKG_CRED_OUTBOUND,
 		NULL,
 		&sc,
 		NULL,
 		NULL,
 		m_credHandle.Reset(),
 		NULL);
 
 	// cleanup: Free the certificate context. Schannel has already made its own copy.
 	if (pCertContext) CertFreeCertificateContext(pCertContext);
 
 	if (status != SEC_E_OK) 
 	{
 		// We failed to obtain the credentials handle
author	Alexey Melnikov <alexey.melnikov@isode.com>	2012-03-07 19:16:54 (GMT)
committer	Kevin Smith <git@kismith.co.uk>	2012-03-08 15:18:38 (GMT)
commit	0833f7da453db9cd0fc3a78c793e7532663ab86b (patch)
tree	0f0dab24760aa0ad5a77a3f5a4b3a25af3420a2a
parent	2ba2c34e59c4ec5da9254b6c6f4669e0ee3c3225 (diff)
download	swift-contrib-0833f7da453db9cd0fc3a78c793e7532663ab86b.zip swift-contrib-0833f7da453db9cd0fc3a78c793e7532663ab86b.tar.bz2