On 16/08/2019 10:27, Rolf Fokkens via Openvpn-devel wrote:
> We're considering to use shorter-lived client certificates for our VPN
> users. In an effort to prevent negative impact for our staff due to
> expired certificates, we 'd like to keep track of imminent expiration
> of certificates in the client-connect script (which we're using anyway
> to check is the certificate matches the user id). Many certificate
> attributes are passed to the script, but not the "NotAfter" and
> "NotBefore" attributes.
> The attached patch adds these to the mix.

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.

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.

Other than that, the code looks reasonable at first glance (I have not compile
tested it yet)

kind regards,

David Sommerseth
OpenVPN Inc

Attachment: signature.asc
Description: OpenPGP digital signature

Openvpn-devel mailing list

Reply via email to