Skip to content

Commit 0faa960

Browse files
authored
Validate signing certificate on issue date and on TSA time (#644)
IB-8296 Signed-off-by: Raul Metsma <[email protected]>
1 parent 8ffec98 commit 0faa960

16 files changed

+185
-156
lines changed

src/SignatureTST.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
#include "ASiC_S.h"
2323
#include "DataFile_p.h"
24+
#include "crypto/Digest.h"
2425
#include "crypto/TS.h"
2526
#include "crypto/X509Cert.h"
2627
#include "util/DateTime.h"

src/SignatureXAdES_T.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "crypto/Signer.h"
2626
#include "crypto/TS.h"
2727
#include "crypto/X509Cert.h"
28+
#include "crypto/X509CertStore.h"
2829
#include "util/DateTime.h"
2930
#include "util/log.h"
3031

@@ -113,9 +114,7 @@ void SignatureXAdES_T::validate(const std::string &policy) const
113114
signatures->c14n(digest, canonicalizationMethod, signatureValue());
114115
});
115116

116-
tm tm = tsa.time();
117-
time_t validateTime = util::date::mkgmtime(tm);
118-
if(!signingCertificate().isValid(&validateTime))
117+
if(!X509CertStore::instance()->verify(signingCertificate(), policy == POLv1, tsa.time()))
119118
THROW("Signing certificate was not valid on signing time");
120119

121120
auto completeCertRefs = usp/"CompleteCertificateRefs";

src/XMLDocument.h

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
#include "crypto/Digest.h"
2323
#include "util/log.h"
24+
#include "util/memory.h"
2425

2526
#include <libxml/parser.h>
2627
#include <libxml/xmlschemas.h>
@@ -32,30 +33,12 @@
3233

3334
#include <openssl/evp.h>
3435

35-
#include <memory>
3636
#include <istream>
3737

3838
namespace digidoc {
3939

4040
#define VERSION_CHECK(major, minor, patch) (((major)<<16)|((minor)<<8)|(patch))
4141

42-
template<typename> struct unique_xml;
43-
template<class T>
44-
struct unique_xml<void(T *)>
45-
{
46-
using type = std::unique_ptr<T,void(*)(T *)>;
47-
};
48-
49-
template<typename T>
50-
using unique_xml_t = typename unique_xml<T>::type;
51-
52-
template<class T, typename D>
53-
[[nodiscard]]
54-
constexpr std::unique_ptr<T, D> make_unique_ptr(T *p, D d) noexcept
55-
{
56-
return {p, std::forward<D>(d)};
57-
}
58-
5942
static std::vector<unsigned char> from_base64(std::string_view data)
6043
{
6144
static constexpr std::string_view whitespace {" \n\r\f\t\v"};
@@ -297,7 +280,7 @@ struct XMLNode: public XMLElem<xmlNode>
297280
}
298281
};
299282

