Schannel: Use modern key management APIs
The key/certificate lifetime management in our Schannel backend is a little lacking. We haven't guaranteed that the original contexts are held alive for the full duration of their usage. Though with default settings they get persisted to disk so it has been mostly fine. One problem with that is that the legacy APIs in Windows for this is not smart enough to figure out that a repeatedly-loaded key is the same one, so it 'persists' a new file to disk every time we set up a credential context for a connection. For a busy server this may end up with creating a ton of small files that don't get deleted (or reused). By using the ncrypt APIs we don't fully stop persisting _all_ data to disk, but from testing we now only have one file per key. Regardless of the amount of connections. Another patch around lifetimes can be done for dev, and dev only, as it's quite a bit more extensive, and not fit for picking back to the LTS branches. Fixes: QTBUG-136055 Pick-to: 6.10 6.9 6.8 Change-Id: I61398a3773ef8c25aab21df3e78b71f3ab11d488 Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
This commit is contained in:
parent
a0c591c120
commit
94f0ff704e
@ -7,6 +7,7 @@
|
||||
#include "qtlskey_schannel_p.h"
|
||||
#include "qx509_schannel_p.h"
|
||||
#include "qtls_schannel_p.h"
|
||||
#include "../shared/qasn1element_p.h"
|
||||
|
||||
#include <QtNetwork/private/qsslcertificate_p.h>
|
||||
#include <QtNetwork/private/qsslcipher_p.h>
|
||||
@ -126,8 +127,9 @@ using namespace Qt::StringLiterals;
|
||||
Q_LOGGING_CATEGORY(lcTlsBackendSchannel, "qt.tlsbackend.schannel");
|
||||
|
||||
// Defined in qsslsocket_qt.cpp.
|
||||
QByteArray _q_makePkcs12(const QList<QSslCertificate> &certs, const QSslKey &key,
|
||||
extern QByteArray _q_makePkcs12(const QList<QSslCertificate> &certs, const QSslKey &key,
|
||||
const QString &passPhrase);
|
||||
extern QAsn1Element _q_PKCS12_key(const QSslKey &key);
|
||||
|
||||
namespace {
|
||||
bool supportsTls13();
|
||||
@ -1008,7 +1010,6 @@ TlsCryptographSchannel::~TlsCryptographSchannel()
|
||||
closeCertificateStores();
|
||||
deallocateContext();
|
||||
freeCredentialsHandle();
|
||||
CertFreeCertificateContext(localCertContext);
|
||||
}
|
||||
|
||||
void TlsCryptographSchannel::init(QSslSocket *qObj, QSslSocketPrivate *dObj)
|
||||
@ -1103,12 +1104,6 @@ bool TlsCryptographSchannel::acquireCredentialsHandle()
|
||||
return false;
|
||||
}
|
||||
|
||||
const CERT_CHAIN_CONTEXT *chainContext = nullptr;
|
||||
auto freeCertChain = qScopeGuard([&chainContext]() {
|
||||
if (chainContext)
|
||||
CertFreeCertificateChain(chainContext);
|
||||
});
|
||||
|
||||
DWORD certsCount = 0;
|
||||
// Set up our certificate stores before trying to use one...
|
||||
initializeCertificateStores();
|
||||
@ -1119,36 +1114,11 @@ bool TlsCryptographSchannel::acquireCredentialsHandle()
|
||||
if (!configuration.localCertificateChain().isEmpty() && !localCertificateStore)
|
||||
return true; // 'true' because "tst_QSslSocket::setEmptyKey" expects us to not disconnect
|
||||
|
||||
PCCERT_CONTEXT localCertificate = nullptr;
|
||||
if (localCertificateStore != nullptr) {
|
||||
CERT_CHAIN_FIND_BY_ISSUER_PARA findParam;
|
||||
ZeroMemory(&findParam, sizeof(findParam));
|
||||
findParam.cbSize = sizeof(findParam);
|
||||
findParam.pszUsageIdentifier = isClient ? szOID_PKIX_KP_CLIENT_AUTH : szOID_PKIX_KP_SERVER_AUTH;
|
||||
|
||||
// There should only be one chain in our store, so.. we grab that one.
|
||||
chainContext = CertFindChainInStore(localCertificateStore.get(),
|
||||
X509_ASN_ENCODING,
|
||||
0,
|
||||
CERT_CHAIN_FIND_BY_ISSUER,
|
||||
&findParam,
|
||||
nullptr);
|
||||
if (!chainContext) {
|
||||
const QString message = isClient
|
||||
? QSslSocket::tr("The certificate provided cannot be used for a client.")
|
||||
: QSslSocket::tr("The certificate provided cannot be used for a server.");
|
||||
setErrorAndEmit(d, QAbstractSocket::SocketError::SslInvalidUserDataError, message);
|
||||
return false;
|
||||
}
|
||||
Q_ASSERT(chainContext->cChain == 1);
|
||||
Q_ASSERT(chainContext->rgpChain[0]);
|
||||
Q_ASSERT(chainContext->rgpChain[0]->cbSize >= 1);
|
||||
Q_ASSERT(chainContext->rgpChain[0]->rgpElement[0]);
|
||||
Q_ASSERT(!localCertContext);
|
||||
localCertContext = CertDuplicateCertificateContext(chainContext->rgpChain[0]
|
||||
->rgpElement[0]
|
||||
->pCertContext);
|
||||
certsCount = 1;
|
||||
Q_ASSERT(localCertContext);
|
||||
localCertificate = static_cast<PCCERT_CONTEXT>(configuration.localCertificate().handle());
|
||||
Q_ASSERT(localCertificate);
|
||||
}
|
||||
|
||||
const QList<QSslCipher> ciphers = configuration.ciphers();
|
||||
@ -1172,7 +1142,7 @@ bool TlsCryptographSchannel::acquireCredentialsHandle()
|
||||
SCH_CREDENTIALS_VERSION,
|
||||
0,
|
||||
certsCount,
|
||||
&localCertContext,
|
||||
&localCertificate,
|
||||
nullptr,
|
||||
0,
|
||||
nullptr,
|
||||
@ -1729,9 +1699,6 @@ void TlsCryptographSchannel::reset()
|
||||
connectionInfo = {};
|
||||
streamSizes = {};
|
||||
|
||||
CertFreeCertificateContext(localCertContext);
|
||||
localCertContext = nullptr;
|
||||
|
||||
contextAttributes = 0;
|
||||
intermediateBuffer.clear();
|
||||
schannelState = SchannelState::InitializeHandshake;
|
||||
@ -2282,6 +2249,70 @@ bool TlsCryptographSchannel::checkSslErrors()
|
||||
return true;
|
||||
}
|
||||
|
||||
static void attachPrivateKeyToCertificate(const QSslCertificate &certificate,
|
||||
const QSslKey &privateKey)
|
||||
{
|
||||
QAsn1Element elem = _q_PKCS12_key(privateKey);
|
||||
QByteArray buffer;
|
||||
QDataStream stream(&buffer, QDataStream::WriteOnly);
|
||||
elem.write(stream);
|
||||
NCRYPT_PROV_HANDLE provider = 0;
|
||||
SECURITY_STATUS status = NCryptOpenStorageProvider(&provider, MS_KEY_STORAGE_PROVIDER, 0);
|
||||
if (status != SEC_E_OK) {
|
||||
qCWarning(lcTlsBackendSchannel())
|
||||
<< "Failed to open ncrypt storage provider:" << schannelErrorToString(status);
|
||||
return;
|
||||
}
|
||||
const auto freeProvider = qScopeGuard([provider]() { NCryptFreeObject(provider); });
|
||||
|
||||
const QString certName = certificate.subjectInfo(QSslCertificate::CommonName).front();
|
||||
QSpan<const QChar> nameSpan(certName);
|
||||
NCryptBuffer nbuffer{ ULONG(nameSpan.size_bytes() + sizeof(char16_t)),
|
||||
NCRYPTBUFFER_PKCS_KEY_NAME,
|
||||
const_reinterpret_cast<void *>(nameSpan.data()) };
|
||||
NCryptBufferDesc bufferDesc{ NCRYPTBUFFER_VERSION, 1, &nbuffer };
|
||||
NCRYPT_KEY_HANDLE ncryptKey = 0;
|
||||
status = NCryptImportKey(provider, 0, NCRYPT_PKCS8_PRIVATE_KEY_BLOB, &bufferDesc, &ncryptKey,
|
||||
PBYTE(buffer.data()), buffer.size(), 0);
|
||||
if (status != SEC_E_OK) {
|
||||
qCWarning(lcTlsBackendSchannel())
|
||||
<< "Failed to import private key:" << schannelErrorToString(status);
|
||||
return;
|
||||
}
|
||||
const auto freeKey = qScopeGuard([ncryptKey]() { NCryptFreeObject(ncryptKey); });
|
||||
|
||||
CERT_CONTEXT *context = PCERT_CONTEXT(certificate.handle());
|
||||
Q_ASSERT(context);
|
||||
|
||||
CRYPT_DATA_BLOB keyBlob = { sizeof(ncryptKey), PBYTE(&ncryptKey) };
|
||||
BOOL ok =
|
||||
CertSetCertificateContextProperty(context, CERT_NCRYPT_KEY_HANDLE_PROP_ID, 0, &keyBlob);
|
||||
if (!ok) {
|
||||
auto error = GetLastError();
|
||||
if (lcTlsBackendSchannel().isWarningEnabled())
|
||||
qErrnoWarning(int(error), "Failed to set ncrypt handle property.");
|
||||
return;
|
||||
}
|
||||
|
||||
CRYPT_KEY_PROV_INFO provInfo{
|
||||
const_reinterpret_cast<LPWSTR>(certName.constData()),
|
||||
const_cast<LPWSTR>(MS_KEY_STORAGE_PROVIDER),
|
||||
0,
|
||||
CERT_SET_KEY_PROV_HANDLE_PROP_ID | CERT_SET_KEY_CONTEXT_PROP_ID,
|
||||
0,
|
||||
nullptr,
|
||||
0,
|
||||
};
|
||||
ok = CertSetCertificateContextProperty(context, CERT_KEY_PROV_INFO_PROP_ID,
|
||||
CERT_SET_PROPERTY_INHIBIT_PERSIST_FLAG, &provInfo);
|
||||
if (!ok) {
|
||||
auto error = GetLastError();
|
||||
if (lcTlsBackendSchannel().isWarningEnabled())
|
||||
qErrnoWarning(int(error), "Failed to set key provider info property.");
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
void TlsCryptographSchannel::initializeCertificateStores()
|
||||
{
|
||||
//// helper function which turns a chain into a certificate store
|
||||
@ -2298,7 +2329,10 @@ void TlsCryptographSchannel::initializeCertificateStores()
|
||||
CRYPT_DATA_BLOB pfxBlob;
|
||||
pfxBlob.cbData = DWORD(pkcs12.length());
|
||||
pfxBlob.pbData = reinterpret_cast<unsigned char *>(pkcs12.data());
|
||||
return QHCertStorePointer(PFXImportCertStore(&pfxBlob, passphrase, 0));
|
||||
// ALWAYS_CNG to import using "Cryptography API: Next Generation (CNG)"
|
||||
// NO_PERSIST_KEY to request not persisting anything imported to disk
|
||||
constexpr DWORD flags = PKCS12_ALWAYS_CNG_KSP | PKCS12_NO_PERSIST_KEY;
|
||||
return QHCertStorePointer(PFXImportCertStore(&pfxBlob, passphrase, flags));
|
||||
};
|
||||
|
||||
if (!configuration.localCertificateChain().isEmpty()) {
|
||||
@ -2308,10 +2342,34 @@ void TlsCryptographSchannel::initializeCertificateStores()
|
||||
return;
|
||||
}
|
||||
if (localCertificateStore == nullptr) {
|
||||
localCertificateStore = createStoreFromCertificateChain(configuration.localCertificateChain(),
|
||||
configuration.privateKey());
|
||||
if (localCertificateStore == nullptr)
|
||||
localCertificateStore =
|
||||
createStoreFromCertificateChain(configuration.localCertificateChain(), {});
|
||||
if (localCertificateStore) {
|
||||
const CERT_CONTEXT *certificateContext = CertFindCertificateInStore(
|
||||
localCertificateStore.get(), X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, 0,
|
||||
CERT_FIND_ANY, nullptr, nullptr);
|
||||
if (certificateContext) {
|
||||
auto *backend = QTlsBackend::backend<X509CertificateSchannel>(
|
||||
configuration.localCertificate());
|
||||
backend->certificateContext.reset(
|
||||
CertDuplicateCertificateContext(certificateContext));
|
||||
|
||||
DWORD keySpec = 0;
|
||||
BOOL mustFree = FALSE;
|
||||
NCRYPT_KEY_HANDLE testKey = 0;
|
||||
BOOL ok = CryptAcquireCertificatePrivateKey(
|
||||
certificateContext, CRYPT_ACQUIRE_ONLY_NCRYPT_KEY_FLAG, nullptr,
|
||||
&testKey, &keySpec, &mustFree);
|
||||
if (mustFree)
|
||||
NCryptFreeObject(testKey);
|
||||
if (!ok) {
|
||||
attachPrivateKeyToCertificate(configuration.localCertificate(),
|
||||
configuration.privateKey());
|
||||
}
|
||||
}
|
||||
} else {
|
||||
qCWarning(lcTlsBackendSchannel, "Failed to load certificate chain!");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -114,8 +114,6 @@ private:
|
||||
QHCertStorePointer peerCertificateStore = nullptr;
|
||||
QHCertStorePointer caCertificateStore = nullptr;
|
||||
|
||||
const CERT_CONTEXT *localCertContext = nullptr;
|
||||
|
||||
ULONG contextAttributes = 0;
|
||||
qint64 missingData = 0;
|
||||
|
||||
|
@ -40,7 +40,7 @@ public:
|
||||
static bool importPkcs12(QIODevice *device, QSslKey *key, QSslCertificate *cert,
|
||||
QList<QSslCertificate> *caCertificates,
|
||||
const QByteArray &passPhrase);
|
||||
private:
|
||||
|
||||
QPCCertContextPointer certificateContext;
|
||||
|
||||
Q_DISABLE_COPY_MOVE(X509CertificateSchannel);
|
||||
|
@ -134,7 +134,7 @@ static QByteArray _q_PKCS12_certBag(const QSslCertificate &cert)
|
||||
return ba;
|
||||
}
|
||||
|
||||
static QAsn1Element _q_PKCS12_key(const QSslKey &key)
|
||||
QAsn1Element _q_PKCS12_key(const QSslKey &key)
|
||||
{
|
||||
Q_ASSERT(key.algorithm() == QSsl::Rsa || key.algorithm() == QSsl::Dsa);
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user