| Message ID | 20200805102333.3109-1-themiron@yandex-team.ru |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel,v2] Log serial number of revoked certificate | expand |
Code looks good. Compiled and tested (openssl and revoked cert) on Ubuntu 20.04.
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Your patch has been applied to the master branch.
I have not done much testing, just a test run "make check" on an
OpenSSL and mbedTLS build.
I have not looked into applying it to "release/2.4" - if you think
it's needed, let me know (or if it needs more work because the code
has diverged too much, send a 2.4 patch) - thanks.
commit 992e9cec40539a155afa9eae10502aa62f617965
Author: Vladislav Grishenko
Date: Wed Aug 5 15:23:33 2020 +0500
Log serial number of revoked certificate
Signed-off-by: Vladislav Grishenko <themiron@yandex-team.ru>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20200805102333.3109-1-themiron@yandex-team.ru>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20642.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
Hi, Gert Thank you. I'd appreciate if patch could be applied to release/2.4 too, no changes are required - related code is the same, just hunks offset in ssl_verify.c and ssl_verify_openssl.c I've tested 2.4.9 [git:release/2.4/7c428ca19a8df6b9+] with patch on sample certificates, please refer log is below: OpenSSL, --crl-verify sample-keys/ca.crl Wed Aug 5 17:18:49 2020 127.0.0.1:16001 VERIFY ERROR: depth=0, error=certificate revoked: C=KG, ST=NA, O=OpenVPN-TEST, CN=client-revoked, emailAddress=me@myhost.mydomain, serial=3 Wed Aug 5 17:18:49 2020 127.0.0.1:16001 OpenSSL: error:1417C086:SSL routines:tls_process_client_certificate:certificate verify failed Wed Aug 5 17:18:49 2020 127.0.0.1:16001 TLS_ERROR: BIO read tls_read_plaintext error mbedTLS, --crl-verify sample-keys/ca.crl Wed Aug 5 17:25:28 2020 127.0.0.1:16001 VERIFY OK: depth=1, C=KG, ST=NA, L=BISHKEK, O=OpenVPN-TEST, emailAddress=me@myhost.mydomain Wed Aug 5 17:25:28 2020 127.0.0.1:16001 VERIFY ERROR: depth=0, subject=C=KG, ST=NA, O=OpenVPN-TEST, CN=client-revoked, emailAddress=me@myhost.mydomain, serial=3: The certificate has been revoked (is on a CRL) Wed Aug 5 17:25:28 2020 127.0.0.1:16001 TLS_ERROR: read tls_read_plaintext error: X509 - Certificate verification failed, e.g. CRL, CA or signature check failed touch sample-keys/3, --crl-verify sample-keys/ dir Wed Aug 5 17:18:12 2020 127.0.0.1:16001 VERIFY OK: depth=1, C=KG, ST=NA, L=BISHKEK, O=OpenVPN-TEST, emailAddress=me@myhost.mydomain Wed Aug 5 17:18:12 2020 127.0.0.1:16001 VERIFY CRL: depth=0, C=KG, ST=NA, O=OpenVPN-TEST, CN=client-revoked, emailAddress=me@myhost.mydomain, serial=3 is revoked Wed Aug 5 17:18:12 2020 127.0.0.1:16001 OpenSSL: error:1417C086:SSL routines:tls_process_client_certificate:certificate verify failed Wed Aug 5 17:18:12 2020 127.0.0.1:16001 TLS_ERROR: BIO read tls_read_plaintext error -- Best Regards, Vladislav Grishenko -----Original Message----- From: Gert Doering <gert@greenie.muc.de> Sent: Wednesday, August 5, 2020 4:55 PM To: Vladislav Grishenko <themiron@yandex-team.ru> Cc: openvpn-devel@lists.sourceforge.net Subject: [PATCH applied] Re: Log serial number of revoked certificate Your patch has been applied to the master branch. I have not done much testing, just a test run "make check" on an OpenSSL and mbedTLS build. I have not looked into applying it to "release/2.4" - if you think it's needed, let me know (or if it needs more work because the code has diverged too much, send a 2.4 patch) - thanks. commit 992e9cec40539a155afa9eae10502aa62f617965 Author: Vladislav Grishenko Date: Wed Aug 5 15:23:33 2020 +0500 Log serial number of revoked certificate Signed-off-by: Vladislav Grishenko <themiron@yandex-team.ru> Acked-by: Lev Stipakov <lstipakov@gmail.com> Message-Id: <20200805102333.3109-1-themiron@yandex-team.ru> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20642.ht ml Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
Hi, On Wed, Aug 05, 2020 at 05:27:45PM +0500, Vladislav Grishenko wrote: > Thank you. > I'd appreciate if patch could be applied to release/2.4 too, no changes are > required - related code is the same, just hunks offset in ssl_verify.c and > ssl_verify_openssl.c > I've tested 2.4.9 [git:release/2.4/7c428ca19a8df6b9+] with patch on sample > certificates, please refer log is below: OK. Done. Thanks :-) commit 4ee2c1cd877b2e99b41fd248bf853329af825188 (HEAD -> release/2.4) gert
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index 844bc57d..97ccb93b 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -599,7 +599,8 @@ cleanup: * check peer cert against CRL directory */ static result_t -verify_check_crl_dir(const char *crl_dir, openvpn_x509_cert_t *cert) +verify_check_crl_dir(const char *crl_dir, openvpn_x509_cert_t *cert, + const char *subject, int cert_depth) { result_t ret = FAILURE; char fn[256]; @@ -607,6 +608,12 @@ verify_check_crl_dir(const char *crl_dir, openvpn_x509_cert_t *cert) struct gc_arena gc = gc_new(); char *serial = backend_x509_get_serial(cert, &gc); + if (!serial) + { + msg(D_HANDSHAKE, "VERIFY CRL: depth=%d, %s, serial number is not available", + cert_depth, subject); + goto cleanup; + } if (!openvpn_snprintf(fn, sizeof(fn), "%s%c%s", crl_dir, OS_SPECIFIC_DIRSEP, serial)) { @@ -616,7 +623,8 @@ verify_check_crl_dir(const char *crl_dir, openvpn_x509_cert_t *cert) fd = platform_open(fn, O_RDONLY, 0); if (fd >= 0) { - msg(D_HANDSHAKE, "VERIFY CRL: certificate serial number %s is revoked", serial); + msg(D_HANDSHAKE, "VERIFY CRL: depth=%d, %s, serial=%s is revoked", + cert_depth, subject, serial); goto cleanup; } @@ -758,7 +766,7 @@ verify_cert(struct tls_session *session, openvpn_x509_cert_t *cert, int cert_dep { if (opt->ssl_flags & SSLF_CRL_VERIFY_DIR) { - if (SUCCESS != verify_check_crl_dir(opt->crl_file, cert)) + if (SUCCESS != verify_check_crl_dir(opt->crl_file, cert, subject, cert_depth)) { goto cleanup; } diff --git a/src/openvpn/ssl_verify_mbedtls.c b/src/openvpn/ssl_verify_mbedtls.c index fd31bbbd..93891038 100644 --- a/src/openvpn/ssl_verify_mbedtls.c +++ b/src/openvpn/ssl_verify_mbedtls.c @@ -68,6 +68,7 @@ verify_callback(void *session_obj, mbedtls_x509_crt *cert, int cert_depth, int ret = 0; char errstr[512] = { 0 }; char *subject = x509_get_subject(cert, &gc); + char *serial = backend_x509_get_serial(cert, &gc); ret = mbedtls_x509_crt_verify_info(errstr, sizeof(errstr)-1, "", *flags); if (ret <= 0 && !openvpn_snprintf(errstr, sizeof(errstr), @@ -82,8 +83,8 @@ verify_callback(void *session_obj, mbedtls_x509_crt *cert, int cert_depth, if (subject) { - msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, subject=%s: %s", - cert_depth, subject, errstr); + msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, subject=%s, serial=%s: %s", + cert_depth, subject, serial ? serial : "<not available>", errstr); } else { diff --git a/src/openvpn/ssl_verify_openssl.c b/src/openvpn/ssl_verify_openssl.c index ff14db23..454efeec 100644 --- a/src/openvpn/ssl_verify_openssl.c +++ b/src/openvpn/ssl_verify_openssl.c @@ -71,6 +71,7 @@ verify_callback(int preverify_ok, X509_STORE_CTX *ctx) { /* get the X509 name */ char *subject = x509_get_subject(current_cert, &gc); + char *serial = backend_x509_get_serial(current_cert, &gc); if (!subject) { @@ -89,10 +90,10 @@ verify_callback(int preverify_ok, X509_STORE_CTX *ctx) } /* Remote site specified a certificate, but it's not correct */ - msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, error=%s: %s", + msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, error=%s: %s, serial=%s", X509_STORE_CTX_get_error_depth(ctx), X509_verify_cert_error_string(X509_STORE_CTX_get_error(ctx)), - subject); + subject, serial ? serial : "<not available>"); ERR_clear_error();
As it appears commit 767e4c56becbfeea525e4695a810593f373883cd "Log serial number of revoked certificate" hasn't survive refactoring of CRL handling. In most of situations admin of OpenVPN server needs to know which particular certificate is used by client. In the case when certificate is valid, environment variable can be used for that but once it is revoked, no user scripts are invoked so there is no way to get serial number, only subject is logged. Let's log certificate serial in case it is revoked and additionally log certificate depth & subject in crl-verify "dir" mode for better consistency with crl file (non-dir) mode. v2: log if serial is not availble, require it in crl-verify dir mode Signed-off-by: Vladislav Grishenko <themiron@yandex-team.ru> --- src/openvpn/ssl_verify.c | 14 +++++++++++--- src/openvpn/ssl_verify_mbedtls.c | 5 +++-- src/openvpn/ssl_verify_openssl.c | 5 +++-- 3 files changed, 17 insertions(+), 7 deletions(-)