Re: [Openvpn-devel] [PATCH v3] Mac OS X Keychain management client

[Gert Doering]

Hi,

On Mon, Feb 23, 2015 at 05:40:11PM +0300, Vasily Kulikov wrote:
> > I agree -- the argument to --needs-external-cert should be optional.
> 
> Note: Arne said about 'macos-keychain' prefix in the argument being
> optional, not the argument itself being optional.  Acually, I don't
> think making the argument optional is a good idea -- its parsing would
> be ambiguous unless it is the last argument in argv.

Oh, *that* is something our config parser does all the time :-)

The question to me is "if we don't want to specify anything particular,
because there is only one cert anyway, how would we do it?" - or is the
assumption that we better should specify it always?

[..]
> > I'm not sure exactly how to add an argument to RSA_SIGN and
> > NEEDS-CERTIFICATE without breaking existing management interface
> > software but assume that is possible. (Also, the argument may need to
> > be escaped when it is passed to RSA_SIGN or NEEDS-CERTIFICATE if it
> > contains characters that are used as delimiters.)
> 
> IMNSHO don't change rsa-sign at all and have no API breakage.

That sounds like a reasonable approach to me.

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


pgpvLuH0I4ise.pgp
Description: PGP signature

Re: [Openvpn-devel] [PATCH v3] Mac OS X Keychain management client

[Vasily Kulikov]

On Mon, Feb 23, 2015 at 12:55 +, David Woodhouse wrote:
> On Mon, 2015-02-23 at 09:28 +0100, Arne Schwabe wrote:
> > 
> > Am 23.02.15 um 09:04 schrieb Vasily Kulikov:
> > > management-external-cert 'macosx-keychain:SUBJECT:c=US'
> > >
> > > With the approach in patch v3 a user has to start openvpn with the
> > > config file, start keychain-mcd, and pass identity template as an
> > > argument to keychain-mcd.
> > >
> > > What do you think of the change?
> > I like the idea. You could  make the macos-keychain in the string optional.
> 
> I wouldn't make it optional. Keep it URI-like, with 'macos-keychain' as
> the URI scheme.

I'll keep it.  I don't have a strict understanding what is the purpose
of a default scheme, so I don't declare any default.

Also please note that openvpn itself doesn't know about any "scheme", it
simply passes the value to a management interface client.  A client is the
process which parses the value and must know about schemes, default
scheme, etc.

Thanks,

-- 
Vasily Kulikov
http://www.openwall.com - bringing security into open computing environments

Re: [Openvpn-devel] [PATCH v3] Mac OS X Keychain management client

[Vasily Kulikov]

On Mon, Feb 23, 2015 at 08:04 -0500, Jonathan K. Bullard wrote:
> On Mon, Feb 23, 2015 at 4:00 AM, Gert Doering  wrote:
> >
> > On Mon, Feb 23, 2015 at 09:28:31AM +0100, Arne Schwabe wrote:
> > > > What do you think of the change?
> > > I like the idea. You could  make the macos-keychain in the string 
> > > optional.
> >
> > What Arne said (both parts of it) :-)
> 
> I agree -- the argument to --needs-external-cert should be optional.

Note: Arne said about 'macos-keychain' prefix in the argument being
optional, not the argument itself being optional.  Acually, I don't
think making the argument optional is a good idea -- its parsing would
be ambiguous unless it is the last argument in argv.

> Note: the argument to --needs-external-cert should be passed on to
> "RSA_SIGN", too. (I think Vasily omitted that from his writeup.)
> 
> So the idea would be:
> 
>  * Add an optional UTF-8 string argument to --needs-external-cert.
> (Perhaps the docs should say this requires support from the management
> interface software and that currently such support is only available
> when using certain GUIs on OS X.)

Right.

>  * OpenVPN passes that argument to RSA_SIGN and NEEDS-CERTIFICATE,
> passing an empty string if the argument does not appear.

