Disable certificate notBefore/notAfter sanity check on OpenSSL < 1.0.2

The SSL_CTX_get0_certificate() function I used in 091edd8e is available in OpenSSL 1.0.2+ only. Older versions seem to not have a useful alternative. The remaining option would then be to create a cache for our parsed certificate, but that would mean adding more struct members and code for the select group of people that do use an up-to-date openvpn, but do not update their openssl. I don't think that's worth it. So just disable the code for older openssl versions.

Signed-off-by Steffan Karger <[email protected]...>

---
  src/openvpn/ssl_openssl.c | 2 ++
  1 file changed, 2 insertions(+)

src/openvpn/ssl_openssl.c +2 -0

index 2b74818..4792b08 100644
@@ -353,6 +353,7 @@ tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
  void
  tls_ctx_check_cert_time (const struct tls_root_ctx *ctx)
  {
+#if OPENSSL_VERSION_NUMBER >= 0x10002000L
    int ret;
    const X509 *cert = SSL_CTX_get0_certificate(ctx->ctx);
 

@@ -375,6 +376,7 @@ tls_ctx_check_cert_time (const struct tls_root_ctx *ctx)
      {
        msg (M_WARN, "WARNING: Your certificate has expired!");
      }
+#endif
  }
 
  void

Quoted 8 lines · Steffan Karger

The SSL_CTX_get0_certificate() function I used in 091edd8e is available in OpenSSL 1.0.2+ only. Older versions seem to not have a useful alternative.
The remaining option would then be to create a cache for our parsed certificate, but that would mean adding more struct members and code for the select group of people that do use an up-to-date openvpn, but do not update their openssl. I don't think that's worth it. So just disable the code for older openssl versions.

I have code lying around for checking certificate dates for openssl v0.9.7+ ; you can find it here:
https://www.nikhef.nl/~janjust/proxy-verify/

the function of interest is grid_asn1TimeToTimeT ; it was/is on my TODO list to convert this code into a similar patch - perhaps we can integrate the two?

Hi,

Quoted 9 lines · Jan Just Keijser

Quoted 8 lines · Steffan Karger

The SSL_CTX_get0_certificate() function I used in 091edd8e is available in OpenSSL 1.0.2+ only. Older versions seem to not have a useful alternative.
The remaining option would then be to create a cache for our parsed certificate, but that would mean adding more struct members and code for the select group of people that do use an up-to-date openvpn, but do not update their openssl. I don't think that's worth it. So just disable the code for older openssl versions.

Quoted 7 lines

I have code lying around for checking certificate dates for openssl v0.9.7+ ; you can find it here:
https://www.nikhef.nl/~janjust/proxy-verify/

the function of interest is grid_asn1TimeToTimeT ; it was/is on my TODO list to convert this code into a similar patch - perhaps we can integrate the two?

But before we extract the time from the certificate, we need to either cache our own x509 certificate (in the certificate file reading code, the pkcs11 code, the management-external-key code, the ms crapi code, etc...) or find a way to extract our own x509 cert from an SSL_CTX (which SSL_CTX_get0_certificate() does, since from that part of the code it *can* peek into the opaque 'struct cert_st').

Looking at the mess, I still think it is just not worth the extra code. But if you (or someone else) manage to find a clean and simple way to perform the check pre-1.0.2, I will gladly review a patch :)

Quoted 28 lines

Quoted 16 lines

Quoted 10 lines

The SSL_CTX_get0_certificate() function I used in 091edd8e is available in
OpenSSL 1.0.2+ only. Older versions seem to not have a useful alternative.
The remaining option would then be to create a cache for our parsed certificate, but that would mean adding more struct members and code for the select group of people that do use an up-to-date openvpn, but do not update their openssl. I don't think that's worth it. So just disable the
code for older openssl versions.

I have code lying around for checking certificate dates for openssl v0.9.7+
; you can find it here:
https://www.nikhef.nl/~janjust/proxy-verify/

Quoted 4 lines

the function of interest is grid_asn1TimeToTimeT ; it was/is on my TODO list
to convert this code into a similar patch - perhaps we can integrate the two?

But before we extract the time from the certificate, we need to either cache our own x509 certificate (in the certificate file reading code, the pkcs11 code, the management-external-key code, the ms crapi code, etc...) or find a way to extract our own x509 cert from an SSL_CTX (which SSL_CTX_get0_certificate() does, since from that part of the code it *can* peek into the opaque 'struct cert_st').

Quoted 4 lines

Looking at the mess, I still think it is just not worth the extra code. But if you (or someone else) manage to find a clean and simple way to perform the check pre-1.0.2, I will gladly review a patch :)

err, isn't it much easier to check the certificate expiry date when loading the cert, e.g. in "tls_ctx_load_cert_file_and_copy" in ssl_openssl.c ? or am I missing something here?

Quoted 30 lines

Quoted 30 lines

Quoted 15 lines

Quoted 10 lines

The SSL_CTX_get0_certificate() function I used in 091edd8e is available in
OpenSSL 1.0.2+ only. Older versions seem to not have a useful alternative.
The remaining option would then be to create a cache for our parsed certificate, but that would mean adding more struct members and code for the select group of people that do use an up-to-date openvpn, but do not update their openssl. I don't think that's worth it. So just disable the
code for older openssl versions.

I have code lying around for checking certificate dates for openssl v0.9.7+
; you can find it here:
https://www.nikhef.nl/~janjust/proxy-verify/

Quoted 4 lines

the function of interest is grid_asn1TimeToTimeT ; it was/is on my TODO list
to convert this code into a similar patch - perhaps we can integrate the two?

But before we extract the time from the certificate, we need to either cache our own x509 certificate (in the certificate file reading code, the pkcs11 code, the management-external-key code, the ms crapi code, etc...) or find a way to extract our own x509 cert from an SSL_CTX (which SSL_CTX_get0_certificate() does, since from that part of the code it *can* peek into the opaque 'struct cert_st').

Looking at the mess, I still think it is just not worth the extra code. But if you (or someone else) manage to find a clean and simple way to perform the check pre-1.0.2, I will gladly review a patch :)

Quoted 3 lines

err, isn't it much easier to check the certificate expiry date when loading the cert, e.g. in "tls_ctx_load_cert_file_and_copy" in ssl_openssl.c ? or am I missing something here?

Yes, we could do that. But we would have to do that for both crypto libraries separately, and at each place where we load certificates (same list as above; file, pkcs#11, ms crapi, management-external-cert, maybe more?). Not as nice as a single check after all certificate processing.

ah well, in that case I would simply write out get0_certificate again: the code for that function actually is:

3011 X509 *SSL_CTX_get0_certificate(const SSL_CTX *ctx) 3012 {
3013 if (ctx->cert != NULL)
3014 return ctx->cert->key->x509;
3015 else
3016 return NULL;
3017 }

and as far as I can tell that "struct-path" is also valid for openssl < 1.0.2