Re: [Openvpn-devel] Summary of the IRC meeting (9th Dec 2010)

[David Sommerseth]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 10/12/10 20:30, Markus Kötter wrote:
> Hi,
> 
> Samuli Seppänen wrote:
>> 
>> In deeper analysis it was noted that this feature, or using an external
>> CA for OpenVPN certs in general, may be dangerous. Consider the
>> following scenario:
>>
>> - An external root CA (e.g. Verisign) is used for OpenVPN certificates,
>>   with or without an internal sub-CA
>> - A certificate field such as subjectAltName contains an email address
>>   and is used as common_name in OpenVPN
>>
>> Even if a sub-CA is included in the certificate signing chain, it's
>> likely that OpenVPN/OpenSSL will happily accept certificates signed
>> _only_ by the external root CA. So, a malicious person could connect to
>> the VPN by having a certificate with a faked email address signed by the
>> external CA.
> 
> I had a chat with my PKI oracles on this, and from what I got you are
> correct.
> So you do not only have to trust your CA, but all sub-CA's of your CA.
> As you pay your CA for this, and your CA has every sub-CA, including
> you, sign agreements on the nature of the certificates created, this is
> not considered a problem here.
> 
> If you do not or can not trust your CA, it makes no difference if you
> use an email address extracted from via "extv3:altSubjectName" "email"
> or the default CN for authentication.
> 
>> This feature could be implemented more safely as a separate plugin using
>> the new v3 plug-in API, as that will provide full access to the X509
>> certificate object in C code. Dazo sent the second version of the v3
>> plug-in API to openvpn-devel mailing list today.
> 
> I need the email address from extv3 altSubjectName for the
> connect-script userland script, as the address assignment relies on it.
> Currently this script is pretty easy, some lines of python querying a
> sqlite database, I doubt this will get any easier or secure by writing
> this in c.
> Of course, it may be possible to change the internal common_name in a v3
> plugin instead of patching openvpn to take care, and do
> authentication/authorization in the plugin too, but it would be way more
> complex/error pront to accomplish the same goal using the same path.
> 
> I have to authenticate/authorize users based on their valid certificate
> and the email address from the altSubjectName field.
> It is useful to have the logfiles use the email address as well for the
> internal 'common_name' of the client, it is just consistent.
> As the CN is not meant to be uniq by definition, I may run into CN
> collisions if openvpn uses the CN internally, even though there is no
> collision as it is different users with the same CN but different
> certificates.
> 
> If you are part of a larger PKI and want use this PKI for openvpn
> certificates, you'll need something slightly more uniq than the CN of
> the certificates for authentication/authorization as you can't start
> changing the names in your users birth certificates for technical reasons.
> 
> I can't ask for a separate PKI for openvpn.
> There is no reason, we can and have to rely on our PKI's integrity,
> thats what it exists for, thats what we pay for.
> Our users already have certificates which are used to authenticate via
> https or sign emails, asking for 'a special certificate for vpn' is not
> an option, not even mentioning 'a special smartcard for the vpn'.
> 
> That said, I disagree with the decision to put this down.

I've been silent in this discussion lately, to spend some more time
thinking about it.  And I do see some dilemmas here.

First of all, this patch from Markus extends a feature we incorporated
into the feat_misc branch earlier this summer (commit
935c62be9c0c8a256112) which is moved over to beta2.2 (commit
2e8337de248ef0b5b48cbb2964).  This feature is not an opt-in feature, as
James Yonan mentioned he would like this extension from Markus to be.

Due to the fact that we have accepted --x509-username-field, I do see it
as much more difficult to reject this patch from Markus.  It would
really not make sense, as the points which have been discussed in
regards to this last patch, is just as relevant to the core
- --x509-username-field feature.

Also concerning JJK's findings in regards to CA and sub-CA's, it also
indicates that this feature neither strengthen nor weaken the current
core implementation of SSL certificates in OpenVPN.  But JJK's findings
shows that careless configuration CA certificates in OpenVPN can weaken
the overall security on that particular server.  So these issues are not
related to --x509-username-field at all.

However, as James wanted Markus' patch as an opt-in compile time
configuration, I find that rather odd - taken --x509-username-field in
consideration.  So I wrote the attached patch which will make the whole
- --x509-username-field and compile-time opt-in feature instead.  Thus
covering this feature, including additional extensions of it.

Re: [Openvpn-devel] Summary of the IRC meeting (9th Dec 2010)

[Jan Just Keijser]

Hi Adriaan,

Adriaan de Jong wrote:
  

-Original Message-
From: Jan Just Keijser [mailto:janj...@nikhef.nl]
Hi Samuli, David, list,