Why rsa-sign?  I think doing it with NEEDS-CERTIFICATE is enough.
Identity contains both cert and private key, certificate request is
made at first.  Rsa-sign uses already chosen identity.

>  * OS X GUIs such as Tunnelblick and Viscosity see the new RSA_SIGN or
> NEEDS-CERTIFICATE argument and use keychain-mcd to deal with it. Other
> GUIs ignore it or use something that does something equivalent to what
> keychain-mcd does on OS X.

Right.

> I'm not sure exactly how to add an argument to RSA_SIGN and
> NEEDS-CERTIFICATE without breaking existing management interface
> software but assume that is possible. (Also, the argument may need to
> be escaped when it is passed to RSA_SIGN or NEEDS-CERTIFICATE if it
> contains characters that are used as delimiters.)

IMNSHO don't change rsa-sign at all and have no API breakage.

-- 
Vasily Kulikov
http://www.openwall.com - bringing security into open computing environments

Re: [Openvpn-devel] [PATCH v3] Mac OS X Keychain management client

[Jonathan K. Bullard]

On Mon, Feb 23, 2015 at 8:10 AM, David Woodhouse  wrote:
> On Mon, 2015-02-23 at 13:59 +0100, Arne Schwabe wrote:
>>
>> All fine. My rationale was like, if I want a certificate with a certain
>> SUBJECT (e.g. CN=schw...@mycoolca.com) etc. it should not matter for men
>> wether I get it from OS X, Windows or Android Certificate store.
>
> The canonical way of representing that would be
>  pkcs11:object=schw...@mycoolca.com

