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
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
