Re: [Openvpn-devel] [PATCH] cert_data: fix memory leak
On Mon, Apr 20, 2015 at 16:30 +0200, Yegor Yefremov wrote:
> Release pCertName, if SecCertificateCopyValues() fails.
>
> Found via cppcheck.
>
> Signed-off-by: Yegor Yefremov <yegorsli...@googlemail.com>
> Cc: Vasily Kulikov <seg...@openwall.com>
Acked-by: Vasily Kulikov <seg...@openwall.com>
Thank you!
> ---
> contrib/keychain-mcd/cert_data.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/contrib/keychain-mcd/cert_data.c
> b/contrib/keychain-mcd/cert_data.c
> index f2b33ed..a04bf79 100644
> --- a/contrib/keychain-mcd/cert_data.c
> +++ b/contrib/keychain-mcd/cert_data.c
> @@ -146,6 +146,7 @@ CFArrayRef GetFieldsFromCertificate(SecCertificateRef
> certificate, CFTypeRef oid
>printErrorMsg("GetFieldsFromCertificate: SecCertificateCopyValues",
> error);
>CFRelease(keySelection);
>CFRelease(fields);
> + destroyCertName(pCertName);
>return NULL;
>}
>CFDictionaryRef vals = CFDictionaryGetValue(dict, oid);
> --
> 2.1.0
--
Vasily Kulikov
http://www.openwall.com - bringing security into open computing environments
Re: [Openvpn-devel] [PATCH v4] Mac OS X Keychain management client
Hi, On Mon, Mar 30, 2015 at 10:26 +0300, Samuli Seppänen wrote: > > On Fri, Mar 06, 2015 at 17:29 +0300, Vasily Kulikov wrote: > >> On Fri, Feb 27, 2015 at 20:34 +0100, Gert Doering wrote: > >>> Mmmh. Actually we don't usually do Makefile changes, as this is always > >>> generated by configure for us - so normally, it is good to have it in > >>> .gitignore. Of course your subdirectory has a Makefile in it for > >>> MacOS X only... > >>> > >>> So - git experts to the rescue - how's this normally done? > >>> > >>> (The textual change for doc/management-notes.txt does not warrant an extra > >>> patch, I'll change that on the fly) > >> Is the patch already merged? Or should I make any changes, e.g. remove > >> .gitignore changes? > > Any update? > > > I suggest we figure this out in today's meeting. Arne, AFAIU you are the one to ACK/NACK the v3..v4 diff. Have you looked at the patch after v3? Thank you, -- Vasily Kulikov http://www.openwall.com - bringing security into open computing environments
Re: [Openvpn-devel] [PATCH v4] Mac OS X Keychain management client
Hi, On Fri, Mar 06, 2015 at 17:29 +0300, Vasily Kulikov wrote: > On Fri, Feb 27, 2015 at 20:34 +0100, Gert Doering wrote: > > Mmmh. Actually we don't usually do Makefile changes, as this is always > > generated by configure for us - so normally, it is good to have it in > > .gitignore. Of course your subdirectory has a Makefile in it for > > MacOS X only... > > > > So - git experts to the rescue - how's this normally done? > > > > (The textual change for doc/management-notes.txt does not warrant an extra > > patch, I'll change that on the fly) > > Is the patch already merged? Or should I make any changes, e.g. remove > .gitignore changes? Any update? -- Vasily Kulikov http://www.openwall.com - bringing security into open computing environments
Re: [Openvpn-devel] [PATCH v4] Mac OS X Keychain management client
Hi Gert, On Fri, Feb 27, 2015 at 19:28 +0100, Gert Doering wrote: > On Wed, Feb 25, 2015 at 07:07:18PM +0300, Vasily Kulikov wrote: > > The patch is against commit 3341a98c2852d1d0c1eafdc70a3bdb218ec29049. > > > > v4: > > - added '--management-external-cert' argument > > - keychain-mcd now parses NEED-CERTIFICATE argument if 'auto' is passed > >as cmdline's identity template > > - fixed typo in help output option name > > - added '--management-external-cert' info in openvpn(8) manpage > > - added 'certificate' command documentation into doc/management-notes.txt > > Sorry to be nagging... something in your patch was garbled, it contained > stuff like > > --- > only in patch2: > unchanged: > --- a/doc/management-notes.txt > +++ b/doc/management-notes.txt > @@ -777,6 +777,28 @@ correct signature. > This capability is intended to allow the use of arbitrary cryptographic > service providers with OpenVPN via the management interface. > ... > --- This stuff is missing in the patch itself which is in the email text, and is contained in the attached interdiff file which contains changes between patch v3 and v4. AFAICS, the patch doesn't contain any garbage. > (the diff for doc/management-notes.txt is in there twice), and there > is a patch for .gitignore in it as well. I've included .gitignore changes as my patch adds Makefile changes. It would be rather uncomfortable for openvpn developers to see Makefile and be not able to change it. > Please generate the patch with "git format-patch", that should avoid > spurious stuff. > > > Also, in the doc/management-notes.txt, it has > > +COMMAND -- certificate (OpenVPN 2.3 or higher) > > please make that "2.4", as this code change is too large to go into 2.3 > (where we only do bug fixes and long-term stability stuff, but no new > features generally) It makes sense. -- Vasily Kulikov http://www.openwall.com - bringing security into open computing environments
[Openvpn-devel] [PATCH v4] Mac OS X Keychain management client
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. The patch is based on
the Windows Crypto API certificate functionality that currently exists
in OpenVPN.
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.
v4:
- added '--management-external-cert' argument
- keychain-mcd now parses NEED-CERTIFICATE argument if 'auto' is passed
as cmdline's identity template
- fixed typo in help output option name
- added '--management-external-cert' info in openvpn(8) manpage
- added 'certificate' command documentation into doc/management-notes.txt
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
* ACK from Arne Schwabe for OpenVPN part
* ACK from James based on Arne's testing
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
Signed-off-by: Vasily Kulikov <seg...@openwall.com>
--
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 <br...@irregularexpression.org>
+ * Copyright (C) 2013-2015 Vasily Kulikov <seg...@openwall.com>
+ *
+ * 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(" "),
+kSt
Re: [Openvpn-devel] [PATCH v3] Mac OS X Keychain management client
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
On Mon, Feb 23, 2015 at 08:04 -0500, Jonathan K. Bullard wrote: > On Mon, Feb 23, 2015 at 4:00 AM, Gert Doering <g...@greenie.muc.de> 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
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
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
[Openvpn-devel] [PATCH v3] Mac OS X Keychain management client
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 <seg...@openwall.com>
--
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 <br...@irregularexpression.org>
+ * Copyright (C) 2013-2015 Vasily Kulikov <seg...@openwall.com>
+ *
+ * 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 destroyDe
Re: [Openvpn-devel] [PATCHv2] Mac OS X Keychain management client
On Mon, Jan 12, 2015 at 13:54 +0100, Arne Schwabe wrote: > > Am 12.01.15 12:45, schrieb David Woodhouse: > > On Mon, 2015-01-12 at 11:51 +0300, Vasily Kulikov wrote: > >> 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. The patch is based on > >> the Windows Crypto API certificate functionality that currently exists > >> in OpenVPN. > > I wonder why only certifcates and not ca certifcates. It would be > logical to get all certifcates from the keychain. A user tells keychain-mcd which keychain identity to use for openvpn. The identity contains a certificate and a private key (might be non-extractable). CA is not related to this identity. It can be added to keychain-mcd and openvpn but a user should pass another certificate match string for CA. > >> > >> This patch version implements management client which handles rsa_sign > >> command for RSA offloading. > > FWIW we really ought to be supporting key types other than RSA by now. > > But I appreciate that's not a new limitation and not your fault. > > Well although rsa-sign at the momemnt probably only supports RSA (it is > implemented using rsa_method iirc) the API is not rsa specific. It is > just: "Please sign this hash with the private key". In the case of an > RSA certificate this happens to be RSA encrypt in ECB mode with PKCS#1 > padding. Right. And my patch is not specific to RSA too. It uses generic Mac OS X API which should work with any key type: SecIdentityCopyPrivateKey() SecKeyRawSign() -- Vasily Kulikov http://www.openwall.com - bringing security into open computing environments
[Openvpn-devel] [PATCHv2] Mac OS X Keychain management client
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. The patch is based on
the Windows Crypto API certificate functionality that currently exists
in OpenVPN.
This patch version implements management client which handles rsa_sign
command for RSA offloading. Also it handled new 'Cert' request to pass
a certificate from the keychain to OpenVPN.
This is a PoC version of the patch. If the concept is OK for OpenVPN,
I'll integrate it into autoconf, add documentation based on the
manpage from the previous version of the patch, and add more strict argv
checks for interferred options.
The patch is against commit 3341a98c2852d1d0c1eafdc70a3bdb218ec29049.
The previous version of the patch:
http://thread.gmane.org/gmane.network.openvpn.devel/9320
Signed-off-by: Vasily Kulikov <seg...@openwall.com>
---
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..edfa21b
--- /dev/null
+++ b/contrib/keychain-mcd/cert_data.c
@@ -0,0 +1,761 @@
+/*
+ * 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 <br...@irregularexpression.org>
+ * Copyright (C) 2013-2015 Vasily Kulikov <seg...@openwall.com>
+ *
+ * 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 = CFArrayCreateMutable(NULL, 0,
);
+ pCertName->organizationalUnit = CFArrayCreateMutable(NULL, 0,
);
+ pCertName->commonName = CFArrayCreateMutable(NULL, 0,
);
+ pCertName->description = CFArrayCreateMutable(NULL, 0,
);
+ pCertName->e
Re: [Openvpn-devel] [PATCH] Add Mac OS X keychain support
On Tue, Jan 06, 2015 at 14:00 +0100, Arne Schwabe wrote: > >>> Any comments? > > Some ideas about keychain implementation out of OpenVPN core. > > > > I see 4 possible alternatives here: > > 1) implement keychain rsa offloading in Tunnelblick > > 2) make my patch use plugin interface > > 3) implement external daemon that communicated with openvpn process via > > management interface > > 4) the same as 3) but make openvpn able to handle more than 1 > > management client > > > > There are problems with all these alternatives: > > (1) is limited to Tunnelblick users and doesn't work for users who start > > openvpn from the terminal or use any other GUI > > (2) implies plugin interface is expanded to handle two new actions: 'user > > certificate request' and 'rsa-sign request' > > (3) implies management interface is expanded to handle 'user certificate > > request'; also it doesn't work with Tunnelblick or any other tool that > > communicates with openvpn via management interface as MI currently > > supports only a single client > > (4) needs 'user certificate request' addition and expanding management > > interface to support more than a single client > > > > So, (1) is a no-go as it works only with Tunnelblick (3) is a no-go as > > it breaks Tunnelblick and probably someone else. We stay with only two > > interface expantions: plugin interface or management interface. Either > > (a) add 'user certificate request' to plugin interface or (b) add 'user > > certificate request' to management interface and support more than a > > single client. > But if you encapsulate that stuff in a library or something similar you > could use that library from TunnelBlick and from a small standalone > tool/OpenVPN plugin. > > For (4) Implementing a request for managament ca/cert is relatively > easy. At the moment this has not been implemented because nobody needed > it so far. All current users managment-query-key (or what the option is > called) generate the configuration with the right , options > instead of querying for these values at startup. I'm thinking about using existing 'needstr' command. Looking at the '#ifdef TARGET_ANDROID' code, it should be a very small patch to openvpn which adds a command line option like '--management-external-cert' and using management_query_user_pass() for certificate request (e.g. BASE64 encoded). > I disagress that (3) is inheritly incompatible with Tunnelblick. > Tunnelblick could spawn an openvpn process and the external daemon. On > the rsa-sign request, Tunnelblick would just connect to the external > daemon ask that daemon to process the rsa-sign request. Right. I was talking more about breaking current state of affairs rather than completely incompatible changes that cannot be fixed on the Tunnelblick side. -- Vasily Kulikov http://www.openwall.com - bringing security into open computing environments
Re: [Openvpn-devel] [PATCH] Add Mac OS X keychain support
Hi, On Fri, Dec 12, 2014 at 19:24 +0100, Arne Schwabe wrote: > > On Mon, Dec 08, 2014 at 14:52 +0300, Vasily Kulikov wrote: > >> 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. The patch is very similar > >> to, and also based on, the Windows Crypto API certificate functionality > >> that currently exists in OpenVPN. ... > >> Signed-off-by: Vasily Kulikov <seg...@openwall.com> > >> -- > > Any comments? Some ideas about keychain implementation out of OpenVPN core. I see 4 possible alternatives here: 1) implement keychain rsa offloading in Tunnelblick 2) make my patch use plugin interface 3) implement external daemon that communicated with openvpn process via management interface 4) the same as 3) but make openvpn able to handle more than 1 management client There are problems with all these alternatives: (1) is limited to Tunnelblick users and doesn't work for users who start openvpn from the terminal or use any other GUI (2) implies plugin interface is expanded to handle two new actions: 'user certificate request' and 'rsa-sign request' (3) implies management interface is expanded to handle 'user certificate request'; also it doesn't work with Tunnelblick or any other tool that communicates with openvpn via management interface as MI currently supports only a single client (4) needs 'user certificate request' addition and expanding management interface to support more than a single client So, (1) is a no-go as it works only with Tunnelblick; (3) is a no-go as it breaks Tunnelblick and probably someone else. We stay with only two interface expantions: plugin interface or management interface. Either (a) add 'user certificate request' to plugin interface or (b) add 'user certificate request' to management interface and support more than a single client. I'm not sure which way is better. Do you have any plans/objections on these interfaces expantion? Maybe I missed some critical limitations of the interfaces and they must not be used this way? If no strict objections I'd prefer to implement it via plugin interface as my patch would need to change only several hooks registration and handling functions in this case. Thanks, -- Vasily Kulikov http://www.openwall.com - bringing security into open computing environments
Re: [Openvpn-devel] [PATCH] Add Mac OS X keychain support
Hi Arne, On Fri, Dec 12, 2014 at 19:24 +0100, Arne Schwabe wrote: > Am 12.12.14 17:52, schrieb Vasily Kulikov: > > Any comments? > > > None yet. The patch is very large and our time is unfortenately limited. > And the number of people how do crypto and Mac OS is even smaller. I > haven't have found time yet to look at the code yet. (We don't want to > commit unreviewed code). Thank you! I don't want anybody to commit unreviewed code too. Do you think I should just wait until some people read the code or should I divide the code into smaller parts? However, I don't see the way it can be divided as it is a rather monolithic feature, only Tunnelblick support can be isolated (however, this second part would be very small and the first part would be still very big). -- Vasily Kulikov http://www.openwall.com - bringing security into open computing environments
Re: [Openvpn-devel] [PATCH] Add Mac OS X keychain support
Hi, On Mon, Dec 08, 2014 at 14:52 +0300, Vasily Kulikov wrote: > 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. The patch is very similar > to, and also based on, the Windows Crypto API certificate functionality > that currently exists in OpenVPN. > > The previous version of the patch was sent by Brian Raderman > (http://thread.gmane.org/gmane.network.openvpn.devel/3631). The current > version uses autoconf, doesn't use printf, fixes several small bugs like > ignoring errors, and it now works with Tunnelblick. The previous version > has been tested with an Aladdin eToken on Mac OSX Leopard and with > software only certificates on Mac OSX Leopard and Snow Leopard, as > reported by Brian Raderman in his email. The current version of the > patch was tested in Yandex company on ~3000 hosts using several Mac OS X > versions (10.7, 10.8. 10.9. 10.10) using Tunnelblick. > > It was tested both on OpenVPN started from the terminal and using > Tunnelblick. Renegotiation was tested too. > > There are several warnings on Mac OS X related to functions deprecation > like RSA_new() and similar. However, they are used in other OpenVPN > code, so I decided not to touch it. > > The patch is against commit 3341a98c2852d1d0c1eafdc70a3bdb218ec29049. > > Signed-off-by: Vasily Kulikov <seg...@openwall.com> > -- Any comments? -- Vasily Kulikov http://www.openwall.com - bringing security into open computing environments
[Openvpn-devel] [PATCH] Add Mac OS X keychain support
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. The patch is very similar
to, and also based on, the Windows Crypto API certificate functionality
that currently exists in OpenVPN.
The previous version of the patch was sent by Brian Raderman
(http://thread.gmane.org/gmane.network.openvpn.devel/3631). The current
version uses autoconf, doesn't use printf, fixes several small bugs like
ignoring errors, and it now works with Tunnelblick. The previous version
has been tested with an Aladdin eToken on Mac OSX Leopard and with
software only certificates on Mac OSX Leopard and Snow Leopard, as
reported by Brian Raderman in his email. The current version of the
patch was tested in Yandex company on ~3000 hosts using several Mac OS X
versions (10.7, 10.8. 10.9. 10.10) using Tunnelblick.
It was tested both on OpenVPN started from the terminal and using
Tunnelblick. Renegotiation was tested too.
There are several warnings on Mac OS X related to functions deprecation
like RSA_new() and similar. However, they are used in other OpenVPN
code, so I decided not to touch it.
The patch is against commit 3341a98c2852d1d0c1eafdc70a3bdb218ec29049.
Signed-off-by: Vasily Kulikov <seg...@openwall.com>
--
diff --git a/configure.ac b/configure.ac
index 608ab6d..127e173 100644
--- a/configure.ac
+++ b/configure.ac
@@ -258,6 +258,13 @@ AC_ARG_ENABLE(
)
AC_ARG_ENABLE(
+ [macosx-keychain],
+ [AS_HELP_STRING([--enable-macosx-keychain], [enable MAC OS X keychain
support @<:@default=yes@:>@])],
+ ,
+ [enable_keychain="no"]
+)
+
+AC_ARG_ENABLE(
[systemd],
[AS_HELP_STRING([--enable-systemd], [enable systemd suppport
@<:@default=no@:>@])],
,
@@ -330,6 +337,7 @@ case "$host" in
have_tap_header="yes"
dnl some Mac OS X tendering (we use vararg macros...)
CPPFLAGS="$CPPFLAGS -no-cpp-precomp"
+ MACOSX=yes
;;
*-mingw*)
AC_DEFINE([TARGET_WIN32], [1], [Are we running WIN32?])
@@ -1088,6 +1096,11 @@ if test "${enable_ssl}" = "yes"; then
AC_DEFINE([ENABLE_SSL], [1], [Enable ssl library])
fi
+if test "${enable_macosx_keychain}" = "yes"; then
+ test "${MACOSX}" != "yes" && AC_MSG_ERROR([keychain is available on MAC
OS X only])
+ AC_DEFINE([ENABLE_KEYCHAIN], [1], [Enable MAC OS X keychain support])
+fi
+
if test "${enable_crypto}" = "yes"; then
test "${have_crypto_crypto}" != "yes" &&
AC_MSG_ERROR([${with_crypto_library} crypto is required but missing])
test "${enable_crypto_ofb_cfb}" = "yes" &&
AC_DEFINE([ENABLE_OFB_CFB_MODE], [1], [Enable OFB and CFB cipher modes])
@@ -1199,6 +1212,7 @@ AC_SUBST([PLUGIN_AUTH_PAM_CFLAGS])
AC_SUBST([PLUGIN_AUTH_PAM_LIBS])
AM_CONDITIONAL([WIN32], [test "${WIN32}" = "yes"])
+AM_CONDITIONAL([MACOSX], [test "${MACOSX}" = "yes"])
AM_CONDITIONAL([GIT_CHECKOUT], [test "${GIT_CHECKOUT}" = "yes"])
AM_CONDITIONAL([ENABLE_PLUGIN_AUTH_PAM], [test "${enable_plugin_auth_pam}" =
"yes"])
AM_CONDITIONAL([ENABLE_PLUGIN_DOWN_ROOT], [test "${enable_plugin_down_root}" =
"yes"])
diff --git a/doc/openvpn.8 b/doc/openvpn.8
index 96ba555..a23d950 100644
--- a/doc/openvpn.8
+++ b/doc/openvpn.8
@@ -4450,6 +4450,77 @@ Certificate Store GUI.
.\"*
.TP
+.B --keychaincert select-string
+Load the certificate and private key from the
+Mac OSX Keychain (Mac OSX Only).
+
+Use this option instead of
+.B --cert
+and
+.B --key.
+
+This makes it possible to use any smart card supported by Mac OSX using the
tokend interface, but also any
+kind of certificate, residing in the Keychain, where you have access to
+the private key. This option has been tested on the client side with an
Aladdin eToken
+on Mac OSX Leopard and with software certificates stored in the Keychain on
Mac OSX Leopard and
+Mac OSX Snow Leopard. As of this writing (4/27/10) Aladdin has not yet
released Snow Leopard drivers.
+
+To select a certificate based on a string search in the
+certificate's subject and/or issuer:
+
+.B --keychaincert
+"SUBJECT:c=US/o=Apple Inc./ou=me.com/cn=username ISSUER:c=US/o=Apple Computer,
Inc./ou=Apple Computer Certificate Authority/cn=Apple .Mac Certificate
Authority"
+
+.I "Distinguished Name Component Abbreviations:"
+.br
+o = organization
+.br
+ou = organizational unit
+.br
+c = country
+.br
+l = locality
+.br
+st = state
+.br
+cn = common name
+.br
+e = emai