What some people get confused about is a stacked certificate vs a
certificate chain: OpenVPN only supports stacked CA certificates,
meaning that any of the certs present in the stacked file are
considered
trusted.
What some people want is a certificate chain, like some websites use:
the website has a server certificate signed by a sub-CA which in turn
is
signed by Verisign. The webserver sends the entire chain (server cert +
sub-CA cert + verisign cert) to the client for verification. That way
the client only needs the Verisign cert to trust a particular website
(the Verisign CA cert is installed in most browser by default). OpenVPN
does not support this at the moment. It would require changing
  SSL_CTX_use_certificate()
to
  SSL_CTX_use_certificate_chain()
to ensure that the entire certificate chain is sent to the remote end
(client or server).



I think SSL_CTX_use_certificate_chain_file() is actually used when loading the 
certificate from a separate cert file. It's just slightly confusing due to the 
fact that it is loaded twice:

- SSL_CTX_use_certificate_file is used here:
http://openvpn.git.sourceforge.net/git/gitweb.cgi?p=openvpn/openvpn-testing.git;a=blob;f=ssl.c;h=a1268ac2a9291dc1512fa28e3ab4efc65085c952;hb=HEAD#l1639

- SSL_CTX_use_certificate_chain_file is used here:
  
http://openvpn.git.sourceforge.net/git/gitweb.cgi?p=openvpn/openvpn-testing.git;a=blob;f=ssl.c;h=a1268ac2a9291dc1512fa28e3ab4efc65085c952;hb=HEAD#l1738

I'm not entirely sure whether I'm right, and why things are done this way. Does 
anyone know the reasoning behind this?

  


ah mea culpa - I did not read the rest of the init_ssl function. It does 
seem a bit odd though:


this happens first (2.1.3 code base):
1623   if (!SSL_CTX_use_certificate_file (ctx, 
options->cert_file, SSL_FILETYPE_PEM))
1624 msg (M_SSLERR, "Cannot load certificate file %s", 
options->cert_file);

1625   using_cert_file = true;