That representation loses the "CN=" (and maybe doesn't allow for
additional search options -- I'm not sure).

OpenVPN already has an entire set of options that deal with PKCS-11.
This patch is a completely different, parallel way of dealing with
certificates, so I'm not sure why a "pkcs11:" scheme is needed. I
probably would not implement it in Tunnelblick myself.

There are two things here: (A) what this "select" string is and (B)
where/how to find the cert.

As to "what this 'select' string is" -- well, for "future-proofing", I
would want it to be labeled as a selection string in case we want to
pass something else at some time in the future.

Adding a scheme makes sense to me -- the "file:" scheme looks
interesting, but it isn't part of this patch. But I think the scheme
should be optional, so the configuration could include (the
platform-independent):

 --management-external-keychain select:CN=schw...@mycoolca.com

and have it use the Keychain on OS X, Android Certificate Store on
Android, something-I-don't-know on Windows, etc.

The tricky aspect of that is that the default would really depend on
the management interface software, which really means it depends on
the GUI. It is conceivable that two GUIs for the same platform could
have different defaults (although I assume both Tunnelblick and
Viscosity would use Keychain as the default, accessing it with
keychain-mcd).

Re: [Openvpn-devel] [PATCH v3] Mac OS X Keychain management client

[Steffan Karger]

On 02/23/2015 02:10 PM, David Woodhouse wrote:
> On Mon, 2015-02-23 at 13:59 +0100, Arne Schwabe wrote:
>>
>> All fine. My rationale was like, if I want a certificate with a certain
>> SUBJECT (e.g. CN=schw...@mycoolca.com) etc. it should not matter for men
>> wether I get it from OS X, Windows or Android Certificate store.
> 
> The canonical way of representing that would be
>  pkcs11:object=schw...@mycoolca.com

But that implies that pkcs11 is somehow in the loop. I might (1) use a
different mechanism, such as a separate daemon that directly uses a
private key file (as is possible on Android), or (2) simply don't care
for what mechanism my daemon uses.

That said, I *do* agree that it would be worthwhile to see if we can use
the same URI format as pkcs11-uris (most recent draft:
https://tools.ietf.org/html/draft-pechanec-pkcs11uri-21). If I recall
correctly, there even are pkcs11-uri parsing libs available.

I think omitting 'pkcs11:' or 'osxkeychain:' should be possible, and if
omitted the backend application should just use whatever it prefers.

-Steffan

Re: [Openvpn-devel] [PATCH v3] Mac OS X Keychain management client

[David Woodhouse]

On Mon, 2015-02-23 at 13:59 +0100, Arne Schwabe wrote:
> 
> All fine. My rationale was like, if I want a certificate with a certain
> SUBJECT (e.g. CN=schw...@mycoolca.com) etc. it should not matter for men
> wether I get it from OS X, Windows or Android Certificate store.

The canonical way of representing that would be
 pkcs11:object=schw...@mycoolca.com


-- 
dwmw2


smime.p7s
Description: S/MIME cryptographic signature

Re: [Openvpn-devel] [PATCH v3] Mac OS X Keychain management client

[Jonathan K. Bullard]

On Mon, Feb 23, 2015 at 4:00 AM, Gert Doering  wrote:
>
> On Mon, Feb 23, 2015 at 09:28:31AM +0100, Arne Schwabe wrote:
> > > What do you think of the change?
> > I like the idea. You could  make the macos-keychain in the string optional.
>
> What Arne said (both parts of it) :-)

I agree -- the argument to --needs-external-cert should be optional.

Note: the argument to --needs-external-cert should be passed on to
"RSA_SIGN", too. (I think Vasily omitted that from his writeup.)

So the idea would be:

 * Add an optional UTF-8 string argument to --needs-external-cert.
(Perhaps the docs should say this requires support from the management
interface software and that currently such support is only available
when using certain GUIs on OS X.)

 * OpenVPN passes that argument to RSA_SIGN and NEEDS-CERTIFICATE,
passing an empty string if the argument does not appear.

 * OS X GUIs such as Tunnelblick and Viscosity see the new RSA_SIGN or
NEEDS-CERTIFICATE argument and use keychain-mcd to deal with it. Other
GUIs ignore it or use something that does something equivalent to what
keychain-mcd does on OS X.

I'm not sure exactly how to add an argument to RSA_SIGN and
NEEDS-CERTIFICATE without breaking existing management interface
software but assume that is possible. (Also, the argument may need to
be escaped when it is passed to RSA_SIGN or NEEDS-CERTIFICATE if it
contains characters that are used as delimiters.)

Re: [Openvpn-devel] [PATCH v3] Mac OS X Keychain management client

[Gert Doering]

Hi,

On Mon, Feb 23, 2015 at 09:28:31AM +0100, Arne Schwabe wrote:
> > What do you think of the change?
> I like the idea. You could  make the macos-keychain in the string optional.

What Arne said (both parts of it) :-)

gert

-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


pgp_5Mi74ntMW.pgp
Description: PGP signature

Re: [Openvpn-devel] [PATCH v3] Mac OS X Keychain management client

[Vasily Kulikov]

Hi,

