On Fri, 2019-08-16 at 13:45 +0200, David Sommerseth wrote:
> This gets a Feature-ACK from me. This is useful information, and
> something
> other users in the community have asked for earlier too. But there
> are a few
> things here before starting to dive into the details.
>
> First of all, we want to have patches first into git master, and then
> we need
> to discuss in the community if this feature is something we want to
> backport
> to the 2.4 release. After a new release has stabilized (which 2.4
> has), we
> are quite reluctant to add new features to those releases.
I started off by creating a pull request:
https://github.com/OpenVPN/openvpn/pull/129
During creation of the pull request I was pointed to the openvpn-devel
list, so I attached the patch there too. That one was based on 2.4,
because that's what we're using and how we're testing (and using) the
patch.
> Another thing is that I think it would be valuable to also print this
> information into the logs as well. The X509_get_notBefore() value is
> probably
> not so important unless that has a value which is in the future. The
> X509_get_notAfter() is fine to always log, but would be nice if it
> would come
> a M_WARN log entry if it has expired.
>
> To achieve this logging feature, setenv_ASN1_TIME() would need to be
> refactored a bit - possibly by returning a string as well as "is
> now() after
> the time stamp?" bool flag. The "printing" could happen to a
> gc_arena
> allocated buffer (which is available in verify_cert_set_env()). The
> logging
> should probably already happen in verify_cert(), which also has its
> own
> gc_arena. There are various alternatives to avoid doing the
> ASN1_TIME_print()
> preparations and processing multiple times (for logging and setenv),
> but I
> don't have a clear idea right now what could be a reasonable
> approach.
>
> And lastly, this code will break compilation if using
> ./configure --with-crypto-library=mbedtls ... This should also be
> improved.
>
I updated my pull request based on your feedback. I'm not sure if I
correcty understood the structure of the software, but I think it's a
decent attempt.
- The notAfter information is in the logs now (appended to the "VERIFY
OK" lines)
- Warnings are issued if the now is before notBefore of after notAfter
- openssl specifics are moved to ssl_verify_openssl.c.
ssl_verify_mbedtls.c has a dummy equivalent which should make openvpn
both compile and run.
Attached you'll find the updated patch too.
diff -ruN openvpn-2.4.7.orig/src/openvpn/ssl_verify_backend.h openvpn-2.4.7/src/openvpn/ssl_verify_backend.h
--- openvpn-2.4.7.orig/src/openvpn/ssl_verify_backend.h 2019-02-20 13:28:23.000000000 +0100
+++ openvpn-2.4.7/src/openvpn/ssl_verify_backend.h 2019-08-17 13:35:38.832574389 +0200
@@ -267,4 +267,25 @@
*/
bool tls_verify_crl_missing(const struct tls_options *opt);
+/*
+ * Get certificate notBefore and notAfter attributes
+ *
+ * @param cert Certificate to retrieve attributes from
+ * @param notsize Size of char buffers for notbefore and notafter
+ * @param notbefore Charachter representation of notBefore attribute
+ * @param cmpbefore Compare notBefore with "now"; > 0 if notBefore in the past
+ * @param notafter Character representation of notAfter attribute
+ * @param cmpafter Compare notAfter with "now"; > 0 if notAfter in the past
+ *
+ * On failing to retrieve notBefore attributes:
+ * - notbefore[0] = '\0'
+ * - cmpbefore = 0
+ *
+ * On failing to retrieve notAfter attributes:
+ * - notafter[0] = '\0'
+ * - cmpafter = 0
+ */
+
+void x509_get_validity(openvpn_x509_cert_t *cert, int notsize, char *notbefore, int *cmpbefore, char *notafter, int *cmpafter);
+
#endif /* SSL_VERIFY_BACKEND_H_ */
diff -ruN openvpn-2.4.7.orig/src/openvpn/ssl_verify.c openvpn-2.4.7/src/openvpn/ssl_verify.c
--- openvpn-2.4.7.orig/src/openvpn/ssl_verify.c 2019-02-20 13:28:23.000000000 +0100
+++ openvpn-2.4.7/src/openvpn/ssl_verify.c 2019-08-17 13:39:43.229250136 +0200
@@ -447,6 +447,17 @@
return SUCCESS;
}
+static void
+setenv_validity (struct env_set *es, char *envprefix, int depth, char *dt)
+{
+ char varname[32];
+
+ if (!dt[0]) return;
+
+ openvpn_snprintf(varname, sizeof(varname), "%s_%d", envprefix, depth);
+ setenv_str(es, varname, dt);
+}
+
/*
* Export the subject, common_name, and raw certificate fields to the
* environment for later verification by scripts and plugins.
@@ -673,6 +684,8 @@
char common_name[TLS_USERNAME_LEN+1] = {0}; /* null-terminated */
const struct tls_options *opt;
struct gc_arena gc = gc_new();
+ char notbefore_buf[32], notafter_buf[32];
+ int notbefore_cmp, notafter_cmp;
opt = session->opt;
ASSERT(opt);
@@ -767,6 +780,13 @@
/* export current untrusted IP */
setenv_untrusted(session);
+ x509_get_validity(cert, sizeof (notbefore_buf), notbefore_buf, ¬before_cmp, notafter_buf, ¬after_cmp);
+ setenv_validity (opt->es, "tls_notbefore", cert_depth, notbefore_buf);
+ setenv_validity (opt->es, "tls_notafter", cert_depth, notafter_buf);
+
+ if (notbefore_cmp < 0) msg(M_WARN, "Certificate notBefore (%s)", notbefore_buf);
+ if (notafter_cmp > 0) msg(M_WARN, "Certificate notAfter (%s)", notafter_buf);
+
/* If this is the peer's own certificate, verify it */
if (cert_depth == 0 && SUCCESS != verify_peer_cert(opt, cert, subject, common_name))
{
@@ -806,7 +826,8 @@
}
}
- msg(D_HANDSHAKE, "VERIFY OK: depth=%d, %s", cert_depth, subject);
+ msg(D_HANDSHAKE, "VERIFY OK: depth=%d, %s, notAfter=%s", cert_depth, subject,
+ (notafter_buf[0] ? notafter_buf : "-"));
session->verified = true;
ret = SUCCESS;
diff -ruN openvpn-2.4.7.orig/src/openvpn/ssl_verify_mbedtls.c openvpn-2.4.7/src/openvpn/ssl_verify_mbedtls.c
--- openvpn-2.4.7.orig/src/openvpn/ssl_verify_mbedtls.c 2019-02-20 13:28:23.000000000 +0100
+++ openvpn-2.4.7/src/openvpn/ssl_verify_mbedtls.c 2019-08-17 13:23:46.250827837 +0200
@@ -550,4 +550,14 @@
return false;
}
+void
+x509_get_validity(mbedtls_x509_crt *cert, int notsize, char *notbefore, int *cmpbefore, char *notafter, int *cmpafter)
+{
+ notbefore[0] = '\0';
+ notafter[0] = '\0';
+
+ *cmpbefore = 0;
+ *cmpafter = 0;
+}
+
#endif /* #if defined(ENABLE_CRYPTO) && defined(ENABLE_CRYPTO_MBEDTLS) */
diff -ruN openvpn-2.4.7.orig/src/openvpn/ssl_verify_openssl.c openvpn-2.4.7/src/openvpn/ssl_verify_openssl.c
--- openvpn-2.4.7.orig/src/openvpn/ssl_verify_openssl.c 2019-02-20 13:28:23.000000000 +0100
+++ openvpn-2.4.7/src/openvpn/ssl_verify_openssl.c 2019-08-17 13:36:58.222439208 +0200
@@ -802,4 +802,35 @@
return true;
}
+static int
+get_ASN1_TIME(const ASN1_TIME *asn1_time, char *dt, int dtsize, int *cmpnow)
+{
+ BIO *mem;
+ int ret, pday, psec;
+
+ mem = BIO_new(BIO_s_mem());
+ if ((ret = ASN1_TIME_print (mem, asn1_time))) {
+ dt[BIO_read(mem, dt, dtsize-1)] = '\0';
+ }
+ BIO_free(mem);
+ if (!ret) goto fail;
+
+ if (!ASN1_TIME_diff(&pday, &psec, asn1_time, NULL)) goto fail;
+ *cmpnow = (pday ? pday : psec);
+
+ return 1;
+
+fail:
+ dt[0] = '\0';
+ *cmpnow = 0;
+ return 0;
+}
+
+void
+x509_get_validity(X509 *cert, int notsize, char *notbefore, int *cmpbefore, char *notafter, int *cmpafter)
+{
+ get_ASN1_TIME(X509_get_notBefore(cert), notbefore, notsize, cmpbefore);
+ get_ASN1_TIME(X509_get_notAfter(cert), notafter, notsize, cmpafter);
+}
+
#endif /* defined(ENABLE_CRYPTO) && defined(ENABLE_CRYPTO_OPENSSL) */
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
