Validate signing certificate on issue date and on TSA time

IB-8296

Signed-off-by: Raul Metsma <[email protected]>

.github/workflows/build.yml

@@ -24,6 +24,8 @@ jobs:
         brew install --formula ninja swig doxygen boost
         brew unlink [email protected] || true
         brew unlink [email protected] || true
+        brew unlink [email protected] || true
+        brew unlink openssl@3 || true
         brew unlink xz
     - name: Cache
       uses: actions/cache@v4

src/SignatureTST.cpp

@@ -21,6 +21,7 @@
 
 #include "ASiC_S.h"
 #include "DataFile_p.h"
+#include "crypto/Digest.h"
 #include "crypto/TS.h"
 #include "crypto/X509Cert.h"
 #include "util/DateTime.h"

src/SignatureXAdES_T.cpp

@@ -25,6 +25,7 @@
 #include "crypto/Signer.h"
 #include "crypto/TS.h"
 #include "crypto/X509Cert.h"
+#include "crypto/X509CertStore.h"
 #include "util/DateTime.h"
 #include "util/log.h"
 
@@ -113,9 +114,7 @@ void SignatureXAdES_T::validate(const std::string &policy) const
             signatures->c14n(digest, canonicalizationMethod, signatureValue());
         });
 
-        tm tm = tsa.time();
-        time_t validateTime = util::date::mkgmtime(tm);
-        if(!signingCertificate().isValid(&validateTime))
+        if(!X509CertStore::instance()->verify(signingCertificate(), policy == POLv1, tsa.time()))
             THROW("Signing certificate was not valid on signing time");
 
         auto completeCertRefs = usp/"CompleteCertificateRefs";

src/XMLDocument.h

@@ -21,6 +21,7 @@
 
 #include "crypto/Digest.h"
 #include "util/log.h"
+#include "util/memory.h"
 
 #include <libxml/parser.h>
 #include <libxml/xmlschemas.h>
@@ -40,23 +41,6 @@ namespace digidoc {
 
 #define VERSION_CHECK(major, minor, patch) (((major)<<16)|((minor)<<8)|(patch))
 
-template<typename> struct unique_xml;
-template<class T>
-struct unique_xml<void(T *)>
-{
-    using type = std::unique_ptr<T,void(*)(T *)>;
-};
-
-template<typename T>
-using unique_xml_t = typename unique_xml<T>::type;
-
-template<class T, typename D>
-[[nodiscard]]
-constexpr std::unique_ptr<T, D> make_unique_ptr(T *p, D d) noexcept
-{
-    return {p, std::forward<D>(d)};
-}
-
 static std::vector<unsigned char> from_base64(std::string_view data)
 {
     static constexpr std::string_view whitespace {" \n\r\f\t\v"};
@@ -301,7 +285,7 @@ struct XMLNode: public XMLElem<xmlNode>
     }
 };
 