On Sun, Feb 15, 2015 at 23:01 +0100, Gert Doering wrote:
> Hi,
> 
> On Sun, Feb 15, 2015 at 10:05:07PM +0100, Arne Schwabe wrote:
> > Am 24.01.15 um 18:04 schrieb Vasily Kulikov:
> [..]
> > > OpenVPN itself gets new 'NEED-CERTIFICATE" command which is called when
> > > --management-external-cert is used.  It is implemented as a multiline
> > > command very similar to an existing 'RSA-SIGN' command.
> > >
> > > The patch is against commit 3341a98c2852d1d0c1eafdc70a3bdb218ec29049.
> > ACK from me to the OpenVPN part. I also tested the patch in OpenVPN for
> > Android and the RSA-SIGN still works as expected. I have not reviewed
> > the OS X contrib program (other than a quick glance at the code) but I
> > think marking it as contrib it should be allowed to be still included.
> 
> I hear Arne, and James also ACKed this ("based on testing", which Arne
> did).
> 
> I'm not merging it yet, though - Vasily, please provide a v4 of the patch
> that adds:
> 
>  - documentation of --management-external-cert in doc/openvpn.8
>  - documentation of the new management command and response in 
>doc/management-notes.txt
>  - fix the typos in the options here (please fix the other one, too):
> 
> @@ -2221,6 +2230,8 @@ options_postprocess_verify_ce (const struct options 
> *optio
> ns, const struct conne
>  #ifdef MANAGMENT_EXTERNAL_KEY
>if (options->management_flags & MF_EXTERNAL_KEY)
> msg(M_USAGE, "Parameter --external-management-key cannot be used 
> whe
> n --pkcs12 is also specified."); 
> +  if (options->management_flags & MF_EXTERNAL_CERT)
> +   msg(M_USAGE, "Parameter --external-management-cert cannot be used 
> wh
> en --pkcs12 is also specified.");
>  #endif
>  #endif
>  }
> 
> ... it's "--management-external-*", not "--external-management-*".
> 
> With that, I'll merge right away :-)

I've implemented the changes above, but don't send the patch yet.  While
talking with Jonathan Bullard we identified that with the current design
it is very hard to implement a simple identity template passing
independently of GUI implementation.  Currently identity template is
passed to keychain-mcd leaving aside openvpn itself and thus the openvpn
configuration file doesn't contain the identity template.  Tunnelblick or
another GUI has to start keychain-mcd and pass the identity template
itself.  The way a user notifies TB/openvpn what identity template to use
is TB-dependent: it can be e.g. storing XXX-keychain-identityTemplate
preference in Info.plist file inside .tblk directory.  Other GUI might
use a different configuration method and thus store the identity template
in some other place.  It would be better to have a single
GUI-independent way of configuring keychain-mcd.

Jonathan suggested the following method.  --management-external-cert has
a single argument, the identity template.  This template is passed as-is
to management interface client as an argument to NEED-CERTIFICATE
request.  Management interface client parses this string and chooses an
identity based on the argument.  In this case the argument is an opaque
value which is simply passed to a management interface client, which
handles it as it likes (or ignores).  Also an argument can be used with
an arbitrary management interface client, not only keychain-mcd.

We can restrict a bit a format of XXX in '--management-external-cert XXX'
option argument.  It can consist of two parts, e.g.:

--management-external-cert 'macosx-keychain:SUBJECT:c=US'

The first part is a hint how management client should handle the
argument.  In case of keychain-mcd it parses the argument, checks
whether it starts with 'macosx-keychain:', and uses the end of the
argument as an identity template.

If the change is implemented openvpn config can contain all information
needed to start openvpn connection.  A user has to start openvpn with
the config file and to start keychain-mcd.  Specifically it would contain a
line like the following:

management-external-cert 'macosx-keychain:SUBJECT:c=US'

With the approach in patch v3 a user has to start openvpn with the
config file, start keychain-mcd, and pass identity template as an
argument to keychain-mcd.

What do you think of the change?

Thanks,

-- 
Vasily Kulikov
http://www.openwall.com - bringing security into open computing environments

Re: [Openvpn-devel] [PATCH v3] Mac OS X Keychain management client

[Vasily Kulikov]

Hi Gert,

On Sun, Feb 15, 2015 at 23:01 +0100, Gert Doering wrote:
> I hear Arne, and James also ACKed this ("based on testing", which Arne
> did).
> 
> I'm not merging it yet, though - Vasily, please provide a v4 of the patch
> that adds:
...
> With that, I'll merge right away :-)

Thank you for the review, I'll send the patch during the weekends.

-- 
Vasily Kulikov
http://www.openwall.com - bringing security into open computing environments

Re: [Openvpn-devel] [PATCH v3] Mac OS X Keychain management client

[Gert Doering]

Hi,

On Sun, Feb 15, 2015 at 10:05:07PM +0100, Arne Schwabe wrote:
> Am 24.01.15 um 18:04 schrieb Vasily Kulikov:
[..]
> > OpenVPN itself gets new 'NEED-CERTIFICATE" command which is called when
> > --management-external-cert is used.  It is implemented as a multiline
> > command very similar to an existing 'RSA-SIGN' command.
> >
> > The patch is against commit 3341a98c2852d1d0c1eafdc70a3bdb218ec29049.
> ACK from me to the OpenVPN part. I also tested the patch in OpenVPN for
> Android and the RSA-SIGN still works as expected. I have not reviewed
> the OS X contrib program (other than a quick glance at the code) but I
> think marking it as contrib it should be allowed to be still included.

I hear Arne, and James also ACKed this ("based on testing", which Arne
did).

I'm not merging it yet, though - Vasily, please provide a v4 of the patch
that adds:

 - documentation of --management-external-cert in doc/openvpn.8
 - documentation of the new management command and response in 
   doc/management-notes.txt
 - fix the typos in the options here (please fix the other one, too):

@@ -2221,6 +2230,8 @@ options_postprocess_verify_ce (const struct options *optio
ns, const struct conne
 #ifdef MANAGMENT_EXTERNAL_KEY
   if (options->management_flags & MF_EXTERNAL_KEY)
msg(M_USAGE, "Parameter --external-management-key cannot be used whe
n --pkcs12 is also specified."); 
+  if (options->management_flags & MF_EXTERNAL_CERT)
+   msg(M_USAGE, "Parameter --external-management-cert cannot be used wh
en --pkcs12 is also specified.");
 #endif
 #endif
 }

... it's "--management-external-*", not "--external-management-*".

With that, I'll merge right away :-)

thanks,

gert

-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


pgp1k4t7kJD0_.pgp
Description: PGP signature

Re: [Openvpn-devel] [PATCH v3] Mac OS X Keychain management client

[Arne Schwabe]

Am 24.01.15 um 18:04 schrieb Vasily Kulikov:
> This patch adds support for using certificates stored in the Mac OSX
> Keychain to authenticate with the OpenVPN server.  This works with
> certificates stored on the computer as well as certificates on hardware
> tokens that support Apple's tokend interface.
>
> This patch version implements management client which handles RSA-SIGN
> command for RSA offloading.  Also it handles new 'NEED-CERTIFICATE'
> request to pass a certificate from the keychain to OpenVPN.
>
> OpenVPN itself gets new 'NEED-CERTIFICATE" command which is called when
> --management-external-cert is used.  It is implemented as a multiline
> command very similar to an existing 'RSA-SIGN' command.
>
> The patch is against commit 3341a98c2852d1d0c1eafdc70a3bdb218ec29049.
>
>
ACK from me to the OpenVPN part. I also tested the patch in OpenVPN for
Android and the RSA-SIGN still works as expected. I have not reviewed
the OS X contrib program (other than a quick glance at the code) but I
think marking it as contrib it should be allowed to be still included.

Arne



signature.asc
Description: OpenPGP digital signature

[Openvpn-devel] [PATCH v3] Mac OS X Keychain management client

[Vasily Kulikov]

This patch adds support for using certificates stored in the Mac OSX
Keychain to authenticate with the OpenVPN server.  This works with
certificates stored on the computer as well as certificates on hardware
tokens that support Apple's tokend interface.

This patch version implements management client which handles RSA-SIGN
command for RSA offloading.  Also it handles new 'NEED-CERTIFICATE'
request to pass a certificate from the keychain to OpenVPN.

OpenVPN itself gets new 'NEED-CERTIFICATE" command which is called when
--management-external-cert is used.  It is implemented as a multiline
command very similar to an existing 'RSA-SIGN' command.

The patch is against commit 3341a98c2852d1d0c1eafdc70a3bdb218ec29049.

v3:
 - used new 'NEED-CERTIFICATE' command for certificate data request
   instead of 'NEED-OK'
 - improved option checking
 - improved invalid certificate selection string handling
 - added man page for keychain-mcd
 - handle INFO, FATAL commands from openvpn and show them to user

v2 (http://sourceforge.net/p/openvpn/mailman/message/33225603/):
 - used management interface to communicate with OpenVPN process

v1 (http://sourceforge.net/p/openvpn/mailman/message/33125844/):
 - used RSA_METHOD to extend openvpn itself
 - used autoconf and automake scripts
 - used newer Mac OS X API
 - improved crypto API errors checking

Brian Raderman's version:
 http://thread.gmane.org/gmane.network.openvpn.devel/3631

Signed-off-by: Vasily Kulikov 
--
diff --git a/.gitignore b/.gitignore
index 538c020..f504ddb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -19,7 +19,6 @@ Debug
 Win32-Output
 .deps
 .libs
-Makefile
 Makefile.in
 aclocal.m4
 autodefs.h
diff --git a/contrib/keychain-mcd/Makefile b/contrib/keychain-mcd/Makefile
new file mode 100644
index 000..c6431df
--- /dev/null
+++ b/contrib/keychain-mcd/Makefile
@@ -0,0 +1,13 @@
+CFILES = cert_data.c common_osx.c crypto_osx.c main.c
+OFILES = $(CFILES:.c=.o) ../../src/openvpn/base64.o
+prog = keychain-mcd
+
+CC = gcc
+CFLAGS = -Wall
+LDFLAGS =  -framework CoreFoundation -framework Security -framework 
CoreServices
+
+$(prog): $(OFILES)
+   $(CC) $(LDFLAGS) $(OFILES) -o $(prog)
+
+%.o: %.c
+   $(CC) $(CFLAGS) -c $< -o $@
diff --git a/contrib/keychain-mcd/cert_data.c b/contrib/keychain-mcd/cert_data.c
new file mode 100644
index 000..4f06cdf
--- /dev/null
+++ b/contrib/keychain-mcd/cert_data.c
@@ -0,0 +1,733 @@
+/*
+ *  OpenVPN -- An application to securely tunnel IP networks
+ * over a single UDP port, with support for SSL/TLS-based
+ * session authentication and key exchange,
+ * packet encryption, packet authentication, and
+ * packet compression.
+ *
+ *  Copyright (C) 2010 Brian Raderman 
+ *  Copyright (C) 2013-2015 Vasily Kulikov 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program (see the file COPYING included with this
+ *  distribution); if not, write to the Free Software Foundation, Inc.,
+ *  59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+
+#include "cert_data.h"
+#include 
+#include 
+
+#include "common_osx.h"
+#include "crypto_osx.h"
+#include 
+
+CFStringRef kCertDataSubjectName = CFSTR("subject"),
+kCertDataIssuerName = CFSTR("issuer"),
+kCertDataSha1Name = CFSTR("SHA1"),
+kCertDataMd5Name = CFSTR("MD5"),
+kCertDataSerialName = CFSTR("serial"),
+kCertNameFwdSlash = CFSTR("/"),
+kCertNameEquals = CFSTR("=");
+CFStringRef kCertNameOrganization = CFSTR("o"),
+kCertNameOrganizationalUnit = CFSTR("ou"),
+kCertNameCountry = CFSTR("c"),
+kCertNameLocality = CFSTR("l"),
+kCertNameState = CFSTR("st"),
+kCertNameCommonName = CFSTR("cn"),
+kCertNameEmail = CFSTR("e");
+CFStringRef kStringSpace = CFSTR(" "),
+kStringEmpty = CFSTR("");
+
+typedef struct _CertName
+{
+  CFArrayRef countryName, organization, organizationalUnit, commonName, 
description, emailAddress, 
+ stateName, localityName;
+} CertName, *CertNameRef;  
+
+typedef struct _DescData
+{
+  CFStringRef name, value;
+} DescData, *DescDataRef;
+
+void destroyDescData(DescDataRef pData);
+
+CertNameRef createCertName()
+{
+  CertNameRef pCertName = (CertNameRef)malloc(sizeof(CertName));
+  pCertName->countryName = CFArrayCreateMutable(NULL, 0, 
);
+  pCertName->organization