From a3982181e284f8c5c8fc15bbbd670da4d91a2ba9 Mon Sep 17 00:00:00 2001 From: Mathieu GIANNECCHINI Date: Tue, 2 Mar 2010 00:26:57 +0100 Subject: enhance tls-verify possibility It should be nice to enhance tls-verify check possibilities against peer cert during a pending TLS connection like : - OCSP verification - check any X509 extensions of the peer certificate - delta CRL verification - ... This patch add a new "tls-export-cert" option which allow to get peer certificate in PEM format and to store it in an openvpn temporary file. Peer certificate is stored before tls-script execution and deleted after. The name of the related temporary file is available under tls-verify script by an environment variable "peer_cert". The patch was made from OpenVPN svn Beta21 branches. Here is a very simple exemple of Tls-verify script which provide OCSP support to OpenVPN (with tls-export-cert option) without any OpenVPN "core" modification : X509=$2 openssl ocsp \ -issuer /etc/openvpn/ssl.crt/RootCA.pem \ -CAfile /etc/openvpn/ssl.capath/OpenVPNServeur-cafile.pem \ -cert $peer_cert \ -url http://your-ocsp-url if [ $? -ne 0 ] then echo "error : OCSP check failed for ${X509}" | logger -t "tls-verify" exit 1 fi This has been discussed here: This patch has been modified by David Sommerseth, by fixing a few issues which came up to during the code review process. The man page has been updated and tmp_file in ssl.c is checked for not being NULL before calling delete_file(). Signed-off-by: David Sommerseth Acked-by: Gert Doering --- init.c | 1 + openvpn.8 | 13 +++++++++++++ options.c | 10 ++++++++++ options.h | 1 + ssl.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ ssl.h | 1 + 6 files changed, 87 insertions(+) diff --git a/init.c b/init.c index 3748c2e..19ac032 100644 --- a/init.c +++ b/init.c @@ -1805,6 +1805,7 @@ do_init_crypto_tls (struct context *c, const unsigned int flags) #endif to.verify_command = options->tls_verify; + to.verify_export_cert = options->tls_export_cert; to.verify_x509name = options->tls_remote; to.crl_file = options->crl_file; to.ns_cert_type = options->ns_cert_type; diff --git a/openvpn.8 b/openvpn.8 index 45e61fa..4846d34 100644 --- a/openvpn.8 +++ b/openvpn.8 @@ -4258,6 +4258,14 @@ to to build a command line which will be passed to the script. .\"********************************************************* .TP +.B --tls-export-cert directory +Store the certificates the clients uses upon connection to this +directory. This will be done before --tls-verify is called. The +certificates will use a temporary name and will be deleted when +the tls-verify script returns. The file name used for the certificate +is available via the peer_cert environment variable. +.\"********************************************************* +.TP .B --tls-remote name Accept connections only from a host with X509 name or common name equal to @@ -5242,6 +5250,11 @@ than their names as denoted on the command line or configuration file. .\"********************************************************* .TP +.B peer_cert +Temporary file name containing the client certificate upon +connection. Useful in conjunction with --tls-verify +.\"********************************************************* +.TP .B script_context Set to "init" or "restart" prior to up/down script execution. For more information, see diff --git a/options.c b/options.c index 36b9913..e79f742 100644 --- a/options.c +++ b/options.c @@ -529,6 +529,9 @@ static const char usage_message[] = " tests of certification. cmd should return 0 to allow\n" " TLS handshake to proceed, or 1 to fail. (cmd is\n" " executed as 'cmd certificate_depth X509_NAME_oneline')\n" + "--tls-export-cert [directory] : Get peer cert in PEM format and store it \n" + " in an openvpn temporary file in [directory]. Peer cert is \n" + " stored before tls-verify script execution and deleted after.\n" "--tls-remote x509name: Accept connections only from a host with X509 name\n" " x509name. The remote host must also pass all other tests\n" " of verification.\n" @@ -1325,6 +1328,7 @@ show_settings (const struct options *o) #endif SHOW_STR (cipher_list); SHOW_STR (tls_verify); + SHOW_STR (tls_export_cert); SHOW_STR (tls_remote); SHOW_STR (crl_file); SHOW_INT (ns_cert_type); @@ -1914,6 +1918,7 @@ options_postprocess_verify_ce (const struct options *options, const struct conne MUST_BE_UNDEF (pkcs12_file); MUST_BE_UNDEF (cipher_list); MUST_BE_UNDEF (tls_verify); + MUST_BE_UNDEF (tls_export_cert); MUST_BE_UNDEF (tls_remote); MUST_BE_UNDEF (tls_timeout); MUST_BE_UNDEF (renegotiate_bytes); @@ -5525,6 +5530,11 @@ add_option (struct options *options, goto err; options->tls_verify = string_substitute (p[1], ',', ' ', &options->gc); } + else if (streq (p[0], "tls-export-cert") && p[1]) + { + VERIFY_PERMISSION (OPT_P_GENERAL); + options->tls_export_cert = p[1]; + } else if (streq (p[0], "tls-remote") && p[1]) { VERIFY_PERMISSION (OPT_P_GENERAL); diff --git a/options.h b/options.h index 740e18e..a3114e2 100644 --- a/options.h +++ b/options.h @@ -440,6 +440,7 @@ struct options const char *pkcs12_file; const char *cipher_list; const char *tls_verify; + const char *tls_export_cert; const char *tls_remote; const char *crl_file; diff --git a/ssl.c b/ssl.c index 82e04a3..d5230f7 100644 --- a/ssl.c +++ b/ssl.c @@ -687,6 +687,49 @@ string_mod_sslname (char *str, const unsigned int restrictive_flags, const unsig string_mod (str, restrictive_flags, 0, '_'); } +/* Get peer cert and store it in pem format in a temporary file + * in tmp_dir + */ + +const char * +get_peer_cert(X509_STORE_CTX *ctx, const char *tmp_dir, struct gc_arena *gc) +{ + X509 *peercert; + FILE *peercert_file; + const char *peercert_filename=""; + + if(!tmp_dir) + return NULL; + + /* get peer cert */ + peercert = X509_STORE_CTX_get_current_cert(ctx); + if(!peercert) + { + msg (M_ERR, "Unable to get peer certificate from current context"); + return NULL; + } + + /* create tmp file to store peer cert */ + peercert_filename = create_temp_filename (tmp_dir, "pcf", gc); + + /* write peer-cert in tmp-file */ + peercert_file = fopen(peercert_filename, "w+"); + if(!peercert_file) + { + msg (M_ERR, "Failed to open temporary file : %s", peercert_filename); + return NULL; + } + if(PEM_write_X509(peercert_file,peercert)<0) + { + msg (M_ERR, "Failed to write peer certificate in PEM format"); + fclose(peercert_file); + return NULL; + } + + fclose(peercert_file); + return peercert_filename; +} + /* * Our verify callback function -- check * that an incoming peer certificate is good. @@ -885,10 +928,21 @@ verify_callback (int preverify_ok, X509_STORE_CTX * ctx) /* run --tls-verify script */ if (opt->verify_command) { + const char *tmp_file; + struct gc_arena gc; int ret; setenv_str (opt->es, "script_type", "tls-verify"); + if (opt->verify_export_cert) + { + gc = gc_new(); + if (tmp_file=get_peer_cert(ctx, opt->verify_export_cert,&gc)) + { + setenv_str(opt->es, "peer_cert", tmp_file); + } + } + argv_printf (&argv, "%sc %d %s", opt->verify_command, ctx->error_depth, @@ -896,6 +950,13 @@ verify_callback (int preverify_ok, X509_STORE_CTX * ctx) argv_msg_prefix (D_TLS_DEBUG, &argv, "TLS: executing verify command"); ret = openvpn_execve (&argv, opt->es, S_SCRIPT); + if (opt->verify_export_cert) + { + if (tmp_file) + delete_file(tmp_file); + gc_free(&gc); + } + if (system_ok (ret)) { msg (D_HANDSHAKE, "VERIFY SCRIPT OK: depth=%d, %s", diff --git a/ssl.h b/ssl.h index 9737f26..a22c854 100644 --- a/ssl.h +++ b/ssl.h @@ -441,6 +441,7 @@ struct tls_options /* cert verification parms */ const char *verify_command; + const char *verify_export_cert; const char *verify_x509name; const char *crl_file; int ns_cert_type; -- cgit v1.2.3 From 87afefff8fe7b43b2c5cbba7a03a887fd9c02336 Mon Sep 17 00:00:00 2001 From: "Karl O. Pinc" Date: Tue, 2 Mar 2010 21:41:06 +0100 Subject: Several updates to openvpn.8 (man page updates) This is a collection of 4 patches sent to the -devel mailing list: * [PATCH] Frob the openvpn(8) man page tls-verify section to clarify * [PATCH] More improvments to openvpn(8) --tls-verify * [PATCH] Yet another tweak of openvpn(8) --tls-verify * [PATCH] Final frobbing of openvpn(8) --tls-verify Signed-off-by: David Sommerseth Acked-by: David Sommerseth --- openvpn.8 | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/openvpn.8 b/openvpn.8 index 4846d34..17549d8 100644 --- a/openvpn.8 +++ b/openvpn.8 @@ -4230,11 +4230,23 @@ test). .B cmd should return 0 to allow the TLS handshake to proceed, or 1 to fail. + +Note that +.B cmd +is a command line and as such may (if enclosed in quotes) contain +whitespace separated arguments. The first word of +.B cmd +is the shell command to execute and the remaining words are its +arguments. +When .B cmd -is executed as +is executed two arguments are appended, as follows: .B cmd certificate_depth X509_NAME_oneline +These arguments are, respectively, the current certificate depth and +the X509 common name (cn) of the peer. + This feature is useful if the peer you want to trust has a certificate which was signed by a certificate authority who also signed many other certificates, where you don't necessarily want to trust all of them, @@ -4248,14 +4260,6 @@ in the OpenVPN distribution. See the "Environmental Variables" section below for additional parameters passed as environmental variables. - -Note that -.B cmd -can be a shell command with multiple arguments, in which -case all OpenVPN-generated arguments will be appended -to -.B cmd -to build a command line which will be passed to the script. .\"********************************************************* .TP .B --tls-export-cert directory -- cgit v1.2.3 From 9f4725e86be9700c5894e360e09496d9ee1cfb85 Mon Sep 17 00:00:00 2001 From: Wil Cooley Date: Tue, 2 Mar 2010 21:54:15 +0100 Subject: pkitool lacks expected option "--help" The pkitool script lacks the "--help" parameter to actually display the usage statement; most people are conditioned to try that before running the command without options. This patch adds that and "--version" to display just the program name and version. sf.net tracker: Signed-off-by: David Sommerseth Acked-by: Jan Just Keijser --- easy-rsa/2.0/pkitool | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/easy-rsa/2.0/pkitool b/easy-rsa/2.0/pkitool index 1dffcdd..2a0b2f3 100755 --- a/easy-rsa/2.0/pkitool +++ b/easy-rsa/2.0/pkitool @@ -192,6 +192,12 @@ while [ $# -gt 0 ]; do $PKCS11TOOL --module "$PKCS11_MODULE_PATH" --list-objects --login --slot "$PKCS11_SLOT" exit 0;; + --help|--usage) + usage + exit ;; + --version) + echo "$PROGNAME $VERSION" + exit ;; # errors --* ) die "$PROGNAME: unknown option: $1" ;; * ) break ;; -- cgit v1.2.3 From f27bf509315a48b0070294c3993a718df0c2626c Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Thu, 8 Apr 2010 20:31:01 +0200 Subject: Add comile time information/settings from ./configure to --version This patch will create ./configure.h which will contain two new #define strings. CONFIGURE_DEFINES will contain all USE, ENABLED, DISABLED and DEPRECATED defines from ./config.h. CONFIGURE_CALL will contain the complete ./configure line which was used when configuring the package for building. Signed-off-by: David Sommerseth Acked-by: James Yonan --- Makefile.am | 7 +++++++ configure_h.awk | 39 +++++++++++++++++++++++++++++++++++++++ configure_log.awk | 36 ++++++++++++++++++++++++++++++++++++ options.c | 3 +++ 4 files changed, 85 insertions(+) create mode 100644 configure_h.awk create mode 100644 configure_log.awk diff --git a/Makefile.am b/Makefile.am index 7bccc11..20453d0 100644 --- a/Makefile.am +++ b/Makefile.am @@ -6,6 +6,7 @@ # packet compression. # # Copyright (C) 2002-2009 OpenVPN Technologies, Inc. +# Copyright (C) 2010 David Sommerseth # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License version 2 @@ -140,6 +141,12 @@ openvpn_SOURCES = \ win32.h win32.c \ cryptoapi.h cryptoapi.c +configure.h: Makefile + awk -f configure_h.awk config.h > $@ + awk -f configure_log.awk config.log >> $@ + +clean-local: + -rm -f configure.h dist-hook: cd $(distdir) && for i in $(EXTRA_DIST) $(SUBDIRS) ; do find $$i -name .svn -type d -prune -exec rm -rf '{}' ';' ; rm -f `find $$i -type f | grep -E '(^|\/)\.?\#|\~$$|\.s?o$$'` ; done diff --git a/configure_h.awk b/configure_h.awk new file mode 100644 index 0000000..672e745 --- /dev/null +++ b/configure_h.awk @@ -0,0 +1,39 @@ +# +# 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 David Sommerseth +# +# 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 +# +# +# This script will build up a line which can be included into a C program. +# The line will contain all interesting #define statements from f.ex. ./config.h +# + +BEGIN { + printf ("#define CONFIGURE_DEFINES \"") +} + +/^#define (ENABLE|DISABLE|DEPRECATED|USE)_/ { + printf (" %s", $2) +} + +END { + printf ("\"\n") +} diff --git a/configure_log.awk b/configure_log.awk new file mode 100644 index 0000000..b305f71 --- /dev/null +++ b/configure_log.awk @@ -0,0 +1,36 @@ +# +# 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 David Sommerseth +# +# 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 +# +# +# This script will build up a line which can be included into a C program. +# The line will only contain the first entry of the ./configure line from +# ./config.log. +# + +BEGIN { + printf ("#define CONFIGURE_CALL \"") +} +/\$ .\/configure/ { + print $0,"\"" + exit 0 +} diff --git a/options.c b/options.c index e79f742..25936d1 100644 --- a/options.c +++ b/options.c @@ -45,6 +45,7 @@ #include "pool.h" #include "helper.h" #include "manage.h" +#include "configure.h" #include "memdbg.h" @@ -2751,6 +2752,8 @@ usage_version (void) msg (M_INFO|M_NOPREFIX, "%s", title_string); msg (M_INFO|M_NOPREFIX, "Originally developed by James Yonan"); msg (M_INFO|M_NOPREFIX, "Copyright (C) 2002-2009 OpenVPN Technologies, Inc. "); + msg (M_INFO|M_NOPREFIX, "\n%s\n", CONFIGURE_CALL); + msg (M_INFO|M_NOPREFIX, "Compile time defines: %s", CONFIGURE_DEFINES); openvpn_exit (OPENVPN_EXIT_STATUS_USAGE); /* exit point */ } -- cgit v1.2.3 From 63c367398a57c98ab56f8532e3ff3ea8b89ab92e Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Thu, 22 Apr 2010 23:01:31 +0200 Subject: Fix dependency checking for configure.h (v2) Alon Bar-Lev indicated commit f27bf509315a48b0070294c3993a718df0c2626c was missing proper dependency checking. This patch corrects this and fixes an issue when creating configure.h via make distcheck. This is an enhanced version of the one sent to the openvpn-devel mailing list April 13, 2010 [1], after having received some feedback from Gert Doering, cleaning up configure_log.awk further. [1] Signed-off-by: David Sommerseth Acked-by: Gert Doering --- Makefile.am | 10 +++++++--- configure_log.awk | 7 ++----- options.c | 2 ++ 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/Makefile.am b/Makefile.am index 20453d0..f509a4b 100644 --- a/Makefile.am +++ b/Makefile.am @@ -66,7 +66,8 @@ dist_noinst_SCRIPTS = \ $(TESTS) \ doclean \ domake-win \ - t_cltsrv-down.sh + t_cltsrv-down.sh \ + configure_h.awk configure_log.awk dist_noinst_DATA = \ openvpn.spec \ @@ -141,9 +142,12 @@ openvpn_SOURCES = \ win32.h win32.c \ cryptoapi.h cryptoapi.c +nodist_openvpn_SOURCES = configure.h +options.$(OBJEXT): configure.h + configure.h: Makefile - awk -f configure_h.awk config.h > $@ - awk -f configure_log.awk config.log >> $@ + awk -f $(srcdir)/configure_h.awk config.h > $@ + awk -f $(srcdir)/configure_log.awk config.log >> $@ clean-local: -rm -f configure.h diff --git a/configure_log.awk b/configure_log.awk index b305f71..099e5c4 100644 --- a/configure_log.awk +++ b/configure_log.awk @@ -27,10 +27,7 @@ # ./config.log. # -BEGIN { - printf ("#define CONFIGURE_CALL \"") -} -/\$ .\/configure/ { - print $0,"\"" +/\$ (.*)\/configure/ { + printf ("#define CONFIGURE_CALL \"%s\"\n", $0) exit 0 } diff --git a/options.c b/options.c index 25936d1..294ba58 100644 --- a/options.c +++ b/options.c @@ -2752,7 +2752,9 @@ usage_version (void) msg (M_INFO|M_NOPREFIX, "%s", title_string); msg (M_INFO|M_NOPREFIX, "Originally developed by James Yonan"); msg (M_INFO|M_NOPREFIX, "Copyright (C) 2002-2009 OpenVPN Technologies, Inc. "); +#ifdef CONFIGURE_CALL msg (M_INFO|M_NOPREFIX, "\n%s\n", CONFIGURE_CALL); +#endif msg (M_INFO|M_NOPREFIX, "Compile time defines: %s", CONFIGURE_DEFINES); openvpn_exit (OPENVPN_EXIT_STATUS_USAGE); /* exit point */ } -- cgit v1.2.3 From ef12b6f57b32cc1e79c87741666d1850d6f25bd9 Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Tue, 13 Apr 2010 15:12:27 +0200 Subject: Make use of automake CLEANFILES variable instead of clean-local rule Signed-off-by: David Sommerseth Acked-by: Gert Doering --- Makefile.am | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Makefile.am b/Makefile.am index f509a4b..76573d1 100644 --- a/Makefile.am +++ b/Makefile.am @@ -38,7 +38,7 @@ MAINTAINERCLEANFILES = \ $(srcdir)/depcomp $(srcdir)/aclocal.m4 \ $(srcdir)/config.guess $(srcdir)/config.sub \ $(srcdir)/config-win32.h $(srcdir)/openvpn.spec -CLEANFILES = openvpn.8.html +CLEANFILES = openvpn.8.html configure.h EXTRA_DIST = \ easy-rsa \ @@ -149,9 +149,6 @@ configure.h: Makefile awk -f $(srcdir)/configure_h.awk config.h > $@ awk -f $(srcdir)/configure_log.awk config.log >> $@ -clean-local: - -rm -f configure.h - dist-hook: cd $(distdir) && for i in $(EXTRA_DIST) $(SUBDIRS) ; do find $$i -name .svn -type d -prune -exec rm -rf '{}' ';' ; rm -f `find $$i -type f | grep -E '(^|\/)\.?\#|\~$$|\.s?o$$'` ; done -- cgit v1.2.3 From 8dd2672d72508e9edec3d24b75e698b2669d7623 Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Thu, 22 Apr 2010 23:05:00 +0200 Subject: Don't add compile time information if --enable-small is used This is to satisfy those wanting to build openvpn for embedded devices where every bytes matters. Signed-off-by: David Sommerseth Acked-by: Gert Doering --- options.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/options.c b/options.c index 294ba58..86279e4 100644 --- a/options.c +++ b/options.c @@ -2752,10 +2752,12 @@ usage_version (void) msg (M_INFO|M_NOPREFIX, "%s", title_string); msg (M_INFO|M_NOPREFIX, "Originally developed by James Yonan"); msg (M_INFO|M_NOPREFIX, "Copyright (C) 2002-2009 OpenVPN Technologies, Inc. "); +#ifndef ENABLE_SMALL #ifdef CONFIGURE_CALL msg (M_INFO|M_NOPREFIX, "\n%s\n", CONFIGURE_CALL); #endif msg (M_INFO|M_NOPREFIX, "Compile time defines: %s", CONFIGURE_DEFINES); +#endif openvpn_exit (OPENVPN_EXIT_STATUS_USAGE); /* exit point */ } -- cgit v1.2.3 From 339f2a4d4b487afa53fa99d72c35b16f31e417d3 Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Thu, 29 Apr 2010 23:35:45 +0200 Subject: Revamped the script-security warning logging (version 2) The main task of this patch is to avoid reporting the SCRIPT_SECURITY_WARNING over and over again, in addition to not show this warning when it should not be a problem. This general warning should now only appear once, and only when --script-security is not set, 0 or 1. In all other cases this warning should not appear. In addition, this warning will come close to the script-hook which most probably will fail. It will also give a little bit more concrete hint on which script-hook which failed. If --script-security is 2 or 3, only the execve failure itself will be shown. This message will on the other hand be shown repeatedly. This is a new rewritten version which simplifies the implementaion of the new openvpn_run_script() function. It was considered to remove it completely, but due to code clearity and easy of use it was decided to make this function a static inline function instead. Anyhow, this function will enforce openvpn_execve_check() to be called with the S_SCRIPT flag. Patch ACKed on the developers meeting 2009-04-29. Signed-off-by: David Sommerseth Acked-by: James Yonan --- common.h | 2 +- init.c | 2 +- misc.c | 7 +++++-- misc.h | 10 ++++++++++ multi.c | 6 +++--- socket.c | 2 +- ssl.c | 4 ++-- win32.c | 5 ++++- 8 files changed, 27 insertions(+), 11 deletions(-) diff --git a/common.h b/common.h index 94da09e..3ee3fc8 100644 --- a/common.h +++ b/common.h @@ -90,6 +90,6 @@ typedef unsigned long ptr_type; /* * Script security warning */ -#define SCRIPT_SECURITY_WARNING "openvpn_execve: external program may not be called unless '--script-security 2' or higher is enabled. Use '--script-security 3 system' for backward compatibility with 2.1_rc8 and earlier. See --help text or man page for detailed info." +#define SCRIPT_SECURITY_WARNING "WARNING: External program may not be called unless '--script-security 2' or higher is enabled. Use '--script-security 3 system' for backward compatibility with 2.1_rc8 and earlier. See --help text or man page for detailed info." #endif diff --git a/init.c b/init.c index 19ac032..ec4a4a1 100644 --- a/init.c +++ b/init.c @@ -975,7 +975,7 @@ do_route (const struct options *options, struct argv argv = argv_new (); setenv_str (es, "script_type", "route-up"); argv_printf (&argv, "%sc", options->route_script); - openvpn_execve_check (&argv, es, S_SCRIPT, "Route script failed"); + openvpn_run_script (&argv, es, 0, "--route-up"); argv_reset (&argv); } diff --git a/misc.c b/misc.c index 33e6762..9fce0c8 100644 --- a/misc.c +++ b/misc.c @@ -230,7 +230,7 @@ run_up_down (const char *command, ifconfig_local, ifconfig_remote, context); argv_msg (M_INFO, &argv); - openvpn_execve_check (&argv, es, S_SCRIPT|S_FATAL, "script failed"); + openvpn_run_script (&argv, es, S_FATAL, "--up/--down"); argv_reset (&argv); } @@ -493,6 +493,7 @@ openvpn_execve_allowed (const unsigned int flags) return script_security >= SSEC_BUILT_IN; } + #ifndef WIN32 /* * Run execve() inside a fork(). Designed to replicate the semantics of system() but @@ -504,6 +505,7 @@ openvpn_execve (const struct argv *a, const struct env_set *es, const unsigned i { struct gc_arena gc = gc_new (); int ret = -1; + static bool warn_shown = false; if (a && a->argv[0]) { @@ -540,9 +542,10 @@ openvpn_execve (const struct argv *a, const struct env_set *es, const unsigned i ASSERT (0); } } - else + else if (!warn_shown && (script_security < SSEC_SCRIPTS)) { msg (M_WARN, SCRIPT_SECURITY_WARNING); + warn_shown = true; } #else msg (M_WARN, "openvpn_execve: execve function not available"); diff --git a/misc.h b/misc.h index bf51e89..4695d3e 100644 --- a/misc.h +++ b/misc.h @@ -136,6 +136,15 @@ bool openvpn_execve_check (const struct argv *a, const struct env_set *es, const bool openvpn_execve_allowed (const unsigned int flags); int openvpn_system (const char *command, const struct env_set *es, unsigned int flags); +static inline bool +openvpn_run_script (const struct argv *a, const struct env_set *es, const unsigned int flags, const char *hook) +{ + char msg[256]; + + openvpn_snprintf(msg, sizeof(msg), "WARNING: Failed running command (%s)", hook); + return openvpn_execve_check(a, es, flags | S_SCRIPT, msg); +}; + #ifdef HAVE_STRERROR /* a thread-safe version of strerror */ const char* strerror_ts (int errnum, struct gc_arena *gc); @@ -303,6 +312,7 @@ void get_user_pass_auto_userid (struct user_pass *up, const char *tag); extern const char *iproute_path; #endif +/* Script security */ #define SSEC_NONE 0 /* strictly no calling of external programs */ #define SSEC_BUILT_IN 1 /* only call built-in programs such as ifconfig, route, netsh, etc.*/ #define SSEC_SCRIPTS 2 /* allow calling of built-in programs and user-defined scripts */ diff --git a/multi.c b/multi.c index 342871a..92ab4ca 100644 --- a/multi.c +++ b/multi.c @@ -109,7 +109,7 @@ learn_address_script (const struct multi_context *m, mroute_addr_print (addr, &gc)); if (mi) argv_printf_cat (&argv, "%s", tls_common_name (mi->context.c2.tls_multi, false)); - if (!openvpn_execve_check (&argv, es, S_SCRIPT, "WARNING: learn-address command failed")) + if (!openvpn_run_script (&argv, es, 0, "--learn-address")) ret = false; argv_reset (&argv); } @@ -480,7 +480,7 @@ multi_client_disconnect_script (struct multi_context *m, struct argv argv = argv_new (); setenv_str (mi->context.c2.es, "script_type", "client-disconnect"); argv_printf (&argv, "%sc", mi->context.options.client_disconnect_script); - openvpn_execve_check (&argv, mi->context.c2.es, S_SCRIPT, "client-disconnect command failed"); + openvpn_run_script (&argv, mi->context.c2.es, 0, "--client-disconnect"); argv_reset (&argv); } #ifdef MANAGEMENT_DEF_AUTH @@ -1586,7 +1586,7 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi mi->context.options.client_connect_script, dc_file); - if (openvpn_execve_check (&argv, mi->context.c2.es, S_SCRIPT, "client-connect command failed")) + if (openvpn_run_script (&argv, mi->context.c2.es, 0, "--client-connect")) { multi_client_connect_post (m, mi, dc_file, option_permissions_mask, &option_types_found); ++cc_succeeded_count; diff --git a/socket.c b/socket.c index e42ccb9..7d20bb0 100644 --- a/socket.c +++ b/socket.c @@ -1663,7 +1663,7 @@ link_socket_connection_initiated (const struct buffer *buf, struct argv argv = argv_new (); setenv_str (es, "script_type", "ipchange"); ipchange_fmt (true, &argv, info, &gc); - openvpn_execve_check (&argv, es, S_SCRIPT, "ip-change command failed"); + openvpn_run_script (&argv, es, 0, "--ipchange"); argv_reset (&argv); } diff --git a/ssl.c b/ssl.c index d5230f7..a4af6a5 100644 --- a/ssl.c +++ b/ssl.c @@ -948,7 +948,7 @@ verify_callback (int preverify_ok, X509_STORE_CTX * ctx) ctx->error_depth, subject); argv_msg_prefix (D_TLS_DEBUG, &argv, "TLS: executing verify command"); - ret = openvpn_execve (&argv, opt->es, S_SCRIPT); + ret = openvpn_run_script (&argv, opt->es, 0, "--tls-verify script"); if (opt->verify_export_cert) { @@ -3232,7 +3232,7 @@ verify_user_pass_script (struct tls_session *session, const struct user_pass *up argv_printf (&argv, "%sc %s", session->opt->auth_user_pass_verify_script, tmp_file); /* call command */ - retval = openvpn_execve (&argv, session->opt->es, S_SCRIPT); + retval = openvpn_run_script (&argv, session->opt->es, 0, "--auth-user-pass-verify"); /* test return status of command */ if (system_ok (retval)) diff --git a/win32.c b/win32.c index eb94eb8..4de4139 100644 --- a/win32.c +++ b/win32.c @@ -952,6 +952,8 @@ int openvpn_execve (const struct argv *a, const struct env_set *es, const unsigned int flags) { int ret = -1; + static bool exec_warn = false; + if (a && a->argv[0]) { if (openvpn_execve_allowed (flags)) @@ -1004,9 +1006,10 @@ openvpn_execve (const struct argv *a, const struct env_set *es, const unsigned i ASSERT (0); } } - else + else if (!exec_warn && (script_security < SSEC_SCRIPTS)) { msg (M_WARN, SCRIPT_SECURITY_WARNING); + exec_warn = true; } } else -- cgit v1.2.3 From c5b7923a2b0a94d702e1dad59438f7ee75971d3b Mon Sep 17 00:00:00 2001 From: Fabian Knittel Date: Tue, 4 May 2010 16:21:47 +0200 Subject: ssl.c: fix use of openvpn_run_script()'s return value This patch fixes two bugs introduced in commit 339f2a4d4b487afa53fa99d72c35b16f31e417d3 Author: David Sommerseth Date: Thu Apr 29 23:35:45 2010 +0200 David's patch replaced openvpn_execve() with openvpn_run_script() in two places, but didn't adjust the return value handling. openvpn_run_script() returns true or false, while openvpn_execve() returns the program's exit code. Without the fix, the --tls-verify script and the --auth-user-pass-verify script fail to run. (I noticed the latter, but haven't actually tested the former.) The return value handling is fine for the other places where openvpn_run_script() is used, because those places previously used openvpn_execve_check() (notice the "_check" suffix). Signed-off-by: Fabian Knittel Signed-off-by: David Sommerseth Acked-by: David Sommerseth --- ssl.c | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/ssl.c b/ssl.c index a4af6a5..276322f 100644 --- a/ssl.c +++ b/ssl.c @@ -957,21 +957,19 @@ verify_callback (int preverify_ok, X509_STORE_CTX * ctx) gc_free(&gc); } - if (system_ok (ret)) + if (ret) { msg (D_HANDSHAKE, "VERIFY SCRIPT OK: depth=%d, %s", ctx->error_depth, subject); } else { - if (!system_executed (ret)) - argv_msg_prefix (M_ERR, &argv, "Verify command failed to execute"); msg (D_HANDSHAKE, "VERIFY SCRIPT ERROR: depth=%d, %s", ctx->error_depth, subject); goto err; /* Reject connection */ } } - + /* check peer cert against CRL */ if (opt->crl_file) { @@ -3192,7 +3190,6 @@ verify_user_pass_script (struct tls_session *session, const struct user_pass *up struct gc_arena gc = gc_new (); struct argv argv = argv_new (); const char *tmp_file = ""; - int retval; bool ret = false; /* Is username defined? */ @@ -3230,16 +3227,11 @@ verify_user_pass_script (struct tls_session *session, const struct user_pass *up /* format command line */ argv_printf (&argv, "%sc %s", session->opt->auth_user_pass_verify_script, tmp_file); - + /* call command */ - retval = openvpn_run_script (&argv, session->opt->es, 0, "--auth-user-pass-verify"); + ret = openvpn_run_script (&argv, session->opt->es, 0, + "--auth-user-pass-verify"); - /* test return status of command */ - if (system_ok (retval)) - ret = true; - else if (!system_executed (retval)) - argv_msg_prefix (D_TLS_ERRORS, &argv, "TLS Auth Error: user-pass-verify script failed to execute"); - if (!session->opt->auth_user_pass_verify_script_via_file) setenv_del (session->opt->es, "password"); } -- cgit v1.2.3 From 935c62be9c0c8a256112df818bfb8470586a23b6 Mon Sep 17 00:00:00 2001 From: Emilien Mantel Date: Thu, 17 Jun 2010 21:38:59 +0200 Subject: Choose a different field in X509 to be username For my company, we use a PKI (linked to a LDAP) with OpenVPN. We can't use "CN" to be username (few people can have the same "CN"). In our case, we only use the UID. With my patch, you can choose another field to be username with a new option called --x509-username-field, the default value is "CN". Signed-off-by: Emilien Mantel Acked-by: David Sommerseth Signed-off-by: David Sommerseth --- options.c | 11 +++++++++++ options.h | 3 +++ ssl.c | 29 +++++++++++++++++------------ ssl.h | 7 +++++-- 4 files changed, 36 insertions(+), 14 deletions(-) diff --git a/options.c b/options.c index 86279e4..ea2dcbe 100644 --- a/options.c +++ b/options.c @@ -46,6 +46,7 @@ #include "helper.h" #include "manage.h" #include "configure.h" +#include #include "memdbg.h" @@ -500,6 +501,8 @@ static const char usage_message[] = "--key file : Local private key in .pem format.\n" "--pkcs12 file : PKCS#12 file containing local private key, local certificate\n" " and optionally the root CA certificate.\n" + "--x509-username-field : Field used in x509 certificat to be username.\n" + " Default is CN.\n" #ifdef WIN32 "--cryptoapicert select-string : Load the certificate and private key from the\n" " Windows Certificate System Store.\n" @@ -753,6 +756,7 @@ init_options (struct options *o, const bool init_gc) o->renegotiate_seconds = 3600; o->handshake_window = 60; o->transition_window = 3600; + o->x509_username_field = X509_USERNAME_FIELD_DEFAULT; #endif #endif #ifdef ENABLE_PKCS11 @@ -5667,6 +5671,13 @@ add_option (struct options *options, } options->key_method = key_method; } + else if (streq (p[0], "x509-username-field") && p[1]) + { + char *s = p[1]; + VERIFY_PERMISSION (OPT_P_GENERAL); + while ((*s = toupper(*s)) != '\0') s++; /* Uppercase if necessary */ + options->x509_username_field = p[1]; + } #endif /* USE_SSL */ #endif /* USE_CRYPTO */ #ifdef ENABLE_PKCS11 diff --git a/options.h b/options.h index a3114e2..10109e7 100644 --- a/options.h +++ b/options.h @@ -484,6 +484,9 @@ struct options within n seconds of handshake initiation. */ int handshake_window; + /* Field used to be the username in X509 cert. */ + char *x509_username_field; + /* Old key allowed to live n seconds after new key goes active */ int transition_window; diff --git a/ssl.c b/ssl.c index 276322f..970270b 100644 --- a/ssl.c +++ b/ssl.c @@ -730,6 +730,8 @@ get_peer_cert(X509_STORE_CTX *ctx, const char *tmp_dir, struct gc_arena *gc) return peercert_filename; } +char * x509_username_field; /* GLOBAL */ + /* * Our verify callback function -- check * that an incoming peer certificate is good. @@ -740,7 +742,7 @@ verify_callback (int preverify_ok, X509_STORE_CTX * ctx) { char *subject = NULL; char envname[64]; - char common_name[TLS_CN_LEN]; + char common_name[TLS_USERNAME_LEN]; SSL *ssl; struct tls_session *session; const struct tls_options *opt; @@ -772,18 +774,20 @@ verify_callback (int preverify_ok, X509_STORE_CTX * ctx) string_mod_sslname (subject, X509_NAME_CHAR_CLASS, opt->ssl_flags); string_replace_leading (subject, '-', '_'); - /* extract the common name */ - if (!extract_x509_field_ssl (X509_get_subject_name (ctx->current_cert), "CN", common_name, TLS_CN_LEN)) + /* extract the username (default is CN) */ + if (!extract_x509_field_ssl (X509_get_subject_name (ctx->current_cert), x509_username_field, common_name, TLS_USERNAME_LEN)) { if (!ctx->error_depth) - { - msg (D_TLS_ERRORS, "VERIFY ERROR: could not extract Common Name from X509 subject string ('%s') -- note that the Common Name length is limited to %d characters", - subject, - TLS_CN_LEN); - goto err; - } + { + msg (D_TLS_ERRORS, "VERIFY ERROR: could not extract %s from X509 subject string ('%s') -- note that the username length is limited to %d characters", + x509_username_field, + subject, + TLS_USERNAME_LEN); + goto err; + } } + string_mod_sslname (common_name, COMMON_NAME_CHAR_CLASS, opt->ssl_flags); cert_hash_remember (session, ctx->error_depth, ctx->current_cert->sha1_hash); @@ -1786,7 +1790,8 @@ init_ssl (const struct options *options) } else #endif - SSL_CTX_set_verify (ctx, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, + x509_username_field = (char *) options->x509_username_field; + SSL_CTX_set_verify (ctx, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, verify_callback); /* Connection information callback */ @@ -3594,9 +3599,9 @@ key_method_2_read (struct buffer *buf, struct tls_multi *multi, struct tls_sessi s2 = verify_user_pass_script (session, up); /* check sizing of username if it will become our common name */ - if ((session->opt->ssl_flags & SSLF_USERNAME_AS_COMMON_NAME) && strlen (up->username) >= TLS_CN_LEN) + if ((session->opt->ssl_flags & SSLF_USERNAME_AS_COMMON_NAME) && strlen (up->username) >= TLS_USERNAME_LEN) { - msg (D_TLS_ERRORS, "TLS Auth Error: --username-as-common name specified and username is longer than the maximum permitted Common Name length of %d characters", TLS_CN_LEN); + msg (D_TLS_ERRORS, "TLS Auth Error: --username-as-common name specified and username is longer than the maximum permitted Common Name length of %d characters", TLS_USERNAME_LEN); s1 = OPENVPN_PLUGIN_FUNC_ERROR; } diff --git a/ssl.h b/ssl.h index a22c854..93aac78 100644 --- a/ssl.h +++ b/ssl.h @@ -278,8 +278,8 @@ * Buffer sizes (also see mtu.h). */ -/* Maximum length of common name */ -#define TLS_CN_LEN 64 +/* Maximum length of the username in cert */ +#define TLS_USERNAME_LEN 64 /* Legal characters in an X509 or common name */ #define X509_NAME_CHAR_CLASS (CC_ALNUM|CC_UNDERBAR|CC_DASH|CC_DOT|CC_AT|CC_COLON|CC_SLASH|CC_EQUAL) @@ -288,6 +288,9 @@ /* Maximum length of OCC options string passed as part of auth handshake */ #define TLS_OPTIONS_LEN 512 +/* Default field in X509 to be username */ +#define X509_USERNAME_FIELD_DEFAULT "CN" + /* * Range of key exchange methods */ -- cgit v1.2.3 From 031d18fcb8a2a552aecabb41f1afdfe3f51bdd58 Mon Sep 17 00:00:00 2001 From: Emilien Mantel Date: Sat, 26 Jun 2010 13:56:48 +0200 Subject: Fixed static defined length check to use sizeof() This comes in addition to commit 935c62be9c0c8a256112d after some additional review comments. Signed-off-by: Emilien Mantel Acked-by: Peter Stuge Signed-off-by: David Sommerseth --- ssl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ssl.c b/ssl.c index 970270b..9780a67 100644 --- a/ssl.c +++ b/ssl.c @@ -775,7 +775,7 @@ verify_callback (int preverify_ok, X509_STORE_CTX * ctx) string_replace_leading (subject, '-', '_'); /* extract the username (default is CN) */ - if (!extract_x509_field_ssl (X509_get_subject_name (ctx->current_cert), x509_username_field, common_name, TLS_USERNAME_LEN)) + if (!extract_x509_field_ssl (X509_get_subject_name (ctx->current_cert), x509_username_field, common_name, sizeof(common_name))) { if (!ctx->error_depth) { -- cgit v1.2.3