Skip to content

Commit

Permalink
(xmlsec-mscng, xmlsec-mscrypto, xmlsec-gnutls) Improve cert verificat…
Browse files Browse the repository at this point in the history 
…ion (#819)
  • Loading branch information
@lsh123
lsh123 authored Jul 18, 2024
1 parent c1f4524 commit 9f76374
Show file tree
Hide file tree
Showing 23 changed files with 499 additions and 150 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/make-check.yml
View file
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
Empty file.
5 changes: 3 additions & 2 deletions docs/index.html
View file
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
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
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
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
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
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

0 comments on commit 9f76374

Please sign in to comment.