[Openvpn-devel,v2] Log serial number of revoked certificate

Message ID 20200805102333.3109-1-themiron@yandex-team.ru
State Accepted
Headers show
Series
  • [Openvpn-devel,v2] Log serial number of revoked certificate
Related show

Commit Message

Vladislav Grishenko Aug. 5, 2020, 10:23 a.m.
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(-)

Comments

Lev Stipakov Aug. 5, 2020, 11:08 a.m. | #1
Code looks good. Compiled and tested (openssl and revoked cert) on Ubuntu 20.04.

Acked-by: Lev Stipakov <lstipakov@gmail.com>
Gert Doering Aug. 5, 2020, 11:54 a.m. | #2
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
Vladislav Grishenko Aug. 5, 2020, 12:27 p.m. | #3
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
Gert Doering Aug. 5, 2020, 12:41 p.m. | #4
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

Patch

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();