Re: [Openvpn-devel] [PATCH] Disable certificate notBefore/notAfter sanity check on OpenSSL < 1.0.2

[Steffan Karger]

Hi,

On Tue, Dec 15, 2015 at 6:24 PM, Jan Just Keijser  wrote:
> 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

Yes that was my first attempt to work around it, but ctx->cert is an
opaque struct cert_st (declared in ssl.h, but defined in ssl_locl.h,
which is not in include/openssl/).

-Steffan

Re: [Openvpn-devel] [PATCH] Disable certificate notBefore/notAfter sanity check on OpenSSL < 1.0.2

[Jan Just Keijser]

Hi,

On 15/12/15 13:21, Steffan Karger wrote:

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?

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 :)


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


cheers,

JJK

Re: [Openvpn-devel] [PATCH] Disable certificate notBefore/notAfter sanity check on OpenSSL < 1.0.2

[Jan Just Keijser]

Hi,

On 15/12/15 10:12, Steffan Karger wrote:

Hi,

On Tue, Dec 15, 2015 at 9:42 AM, Jan Just Keijser  wrote:

On 14/12/15 23:14, Steffan Karger wrote:

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?

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 :)

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?


JJK

Re: [Openvpn-devel] [PATCH] Disable certificate notBefore/notAfter sanity check on OpenSSL < 1.0.2

[Steffan Karger]

Hi,

On Tue, Dec 15, 2015 at 9:42 AM, Jan Just Keijser  wrote:
> On 14/12/15 23:14, Steffan Karger wrote:
>> 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?

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 :)

-Steffan

Re: [Openvpn-devel] [PATCH] Disable certificate notBefore/notAfter sanity check on OpenSSL < 1.0.2

[Jan Just Keijser]

Hi,

On 14/12/15 23:14, Steffan Karger wrote:

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?


cheers,

JJK



Signed-off-by: Steffan Karger 
---
  src/openvpn/ssl_openssl.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 2b74818..4792b08 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -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

[Openvpn-devel] [PATCH] Disable certificate notBefore/notAfter sanity check on OpenSSL < 1.0.2

[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.

Signed-off-by: Steffan Karger 
---
 src/openvpn/ssl_openssl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 2b74818..4792b08 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -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
-- 
2.5.0