-struct XMLDocument: public unique_xml_t<decltype(xmlFreeDoc)>, public XMLNode
+struct XMLDocument: public unique_free_t<xmlDoc>, public XMLNode
 {
     static constexpr std::string_view C14D_ID_1_0 {"http://www.w3.org/TR/2001/REC-xml-c14n-20010315"};
     static constexpr std::string_view C14D_ID_1_0_COM {"http://www.w3.org/TR/2001/REC-xml-c14n-20010315#WithComments"};
@@ -313,7 +297,7 @@ struct XMLDocument: public unique_xml_t<decltype(xmlFreeDoc)>, public XMLNode
     using XMLNode::operator bool;
 
     XMLDocument(element_type *ptr = {}, const XMLName &n = {}) noexcept
-        : std::unique_ptr<element_type, deleter_type>(ptr, xmlFreeDoc)
+        : unique_free_t<xmlDoc>(ptr, xmlFreeDoc)
         , XMLNode{xmlDocGetRootElement(get())}
     {
         if(d && !n.name.empty() && n.name != name() && !n.ns.empty() && n.ns != ns())

src/crypto/Connect.cpp

@@ -30,6 +30,7 @@
 #include <zlib.h>
 
 #include <algorithm>
+#include <sstream>
 #include <thread>
 
 #ifdef _WIN32
@@ -147,13 +148,13 @@ Connect::Connect(const string &_url, string _method, int _timeout, const vector<
         if(!certs.empty())
         {
             SSL_CTX_set_verify(ssl.get(), SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, nullptr);
-            SSL_CTX_set_cert_verify_callback(ssl.get(), [](X509_STORE_CTX *store, void *data) -> int {
-                X509 *x509 = X509_STORE_CTX_get0_cert(store);
-                auto *certs = (vector<X509Cert>*)data;
-                return any_of(certs->cbegin(), certs->cend(), [x509](const X509Cert &cert) {
-                    return cert && cert == x509;
-                }) ? 1 : 0;
-            }, const_cast<vector<X509Cert>*>(&certs));
+            X509_STORE *store = SSL_CTX_get_cert_store(ssl.get());
+            X509_STORE_set_flags(store, X509_V_FLAG_TRUSTED_FIRST | X509_V_FLAG_PARTIAL_CHAIN);
+            for(const X509Cert &cert: certs)
+            {
+                if(cert.handle())
+                    X509_STORE_add_cert(store, cert.handle());
+            }
         }
         BIO *sbio = BIO_new_ssl(ssl.get(), 1);
         if(!sbio)
@@ -292,7 +293,7 @@ Connect::Result Connect::exec(initializer_list<pair<string_view,string_view>> he
     stringstream stream(r.content);
     string line;
     auto to_lower = [](string str) {
-        std::transform(str.begin(), str.end(), str.begin(), ::tolower);
+        transform(str.begin(), str.end(), str.begin(), ::tolower);
         return str;
     };
     while(getline(stream, line))

src/crypto/Digest.cpp

@@ -26,6 +26,7 @@
 #include <openssl/x509.h>
 
 #include <array>
+#include <istream>
 
 using namespace std;
 using namespace digidoc;
@@ -37,7 +38,7 @@ using namespace digidoc;
  * @throws Exception throws exception if the digest calculator initialization failed.
  */
 Digest::Digest(string_view uri)
-    : d(SCOPE_PTR(EVP_MD_CTX, EVP_MD_CTX_new()))
+    : d(EVP_MD_CTX_new(), EVP_MD_CTX_free)
 {
     if(uri.empty() && Conf::instance()->digestUri() == URI_SHA1)
         THROW("Unsupported digest method %.*s", int(uri.size()), uri.data());
@@ -269,7 +270,7 @@ vector<unsigned char> Digest::result() const
     return result;
 }
 
-vector<unsigned char> Digest::result(const vector<unsigned char> &data)
+vector<unsigned char> Digest::result(const vector<unsigned char> &data) const
 {
     update(data.data(), data.size());
     return result();

src/crypto/Digest.h

@@ -21,7 +21,8 @@
 
 #include "../Exports.h"
 
-#include <memory>
+#include "util/memory.h"
+
 #include <string>
 #include <vector>
 
@@ -73,7 +74,7 @@ namespace digidoc
           Digest(std::string_view uri = {});
           void update(const unsigned char *data, size_t length) const;
           void update(std::istream &is) const;
-          std::vector<unsigned char> result(const std::vector<unsigned char> &data);
+          std::vector<unsigned char> result(const std::vector<unsigned char> &data) const;
           std::vector<unsigned char> result() const;
           std::string uri() const;
 
@@ -89,7 +90,7 @@ namespace digidoc
           static std::string digestInfoUri(const std::vector<unsigned char> &digest);
 
       private:
-          std::unique_ptr<EVP_MD_CTX, void (*)(EVP_MD_CTX*)> d;
+          unique_free_t<EVP_MD_CTX> d;
     };
 
 }

src/crypto/OCSP.cpp

@@ -46,6 +46,8 @@ using namespace std;
  * Initialize OCSP certificate validator.
  */
 OCSP::OCSP(const X509Cert &cert, const X509Cert &issuer, const std::string &userAgent)
+    : resp(nullptr, OCSP_RESPONSE_free)
+    , basic(nullptr, OCSP_BASICRESP_free)
 {
     if(!cert)
         THROW("Can not check X.509 certificate, certificate is NULL pointer.");
@@ -56,7 +58,7 @@ OCSP::OCSP(const X509Cert &cert, const X509Cert &issuer, const std::string &user
     if(url.empty())
     {
         STACK_OF(OPENSSL_STRING) *urls = X509_get1_ocsp(cert.handle());
-        if(sk_OPENSSL_STRING_num(urls) > 0)
+        if(urls && sk_OPENSSL_STRING_num(urls) > 0)
             url = sk_OPENSSL_STRING_value(urls, 0);
         X509_email_free(urls);
     }
@@ -90,8 +92,8 @@ OCSP::OCSP(const X509Cert &cert, const X509Cert &issuer, const std::string &user
         THROW("OCSP service responded - Forbidden");
     if(!result)
         THROW("Failed to send OCSP request");
-    const unsigned char *p2 = (const unsigned char*)result.content.c_str();
-    resp.reset(d2i_OCSP_RESPONSE(nullptr, &p2, long(result.content.size())), OCSP_RESPONSE_free);
+    const auto *p2 = (const unsigned char*)result.content.c_str();
+    resp.reset(d2i_OCSP_RESPONSE(nullptr, &p2, long(result.content.size())));
 
     switch(int respStatus = OCSP_response_status(resp.get()))
     {
@@ -106,7 +108,7 @@ OCSP::OCSP(const X509Cert &cert, const X509Cert &issuer, const std::string &user
         THROW("OCSP request failed, response status: %s", OCSP_response_status_str(respStatus));
     }
 
-    basic.reset(OCSP_response_get1_basic(resp.get()), OCSP_BASICRESP_free);
+    basic.reset(OCSP_response_get1_basic(resp.get()));
     if(!basic)
         THROW("Incorrect OCSP response.");
 
@@ -127,12 +129,14 @@ OCSP::OCSP(const X509Cert &cert, const X509Cert &issuer, const std::string &user
 }
 
 OCSP::OCSP(const unsigned char *data, size_t size)
+    : resp(nullptr, OCSP_RESPONSE_free)
+    , basic(nullptr, OCSP_BASICRESP_free)
 {
     if(size == 0)
         return;
-    resp.reset(d2i_OCSP_RESPONSE(nullptr, &data, long(size)), OCSP_RESPONSE_free);
+    resp.reset(d2i_OCSP_RESPONSE(nullptr, &data, long(size)));
     if(resp)
-        basic.reset(OCSP_response_get1_basic(resp.get()), OCSP_BASICRESP_free);
+        basic.reset(OCSP_response_get1_basic(resp.get()));
 }
 
 bool OCSP::compareResponderCert(const X509Cert &cert) const
@@ -187,17 +191,19 @@ void OCSP::verifyResponse(const X509Cert &cert) const
         THROW("Failed to verify OCSP response.");
 
     tm tm = producedAt();
-    time_t t = util::date::mkgmtime(tm);
-    SCOPE(X509_STORE, store, X509CertStore::createStore(X509CertStore::OCSP, &t));
-    STACK_OF(X509) *stack = sk_X509_new_null();
+    // 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))
     {
         if(compareResponderCert(i))
-            sk_X509_push(stack, i.handle());
+            sk_X509_push(stack.get(), i.handle());
     }
-    int result = OCSP_basic_verify(basic.get(), stack, store.get(), OCSP_NOCHECKS);
-    sk_X509_free(stack);
-    if(result != 1)
+    auto store = X509CertStore::createStore(X509CertStore::OCSP, tm);
+#if OPENSSL_VERSION_NUMBER < 0x30000000L
+    if(OCSP_basic_verify(basic.get(), stack.get(), store.get(), OCSP_NOCHECKS) != 1)
+#else
+    if(OCSP_basic_verify(basic.get(), stack.get(), store.get(), OCSP_NOCHECKS | OCSP_PARTIAL_CHAIN) != 1)
+#endif
     {
         unsigned long err = ERR_get_error();
         if(ERR_GET_LIB(err) == ERR_LIB_OCSP &&
@@ -269,7 +275,7 @@ vector<unsigned char> OCSP::nonce() const
         return {};
 
     ASN1_OCTET_STRING *value = X509_EXTENSION_get_data(ext);
-    vector<unsigned char> nonce(value->data, value->data + value->length);
+    vector<unsigned char> nonce(value->data, std::next(value->data, value->length));
     //OpenSSL OCSP created messages NID_id_pkix_OCSP_Nonce field is DER encoded twice, not a problem with java impl
     //XXX: UglyHackTM check if nonceAsn1 contains ASN1_OCTET_STRING
     //XXX: if first 2 bytes seem to be beginning of DER ASN1_OCTET_STRING then remove them

src/crypto/OCSP.h

@@ -19,7 +19,8 @@
 
 #pragma once
 
-#include <memory>
+#include "util/memory.h"
+
 #include <string>
 #include <vector>
 
@@ -51,7 +52,7 @@ namespace digidoc
       private:
           bool compareResponderCert(const X509Cert &cert) const;
 
-          std::shared_ptr<OCSP_RESPONSE> resp;
-          std::shared_ptr<OCSP_BASICRESP> basic;
+          unique_free_t<OCSP_RESPONSE> resp;
+          unique_free_t<OCSP_BASICRESP> basic;
     };
 }

src/crypto/OpenSSLHelpers.h

@@ -21,9 +21,7 @@
 
 #include "Exception.h"
 #include "util/log.h"
-
-#include <memory>
-#include <sstream>
+#include "util/memory.h"
 
 #include <openssl/err.h>
 
@@ -34,7 +32,7 @@
 namespace digidoc
 {
 
-#define SCOPE_PTR_FREE(TYPE, DATA, FREE) std::unique_ptr<TYPE,decltype(&FREE)>(static_cast<TYPE*>(DATA), FREE)
+#define SCOPE_PTR_FREE(TYPE, DATA, FREE) make_unique_ptr(DATA, FREE)
 #define SCOPE_PTR(TYPE, DATA) SCOPE_PTR_FREE(TYPE, DATA, TYPE##_free)
 #define SCOPE(TYPE, VAR, DATA) auto VAR = SCOPE_PTR_FREE(TYPE, DATA, TYPE##_free)