Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

(xmlsec-mscng, xmlsec-mscrypto, xmlsec-gnutls) Improve cert verification #819

Merged
merged 5 commits into from
Jul 18, 2024
Merged

(xmlsec-mscng, xmlsec-mscrypto, xmlsec-gnutls) Improve cert verification #819

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
af3656e
(xmlsec-mscng) Improve cert verification
lsh123 Jul 18, 2024
f448516
(xmlsec-mscrypto) Improve cert verification
lsh123 Jul 18, 2024
732cdc1
Added test, added docs, cleanup code a bit more
lsh123 Jul 18, 2024
ebe2f7e
Remove additional test for now
lsh123 Jul 18, 2024
cfab4d6
(xmlsec-mscng,xmlsec-mscrypto) Improved certificates verification.
lsh123 Jul 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/make-check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ on:
- xmlsec-1_2_x

jobs:
check-ubuntu-openssl300:
check-ubuntu:
runs-on: ubuntu-22.04
strategy:
fail-fast: false
Expand Down Expand Up @@ -48,7 +48,7 @@ jobs:
run: |
make install

check-ubuntu-openssl111:
check-ubuntu-openssl-111:
runs-on: ubuntu-20.04
strategy:
fail-fast: false
Expand Down
Empty file modified config.h.in
100755 → 100644
View file
Open in desktop
Empty file.
5 changes: 3 additions & 2 deletions docs/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,9 @@ <h1>XML Security Library</h1>
<br>
<br>
<ul>
<li>
(xmlsec-core) Fix deprecated functions in LibXML2 2.13.1 including disabling HTTP support
<li>(xmlsec-mscng,xmlsec-mscrypto) Improved certificates verification.</li>
<li>(xmlsec-gnutls) Added support for self-signed certificates.</li>
<li>(xmlsec-core) Fix deprecated functions in LibXML2 2.13.1 including disabling HTTP support
by default (use ''--enable-http' option to re-enable it).</li>
<li>Several other small fixes (see <a href="https://github.com/lsh123/xmlsec/commits/master">more details</a>).</li>
</ul>
Expand Down
1 change: 1 addition & 0 deletions src/gnutls/private.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ xmlSecPtrListPtr xmlSecGnuTLSKeyDataX509GetCrls (xmlSecKeyDataPt
*
************************************************************************/
gnutls_x509_crt_t xmlSecGnuTLSX509CertDup (gnutls_x509_crt_t src);
int xmlSecGnuTLSX509CertIsSelfSigned (gnutls_x509_crt_t cert);
xmlChar * xmlSecGnuTLSX509CertGetSubjectDN (gnutls_x509_crt_t cert);
xmlChar * xmlSecGnuTLSX509CertGetIssuerDN (gnutls_x509_crt_t cert);
xmlChar * xmlSecGnuTLSX509CertGetIssuerSerial (gnutls_x509_crt_t cert);
Expand Down
11 changes: 11 additions & 0 deletions src/gnutls/x509utils.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,17 @@ xmlSecGnuTLSX509CertDup(gnutls_x509_crt_t src) {
return (res);
}


/* returns 1 if self signed; 0 - if not; <0 on error*/
int
xmlSecGnuTLSX509CertIsSelfSigned(gnutls_x509_crt_t cert) {
unsigned ret;

xmlSecAssert2(cert != NULL, -1);
ret = gnutls_x509_crt_check_issuer(cert, cert);
return ((ret != 0) ? 1 : 0);
}

xmlChar *
xmlSecGnuTLSX509CertGetSubjectDN(gnutls_x509_crt_t cert) {
char* buf = NULL;
Expand Down
27 changes: 18 additions & 9 deletions src/gnutls/x509vfy.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -655,18 +655,27 @@ xmlSecGnuTLSX509StoreVerify(xmlSecKeyDataStorePtr store,
goto done;
}

/* check if we are the "leaf" node in the certs chain */
if(xmlSecGnuTLSX509FindSignedCert(certs, cert) != NULL) {
if(xmlSecGnuTLSX509CertIsSelfSigned(cert) != 1) {
/* check if we are the "leaf" node in the certs chain */
if(xmlSecGnuTLSX509FindSignedCert(certs, cert) != NULL) {
continue;
}

/* build the chain */
ret = xmlSecGnuTLSX509StoreGetCertsChain(ctx, cert, certs, certs_chain, certs_chain_size, &certs_chain_cur_size);
if(ret < 0) {
xmlSecInternalError("xmlSecPtrListGetItem(certs)", xmlSecKeyDataStoreGetName(store));
goto done;
}
} else if (certs_size == 1) {
/* only do self signed cert when it is the only cert */
/* chain for self signed cert is easy */
certs_chain[0] = cert;
certs_chain_cur_size = 1;
} else {
continue;
}

/* build the chain */
ret = xmlSecGnuTLSX509StoreGetCertsChain(ctx, cert, certs, certs_chain, certs_chain_size, &certs_chain_cur_size);
if(ret < 0) {
xmlSecInternalError("xmlSecPtrListGetItem(certs)", xmlSecKeyDataStoreGetName(store));
goto done;
}

/* try to verify */
ret = xmlSecGnuTLSX509StoreVerifyCert(ctx,
certs_chain, certs_chain_cur_size,
Expand Down
2 changes: 1 addition & 1 deletion src/mscng/certkeys.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ xmlSecMSCngKeyDataCertGetPubkey(PCCERT_CONTEXT cert, BCRYPT_KEY_HANDLE* key) {
xmlSecAssert2(key != NULL, -1);

if(!CryptImportPublicKeyInfoEx2(X509_ASN_ENCODING,
&cert->pCertInfo->SubjectPublicKeyInfo,
&(cert->pCertInfo->SubjectPublicKeyInfo),
0,
NULL,
key)) {
Expand Down
125 changes: 86 additions & 39 deletions src/mscng/x509vfy.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,42 @@ xmlSecMSCngCheckRevocation(HCERTSTORE store, PCCERT_CONTEXT cert) {
return(0);
}

/* this function does NOT check for time validity (see xmlSecMSCngVerifyCertTime)
* returns <0 if there is an error; 0 if verification failed and >0 if verification succeeded */
static int
xmlSecMSCngX509StoreVerifySubject(PCCERT_CONTEXT cert, PCCERT_CONTEXT issuerCert) {
DWORD flags;
BOOL ret;

xmlSecAssert2(cert != NULL, -1);
xmlSecAssert2(issuerCert != NULL, -1);

flags = CERT_STORE_REVOCATION_FLAG | CERT_STORE_SIGNATURE_FLAG;
ret = CertVerifySubjectCertificateContext(cert, issuerCert, &flags);
if (!ret) {
xmlSecMSCngLastError("CertVerifySubjectCertificateContext", NULL);
return(-1);
}

/* parse returned flags: https://learn.microsoft.com/en-us/previous-versions/windows/embedded/ms883939(v=msdn.10) */
if ((flags & CERT_STORE_SIGNATURE_FLAG) != 0) {
xmlSecOtherError(XMLSEC_ERRORS_R_CERT_VERIFY_FAILED,
NULL,
"CertVerifySubjectCertificateContext: CERT_STORE_SIGNATURE_FLAG");
return(0);
} else if (((flags & CERT_STORE_REVOCATION_FLAG) != 0) && ((flags & CERT_STORE_NO_CRL_FLAG) == 0)) {
/* If CERT_STORE_REVOCATION_FLAG is enabled and the issuer does not have a CRL in the store,
then CERT_STORE_NO_CRL_FLAG is set in addition to CERT_STORE_REVOCATION_FLAG. */
xmlSecOtherError(XMLSEC_ERRORS_R_CERT_VERIFY_FAILED,
NULL,
"CertVerifySubjectCertificateContext: CERT_STORE_REVOCATION_FLAG");
return(0);
}

/* success */
return(1);
}

/**
* xmlSecMSCngX509StoreContainsCert:
* @store: the certificate store
Expand All @@ -412,37 +448,39 @@ static int
xmlSecMSCngX509StoreContainsCert(HCERTSTORE store, CERT_NAME_BLOB* name,
PCCERT_CONTEXT cert)
{
PCCERT_CONTEXT issuerCert = NULL;
DWORD flags;
PCCERT_CONTEXT storeCert = NULL;
int ret;

xmlSecAssert2(store != NULL, -1);
xmlSecAssert2(name != NULL, -1);
xmlSecAssert2(cert != NULL, -1);

issuerCert = CertFindCertificateInStore(store,
storeCert = CertFindCertificateInStore(store,
X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
0,
CERT_FIND_SUBJECT_NAME,
name,
NULL);
if(issuerCert != NULL) {
flags = CERT_STORE_REVOCATION_FLAG | CERT_STORE_SIGNATURE_FLAG;
ret = CertVerifySubjectCertificateContext(cert,
issuerCert,
&flags);
if(ret == 0) {
xmlSecOtherError(XMLSEC_ERRORS_R_CERT_VERIFY_FAILED,
NULL,
"CertVerifySubjectCertificateContext");
CertFreeCertificateContext(issuerCert);
return(-1);
}
CertFreeCertificateContext(issuerCert);
return(1);
if (storeCert == NULL) {
return (0);
}

return(0);
ret = xmlSecMSCngX509StoreVerifySubject(cert, storeCert);
if (ret < 0) {
xmlSecInternalError("xmlSecMSCngX509StoreVerifySubject", NULL);
CertFreeCertificateContext(storeCert);
return(-1);
} else if (ret == 0) {
xmlSecOtherError(XMLSEC_ERRORS_R_CERT_VERIFY_FAILED,
NULL,
"xmlSecMSCngX509StoreVerifySubject");
CertFreeCertificateContext(storeCert);
return(-1);
}

/* success */
CertFreeCertificateContext(storeCert);
return(1);
}

static int
Expand All @@ -451,14 +489,14 @@ xmlSecMSCngVerifyCertTime(PCCERT_CONTEXT cert, LPFILETIME time) {
xmlSecAssert2(cert->pCertInfo != NULL, -1);
xmlSecAssert2(time != NULL, -1);

if(CompareFileTime(&cert->pCertInfo->NotBefore, time) == 1) {
if(CompareFileTime(&(cert->pCertInfo->NotBefore), time) == 1) {
xmlSecOtherError(XMLSEC_ERRORS_R_CERT_VERIFY_FAILED,
NULL,
"CompareFileTime");
return(-1);
}

if(CompareFileTime(&cert->pCertInfo->NotAfter, time) == -1) {
if(CompareFileTime(&(cert->pCertInfo->NotAfter), time) == -1) {
xmlSecOtherError(XMLSEC_ERRORS_R_CERT_VERIFY_FAILED,
NULL,
"CompareFileTime");
Expand All @@ -485,13 +523,13 @@ xmlSecMSCngX509StoreVerifyCertificateOwn(PCCERT_CONTEXT cert, FILETIME* time,
HCERTSTORE trustedStore, HCERTSTORE untrustedStore, HCERTSTORE certStore
) {
PCCERT_CONTEXT issuerCert = NULL;
DWORD flags;
int ret;

xmlSecAssert2(cert != NULL, -1);
xmlSecAssert2(trustedStore != NULL, -1);
xmlSecAssert2(certStore != NULL, -1);

/* check certificate validity and revokation */
ret = xmlSecMSCngVerifyCertTime(cert, time);
if(ret < 0) {
xmlSecInternalError("xmlSecMSCngVerifyCertTime", NULL);
Expand All @@ -506,19 +544,18 @@ xmlSecMSCngX509StoreVerifyCertificateOwn(PCCERT_CONTEXT cert, FILETIME* time,

/* does trustedStore contain cert directly? */
ret = xmlSecMSCngX509StoreContainsCert(trustedStore,
&cert->pCertInfo->Subject, cert);
&(cert->pCertInfo->Subject), cert);
if(ret < 0) {
xmlSecInternalError("xmlSecMSCngX509StoreContainsCert", NULL);
return(-1);
}
if(ret == 1) {
} else if(ret == 1) {
/* success */
return(1);
}

/* does trustedStore contain the issuer cert? */
ret = xmlSecMSCngX509StoreContainsCert(trustedStore,
&cert->pCertInfo->Issuer, cert);
&(cert->pCertInfo->Issuer), cert);
if(ret < 0) {
xmlSecInternalError("xmlSecMSCngX509StoreContainsCert", NULL);
return(-1);
Expand All @@ -529,8 +566,8 @@ xmlSecMSCngX509StoreVerifyCertificateOwn(PCCERT_CONTEXT cert, FILETIME* time,

/* is cert self-signed? no recursion in that case */
if(CertCompareCertificateName(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
&cert->pCertInfo->Subject,
&cert->pCertInfo->Issuer)) {
&(cert->pCertInfo->Subject),
&(cert->pCertInfo->Issuer))) {
/* not verified */
return(0);
}
Expand All @@ -540,14 +577,19 @@ xmlSecMSCngX509StoreVerifyCertificateOwn(PCCERT_CONTEXT cert, FILETIME* time,
X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
0,
CERT_FIND_SUBJECT_NAME,
&cert->pCertInfo->Issuer,
&(cert->pCertInfo->Issuer),
NULL);
if(issuerCert != NULL) {
flags = CERT_STORE_REVOCATION_FLAG | CERT_STORE_SIGNATURE_FLAG;
ret = CertVerifySubjectCertificateContext(cert, issuerCert, &flags);
if(ret == 0) {
xmlSecOtherError(XMLSEC_ERRORS_R_CERT_VERIFY_FAILED, NULL,
"CertVerifySubjectCertificateContext");
ret = xmlSecMSCngX509StoreVerifySubject(cert, issuerCert);
if (ret < 0) {
xmlSecInternalError("xmlSecMSCngX509StoreVerifySubject", NULL);
CertFreeCertificateContext(issuerCert);
return(-1);
}
else if (ret == 0) {
xmlSecOtherError(XMLSEC_ERRORS_R_CERT_VERIFY_FAILED,
NULL,
"xmlSecMSCngX509StoreVerifySubject");
CertFreeCertificateContext(issuerCert);
return(-1);
}
Expand All @@ -571,14 +613,19 @@ xmlSecMSCngX509StoreVerifyCertificateOwn(PCCERT_CONTEXT cert, FILETIME* time,
X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
0,
CERT_FIND_SUBJECT_NAME,
&cert->pCertInfo->Issuer,
&(cert->pCertInfo->Issuer),
NULL);
if(issuerCert != NULL) {
flags = CERT_STORE_REVOCATION_FLAG | CERT_STORE_SIGNATURE_FLAG;
ret = CertVerifySubjectCertificateContext(cert, issuerCert, &flags);
if(ret == 0) {
xmlSecOtherError(XMLSEC_ERRORS_R_CERT_VERIFY_FAILED, NULL,
"CertVerifySubjectCertificateContext");
ret = xmlSecMSCngX509StoreVerifySubject(cert, issuerCert);
if (ret < 0) {
xmlSecInternalError("xmlSecMSCngX509StoreVerifySubject", NULL);
CertFreeCertificateContext(issuerCert);
return(-1);
}
else if (ret == 0) {
xmlSecOtherError(XMLSEC_ERRORS_R_CERT_VERIFY_FAILED,
NULL,
"xmlSecMSCngX509StoreVerifySubject");
CertFreeCertificateContext(issuerCert);
return(-1);
}
Expand Down
Loading
Loading