so we load the cert and we set 'using_cert_file = true' . Then later:
1721   /* Enable the use of certificate chains */
1722   if (using_cert_file)
1723 {
1724   if (!SSL_CTX_use_certificate_chain_file (ctx, 
options->cert_file))
1725 msg (M_SSLERR, "Cannot load certificate chain file %s 
(SSL_use_certificate_chain_file)", options->cert_file);

1726 }

so the certificate is loaded again but now using _use_certificate_chain 
! It does suggest that you can use certificate chains , however.


I just tested a setup where the client cert is a concatenation of the 
client cert + intermediate CA. The client sends these certs to a server 
which only trusts the "root" CA (i.e. the one that signed the 
intermediate CA to begin with) but I cannot connect. It seems the 
'verify_callback' function does not work with certificate chains (yet) : 
only the top level cert is inspected.


to be continued...

cheers,

JJK

Re: [Openvpn-devel] Summary of the IRC meeting (9th Dec 2010)

[Adriaan de Jong]

> -Original Message-
> From: Jan Just Keijser [mailto:janj...@nikhef.nl]
> Hi Samuli, David, list,
> 
> What some people get confused about is a stacked certificate vs a
> certificate chain: OpenVPN only supports stacked CA certificates,
> meaning that any of the certs present in the stacked file are
> considered
> trusted.
> What some people want is a certificate chain, like some websites use:
> the website has a server certificate signed by a sub-CA which in turn
> is
> signed by Verisign. The webserver sends the entire chain (server cert +
> sub-CA cert + verisign cert) to the client for verification. That way
> the client only needs the Verisign cert to trust a particular website
> (the Verisign CA cert is installed in most browser by default). OpenVPN
> does not support this at the moment. It would require changing
>   SSL_CTX_use_certificate()
> to
>   SSL_CTX_use_certificate_chain()
> to ensure that the entire certificate chain is sent to the remote end
> (client or server).

I think SSL_CTX_use_certificate_chain_file() is actually used when loading the 
certificate from a separate cert file. It's just slightly confusing due to the 
fact that it is loaded twice:

- SSL_CTX_use_certificate_file is used here:
http://openvpn.git.sourceforge.net/git/gitweb.cgi?p=openvpn/openvpn-testing.git;a=blob;f=ssl.c;h=a1268ac2a9291dc1512fa28e3ab4efc65085c952;hb=HEAD#l1639

- SSL_CTX_use_certificate_chain_file is used here:
  
http://openvpn.git.sourceforge.net/git/gitweb.cgi?p=openvpn/openvpn-testing.git;a=blob;f=ssl.c;h=a1268ac2a9291dc1512fa28e3ab4efc65085c952;hb=HEAD#l1738

I'm not entirely sure whether I'm right, and why things are done this way. Does 
anyone know the reasoning behind this?

Adriaan

Re: [Openvpn-devel] Summary of the IRC meeting (9th Dec 2010)

[Markus Kötter]

Hi,

Samuli Seppänen wrote:


In deeper analysis it was noted that this feature, or using an external
CA for OpenVPN certs in general, may be dangerous. Consider the
following scenario:

- An external root CA (e.g. Verisign) is used for OpenVPN certificates,
  with or without an internal sub-CA
- A certificate field such as subjectAltName contains an email address
  and is used as common_name in OpenVPN

Even if a sub-CA is included in the certificate signing chain, it's
likely that OpenVPN/OpenSSL will happily accept certificates signed
_only_ by the external root CA. So, a malicious person could connect to
the VPN by having a certificate with a faked email address signed by the
external CA.


I had a chat with my PKI oracles on this, and from what I got you are
correct.
So you do not only have to trust your CA, but all sub-CA's of your CA.
As you pay your CA for this, and your CA has every sub-CA, including
you, sign agreements on the nature of the certificates created, this is
not considered a problem here.

If you do not or can not trust your CA, it makes no difference if you
use an email address extracted from via "extv3:altSubjectName" "email"
or the default CN for authentication.


This feature could be implemented more safely as a separate plugin using
the new v3 plug-in API, as that will provide full access to the X509
certificate object in C code. Dazo sent the second version of the v3
plug-in API to openvpn-devel mailing list today.


I need the email address from extv3 altSubjectName for the
connect-script userland script, as the address assignment relies on it.
Currently this script is pretty easy, some lines of python querying a
sqlite database, I doubt this will get any easier or secure by writing
this in c.
Of course, it may be possible to change the internal common_name in a v3
plugin instead of patching openvpn to take care, and do
authentication/authorization in the plugin too, but it would be way more
complex/error pront to accomplish the same goal using the same path.

I have to authenticate/authorize users based on their valid certificate
and the email address from the altSubjectName field.
It is useful to have the logfiles use the email address as well for the
internal 'common_name' of the client, it is just consistent.
As the CN is not meant to be uniq by definition, I may run into CN
collisions if openvpn uses the CN internally, even though there is no
collision as it is different users with the same CN but different
certificates.

If you are part of a larger PKI and want use this PKI for openvpn
certificates, you'll need something slightly more uniq than the CN of
the certificates for authentication/authorization as you can't start
changing the names in your users birth certificates for technical reasons.

I can't ask for a separate PKI for openvpn.
There is no reason, we can and have to rely on our PKI's integrity,
thats what it exists for, thats what we pay for.
Our users already have certificates which are used to authenticate via
https or sign emails, asking for 'a special certificate for vpn' is not
an option, not even mentioning 'a special smartcard for the vpn'.

That said, I disagree with the decision to put this down.

Given the requirement to cooperate with existing infrastructure, I'd
rather look into how to make it easier to use openvpn with existing CA
signed sub-CA PKI's and think about adding support for ocsp verification.


I've attached the most recent version of the patch which was
acknowledged code-wise yesterday, but rejected for the cited reasons,
just in case somebody faces similar problems.


MfG
Markus

diff --git a/options.c b/options.c
index d0cec7d..1e3fc6e 100644
--- a/options.c
+++ b/options.c
@@ -5647,7 +5647,8 @@ add_option (struct options *options,
 {
   char *s = p[1];
   VERIFY_PERMISSION (OPT_P_GENERAL);
-  while ((*s = toupper(*s)) != '\0') s++; /* Uppercase if necessary */
+  if( strncmp ("ext:",s,4) != 0 )
+while ((*s = toupper(*s)) != '\0') s++; /* Uppercase if necessary */
   options->x509_username_field = p[1];
 }
 #endif /* USE_SSL */
diff --git a/ssl.c b/ssl.c
index 8644ae4..8f30c6a 100644
--- a/ssl.c
+++ b/ssl.c
@@ -489,6 +489,60 @@ extract_x509_field_ssl (X509_NAME *x509, const char *field_name, char *out, int
   }
 }

+static 
+bool extract_x509_extension(X509 *cert, char *fieldname, char *out, int size)
+{
+  bool retval = false;
+  X509_EXTENSION *pExt;
+  char *buf = 0;
+  int length = 0;
+  GENERAL_NAMES *extensions;
+  int nid = OBJ_txt2nid(fieldname);
+
+  extensions = (GENERAL_NAMES *)X509_get_ext_d2i(cert, nid, NULL, NULL);
+  if ( extensions )
+{
+  int numalts;
+  int i;
+  /* get amount of alternatives, 
+   * RFC2459 claims there MUST be at least 
+   * one, but we don't depend on it... 
+   */
+
+  numalts = sk_GENERAL_NAME_num(extensions);
+
+  /* loop through all alternatives */
+  for