300-
struct XMLDocument: public unique_xml_t<decltype(xmlFreeDoc)>, public XMLNode
283+
struct XMLDocument: public unique_free_t<xmlDoc>, public XMLNode
301284
{
302285
static constexpr std::string_view C14D_ID_1_0 {"http://www.w3.org/TR/2001/REC-xml-c14n-20010315"};
303286
static constexpr std::string_view C14D_ID_1_0_COM {"http://www.w3.org/TR/2001/REC-xml-c14n-20010315#WithComments"};
@@ -309,7 +292,7 @@ struct XMLDocument: public unique_xml_t<decltype(xmlFreeDoc)>, public XMLNode
309292
using XMLNode::operator bool;
310293

311294
XMLDocument(element_type *ptr = {}, const XMLName &n = {}) noexcept
312-
: std::unique_ptr<element_type, deleter_type>(ptr, xmlFreeDoc)
295+
: unique_free_t<xmlDoc>(ptr, xmlFreeDoc)
313296
, XMLNode{xmlDocGetRootElement(get())}
314297
{
315298
if(d && !n.name.empty() && n.name != name() && !n.ns.empty() && n.ns != ns())

src/crypto/Connect.cpp

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include <zlib.h>
3131

3232
#include <algorithm>
33+
#include <sstream>
3334
#include <thread>
3435

3536
#ifdef _WIN32
@@ -147,13 +148,13 @@ Connect::Connect(const string &_url, string _method, int _timeout, const vector<
147148
if(!certs.empty())
148149
{
149150
SSL_CTX_set_verify(ssl.get(), SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, nullptr);
150-
SSL_CTX_set_cert_verify_callback(ssl.get(), [](X509_STORE_CTX *store, void *data) -> int {
151-
X509 *x509 = X509_STORE_CTX_get0_cert(store);
152-
auto *certs = (vector<X509Cert>*)data;
153-
return any_of(certs->cbegin(), certs->cend(), [x509](const X509Cert &cert) {
154-
return cert && cert == x509;
155-
}) ? 1 : 0;
156-
}, const_cast<vector<X509Cert>*>(&certs));
151+
X509_STORE *store = SSL_CTX_get_cert_store(ssl.get());
152+
X509_STORE_set_flags(store, X509_V_FLAG_TRUSTED_FIRST | X509_V_FLAG_PARTIAL_CHAIN);
153+
for(const X509Cert &cert: certs)
154+
{
155+
if(cert.handle())
156+
X509_STORE_add_cert(store, cert.handle());
157+
}
157158
}
158159
BIO *sbio = BIO_new_ssl(ssl.get(), 1);
159160
if(!sbio)
@@ -292,7 +293,7 @@ Connect::Result Connect::exec(initializer_list<pair<string_view,string_view>> he
292293
stringstream stream(r.content);
293294
string line;
294295
auto to_lower = [](string str) {
295-
std::transform(str.begin(), str.end(), str.begin(), ::tolower);
296+
transform(str.begin(), str.end(), str.begin(), ::tolower);
296297
return str;
297298
};
298299
while(getline(stream, line))

src/crypto/Digest.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include <openssl/x509.h>
2727

2828
#include <array>
29+
#include <istream>
2930

3031
using namespace std;
3132
using namespace digidoc;
@@ -37,7 +38,7 @@ using namespace digidoc;
3738
* @throws Exception throws exception if the digest calculator initialization failed.
3839
*/
3940
Digest::Digest(string_view uri)
40-
: d(SCOPE_PTR(EVP_MD_CTX, EVP_MD_CTX_new()))
41+
: d(EVP_MD_CTX_new(), EVP_MD_CTX_free)
4142
{
4243
if(uri.empty() && Conf::instance()->digestUri() == URI_SHA1)
4344
THROW("Unsupported digest method %.*s", int(uri.size()), uri.data());
@@ -269,7 +270,7 @@ vector<unsigned char> Digest::result() const
269270
return result;
270271
}
271272

272-
vector<unsigned char> Digest::result(const vector<unsigned char> &data)
273+
vector<unsigned char> Digest::result(const vector<unsigned char> &data) const
273274
{
274275
update(data.data(), data.size());
275276
return result();

src/crypto/Digest.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@
2121

2222
#include "../Exports.h"
2323

24-
#include <memory>
24+
#include "util/memory.h"
25+
2526
#include <string>
2627
#include <vector>
2728

@@ -73,7 +74,7 @@ namespace digidoc
7374
Digest(std::string_view uri = {});
7475
void update(const unsigned char *data, size_t length) const;
7576
void update(std::istream &is) const;
76-
std::vector<unsigned char> result(const std::vector<unsigned char> &data);
77+
std::vector<unsigned char> result(const std::vector<unsigned char> &data) const;
7778
std::vector<unsigned char> result() const;
7879
std::string uri() const;
7980

@@ -89,7 +90,7 @@ namespace digidoc
8990
static std::string digestInfoUri(const std::vector<unsigned char> &digest);
9091

9192
private:
92-
std::unique_ptr<EVP_MD_CTX, void (*)(EVP_MD_CTX*)> d;
93+
unique_free_t<EVP_MD_CTX> d;
9394
};
9495

9596
}

src/crypto/OCSP.cpp

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ using namespace std;
4646
* Initialize OCSP certificate validator.
4747
*/
4848
OCSP::OCSP(const X509Cert &cert, const X509Cert &issuer, const std::string &userAgent)
49+
: resp(nullptr, OCSP_RESPONSE_free)
50+
, basic(nullptr, OCSP_BASICRESP_free)
4951
{
5052
if(!cert)
5153
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
5658
if(url.empty())
5759
{
5860
STACK_OF(OPENSSL_STRING) *urls = X509_get1_ocsp(cert.handle());
59-
if(sk_OPENSSL_STRING_num(urls) > 0)
61+
if(urls && sk_OPENSSL_STRING_num(urls) > 0)
6062
url = sk_OPENSSL_STRING_value(urls, 0);
6163
X509_email_free(urls);
6264
}
@@ -90,8 +92,8 @@ OCSP::OCSP(const X509Cert &cert, const X509Cert &issuer, const std::string &user
9092
THROW("OCSP service responded - Forbidden");
9193
if(!result)
9294
THROW("Failed to send OCSP request");
93-
const unsigned char *p2 = (const unsigned char*)result.content.c_str();
94-
resp.reset(d2i_OCSP_RESPONSE(nullptr, &p2, long(result.content.size())), OCSP_RESPONSE_free);
95+
const auto *p2 = (const unsigned char*)result.content.c_str();
96+
resp.reset(d2i_OCSP_RESPONSE(nullptr, &p2, long(result.content.size())));
9597

9698
switch(int respStatus = OCSP_response_status(resp.get()))
9799
{
@@ -106,7 +108,7 @@ OCSP::OCSP(const X509Cert &cert, const X509Cert &issuer, const std::string &user
106108
THROW("OCSP request failed, response status: %s", OCSP_response_status_str(respStatus));
107109
}
108110

109-
basic.reset(OCSP_response_get1_basic(resp.get()), OCSP_BASICRESP_free);
111+
basic.reset(OCSP_response_get1_basic(resp.get()));
110112
if(!basic)
111113
THROW("Incorrect OCSP response.");
112114

@@ -127,12 +129,14 @@ OCSP::OCSP(const X509Cert &cert, const X509Cert &issuer, const std::string &user
127129
}
128130

129131
OCSP::OCSP(const unsigned char *data, size_t size)
132+
: resp(nullptr, OCSP_RESPONSE_free)
133+
, basic(nullptr, OCSP_BASICRESP_free)
130134
{
131135
if(size == 0)
132136
return;
133-
resp.reset(d2i_OCSP_RESPONSE(nullptr, &data, long(size)), OCSP_RESPONSE_free);
137+
resp.reset(d2i_OCSP_RESPONSE(nullptr, &data, long(size)));
134138
if(resp)
135-
basic.reset(OCSP_response_get1_basic(resp.get()), OCSP_BASICRESP_free);
139+
basic.reset(OCSP_response_get1_basic(resp.get()));
136140
}
137141

138142
bool OCSP::compareResponderCert(const X509Cert &cert) const
@@ -187,17 +191,19 @@ void OCSP::verifyResponse(const X509Cert &cert) const
187191
THROW("Failed to verify OCSP response.");
188192

189193
tm tm = producedAt();
190-
time_t t = util::date::mkgmtime(tm);
191-
SCOPE(X509_STORE, store, X509CertStore::createStore(X509CertStore::OCSP, &t));
192-
STACK_OF(X509) *stack = sk_X509_new_null();
194+
// Some OCSP-s do not have certificates in response and stack is used for finding certificate for this
195+
auto stack = make_unique_ptr(sk_X509_new_null(), [](auto *sk) { sk_X509_free(sk); });
193196
for(const X509Cert &i: X509CertStore::instance()->certs(X509CertStore::OCSP))
194197
{
195198
if(compareResponderCert(i))
196-
sk_X509_push(stack, i.handle());
199+
sk_X509_push(stack.get(), i.handle());
197200
}
198-
int result = OCSP_basic_verify(basic.get(), stack, store.get(), OCSP_NOCHECKS);
199-
sk_X509_free(stack);
200-
if(result != 1)
201+
auto store = X509CertStore::createStore(X509CertStore::OCSP, tm);
202+
#if OPENSSL_VERSION_NUMBER < 0x30000000L
203+
if(OCSP_basic_verify(basic.get(), stack.get(), store.get(), OCSP_NOCHECKS) != 1)
204+
#else
205+
if(OCSP_basic_verify(basic.get(), stack.get(), store.get(), OCSP_NOCHECKS | OCSP_PARTIAL_CHAIN) != 1)
206+
#endif
201207
{
202208
unsigned long err = ERR_get_error();
203209
if(ERR_GET_LIB(err) == ERR_LIB_OCSP &&
@@ -259,17 +265,18 @@ void OCSP::verifyResponse(const X509Cert &cert) const
259265
*/
260266
vector<unsigned char> OCSP::nonce() const
261267
{
268+
vector<unsigned char> nonce;
262269
if(!basic)
263-
return {};
270+
return nonce;
264271
int resp_idx = OCSP_BASICRESP_get_ext_by_NID(basic.get(), NID_id_pkix_OCSP_Nonce, -1);
265272
if(resp_idx < 0)
266-
return {};
273+
return nonce;
267274
X509_EXTENSION *ext = OCSP_BASICRESP_get_ext(basic.get(), resp_idx);
268275
if(!ext)
269-
return {};
276+
return nonce;
270277

271278
ASN1_OCTET_STRING *value = X509_EXTENSION_get_data(ext);
272-
vector<unsigned char> nonce(value->data, value->data + value->length);
279+
nonce.assign(value->data, std::next(value->data, value->length));
273280
//OpenSSL OCSP created messages NID_id_pkix_OCSP_Nonce field is DER encoded twice, not a problem with java impl
274281
//XXX: UglyHackTM check if nonceAsn1 contains ASN1_OCTET_STRING
275282
//XXX: if first 2 bytes seem to be beginning of DER ASN1_OCTET_STRING then remove them
@@ -281,9 +288,9 @@ vector<unsigned char> OCSP::nonce() const
281288

282289
tm OCSP::producedAt() const
283290
{
284-
if(!basic)
285-
return {};
286291
tm tm {};
292+
if(!basic)
293+
return tm;
287294
ASN1_TIME_to_tm(OCSP_resp_get0_produced_at(basic.get()), &tm);
288295
return tm;
289296
}

src/crypto/OCSP.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@
1919

2020
#pragma once
2121

22-
#include <memory>
22+
#include "util/memory.h"
23+
2324
#include <string>
2425
#include <vector>
2526

@@ -51,7 +52,7 @@ namespace digidoc
5152
private:
5253
bool compareResponderCert(const X509Cert &cert) const;
5354

54-
std::shared_ptr<OCSP_RESPONSE> resp;
55-
std::shared_ptr<OCSP_BASICRESP> basic;
55+
unique_free_t<OCSP_RESPONSE> resp;
56+
unique_free_t<OCSP_BASICRESP> basic;
5657
};
5758
}

src/crypto/OpenSSLHelpers.h

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@
2121

2222
#include "Exception.h"
2323
#include "util/log.h"
24-
25-
#include <memory>
26-
#include <sstream>
24+
#include "util/memory.h"
2725

2826
#include <openssl/err.h>
2927

@@ -34,22 +32,23 @@
3432
namespace digidoc
3533
{
3634

37-
#define SCOPE_PTR_FREE(TYPE, DATA, FREE) std::unique_ptr<TYPE,decltype(&FREE)>(static_cast<TYPE*>(DATA), FREE)
35+
#define SCOPE_PTR_FREE(TYPE, DATA, FREE) make_unique_ptr(DATA, FREE)
3836
#define SCOPE_PTR(TYPE, DATA) SCOPE_PTR_FREE(TYPE, DATA, TYPE##_free)
3937
#define SCOPE(TYPE, VAR, DATA) auto VAR = SCOPE_PTR_FREE(TYPE, DATA, TYPE##_free)
4038

4139
template<class T, typename Func>
4240
[[nodiscard]]
4341
inline std::vector<unsigned char> i2d(T *obj, Func func)
4442
{
43+
std::vector<unsigned char> result;
4544
if(!obj)
46-
return {};
45+
return result;
4746
int size = func(obj, nullptr);
4847
if(size <= 0)
49-
return {};
50-
std::vector<unsigned char> result(size_t(size), 0);
48+
return result;
49+
result.resize(size_t(size), 0);
5150
if(unsigned char *p = result.data(); func(obj, &p) != size)
52-
return {};
51+
result.clear();
5352
return result;
5453
}
5554

0 commit comments

Comments
 (0)