From 43923720c15440d175ef9ba21c8fa1b91d3df6a4 Mon Sep 17 00:00:00 2001 From: Raul Metsma Date: Fri, 30 May 2025 14:17:29 +0300 Subject: [PATCH] Check certificate key type and fetch certifcate when responder is missing IB-8461 Signed-off-by: Raul Metsma --- src/Container.cpp | 1 + src/crypto/OCSP.cpp | 51 +++++++++++++++++++++++++-------------------- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/src/Container.cpp b/src/Container.cpp index 515c25d89..b3c01df1f 100644 --- a/src/Container.cpp +++ b/src/Container.cpp @@ -120,6 +120,7 @@ void digidoc::initialize(const string &appInfo, const string &userAgent, initCal THROW("Error during initialisation of xmlsec. Failed to register custom callbacks."); INFO("Libxml2 version: %s", LIBXML_DOTTED_VERSION); + INFO("OpenSSL version: %s", OpenSSL_version(OPENSSL_VERSION_STRING)); INFO("Xmlsec1 version: %s", XMLSEC_VERSION); INFO("digidocpp version: %s", VERSION_STR); diff --git a/src/crypto/OCSP.cpp b/src/crypto/OCSP.cpp index cb8467a4a..75e654a8d 100644 --- a/src/crypto/OCSP.cpp +++ b/src/crypto/OCSP.cpp @@ -57,10 +57,9 @@ OCSP::OCSP(const X509Cert &cert, const X509Cert &issuer, const std::string &user string url = Conf::instance()->ocsp(cert.issuerName("CN")); if(url.empty()) { - STACK_OF(OPENSSL_STRING) *urls = X509_get1_ocsp(cert.handle()); - if(urls && sk_OPENSSL_STRING_num(urls) > 0) - url = sk_OPENSSL_STRING_value(urls, 0); - X509_email_free(urls); + if(auto urls = make_unique_ptr(X509_get1_ocsp(cert.handle())); + urls && sk_OPENSSL_STRING_num(urls.get()) > 0) + url = sk_OPENSSL_STRING_value(urls.get(), 0); } DEBUG("OCSP url %s", url.c_str()); if(url.empty()) @@ -70,11 +69,11 @@ OCSP::OCSP(const X509Cert &cert, const X509Cert &issuer, const std::string &user throw e; } - OCSP_CERTID *certId = OCSP_cert_to_id(nullptr, cert.handle(), issuer.handle()); - SCOPE(OCSP_REQUEST, req, OCSP_REQUEST_new()); + auto req = make_unique_ptr(OCSP_REQUEST_new()); if(!req) THROW_OPENSSLEXCEPTION("Failed to create new OCSP request, out of memory?"); + OCSP_CERTID *certId = OCSP_cert_to_id(nullptr, cert.handle(), issuer.handle()); if(!OCSP_request_add0_id(req.get(), certId)) THROW_OPENSSLEXCEPTION("Failed to add certificate ID to OCSP request."); @@ -143,32 +142,35 @@ bool OCSP::compareResponderCert(const X509Cert &cert) const { if(!basic || !cert) return false; + const ASN1_OCTET_STRING *hash {}; const X509_NAME *name {}; - OCSP_resp_get0_id(basic.get(), &hash, &name); - if(name) - return X509_NAME_cmp(X509_get_subject_name(cert.handle()), name) == 0; + if(OCSP_resp_get0_id(basic.get(), &hash, &name) != 1) + return false; if(hash) { std::array sha1{}; ASN1_BIT_STRING *key = X509_get0_pubkey_bitstr(cert.handle()); SHA1(key->data, size_t(key->length), sha1.data()); - return equal(sha1.cbegin(), sha1.cend(), hash->data, std::next(hash->data, hash->length)); + if(!equal(sha1.cbegin(), sha1.cend(), hash->data, std::next(hash->data, hash->length))) + return false; } - return false; + else if(X509_NAME_cmp(X509_get_subject_name(cert.handle()), name) != 0) + return false; + + const ASN1_OBJECT *sigalg {}; + X509_ALGOR_get0(&sigalg, nullptr, nullptr, OCSP_resp_get0_tbs_sigalg(basic.get())); + int pknid = 0; + return OBJ_find_sigid_algs(OBJ_obj2nid(sigalg), nullptr, &pknid) == 1 && + EVP_PKEY_is_a(X509_get0_pubkey(cert.handle()), OBJ_nid2sn(pknid)) == 1; } X509Cert OCSP::responderCert() const { if(!basic) return X509Cert(); - const STACK_OF(X509) *certs = OCSP_resp_get0_certs(basic.get()); - for(int i = 0; i < sk_X509_num(certs); ++i) - { - X509Cert cert(sk_X509_value(certs, i)); - if(compareResponderCert(cert)) - return cert; - } + if(X509 *signer{}; OCSP_resp_get0_signer(basic.get(), &signer, nullptr) != 0 && signer) + return X509Cert(signer); for(const X509Cert &cert: X509CertStore::instance()->certs(X509CertStore::OCSP)) { if(compareResponderCert(cert)) @@ -191,12 +193,15 @@ void OCSP::verifyResponse(const X509Cert &cert) const THROW("Failed to verify OCSP response."); tm tm = producedAt(); - // Some OCSP-s do not have certificates in response and stack is used for finding certificate for this auto stack = make_unique_ptr(sk_X509_new_null(), [](auto *sk) { sk_X509_free(sk); }); - for(const X509Cert &i: X509CertStore::instance()->certs(X509CertStore::OCSP)) + // Some OCSP-s do not have certificates in response and stack is used for finding certificate for this + if(X509 *signer{}; OCSP_resp_get0_signer(basic.get(), &signer, nullptr) == 0 || !signer) { - if(compareResponderCert(i)) - sk_X509_push(stack.get(), i.handle()); + for(const X509Cert &i: X509CertStore::instance()->certs(X509CertStore::OCSP)) + { + if(compareResponderCert(i)) + sk_X509_push(stack.get(), i.handle()); + } } auto store = X509CertStore::createStore(X509CertStore::OCSP, tm); if(OCSP_basic_verify(basic.get(), stack.get(), store.get(), OCSP_NOCHECKS | OCSP_PARTIAL_CHAIN) != 1) @@ -230,7 +235,7 @@ void OCSP::verifyResponse(const X509Cert &cert) const ASN1_OBJECT *md {}; if(OCSP_id_get0_info(nullptr, &md, nullptr, nullptr, const_cast(certID)) == 1) evp_md = EVP_get_digestbyobj(md); - SCOPE(OCSP_CERTID, certId, OCSP_cert_to_id(evp_md, cert.handle(), issuer.handle())); + auto certId = make_unique_ptr(OCSP_cert_to_id(evp_md, cert.handle(), issuer.handle())); if(OCSP_resp_find_status(basic.get(), certId.get(), &status, nullptr, nullptr, nullptr, nullptr) == 1) break; }