openvpn-devel Mailing List for OpenVPN

Hi, as discussed today. Pretty much cut-and-paste from the ipv4 counterparts. gert -- USENET is *not* the non-clickable part of WWW! //www.muc.de/~gert/ Gert Doering - Munich, Germany ge...@gr... fax: +49-89-35655025 ge...@ne... 

Hi, On Sat, Feb 04, 2012 at 12:56:24PM +0000, Heiko Hund wrote: > Since the specific character classes for X.509 names are removed, the > "no-name-remapping" configuration option has no use anymore and is removed > as well. --no-name-remapping appears again somewhat further down in openvpn.8, we should remove it from there as well. ---------------- quote ---------------- String Types and Remapping In certain cases, OpenVPN will perform remapping of characters in strings. Essentially, any characters outside the set of permitted characters for each string type will be converted to underbar ('_'). Q: Why is string remapping necessary? A: It's an important security feature to prevent the malicious coding of strings from untrusted sources to be passed as parameters to scripts, saved in the environment, used as a common name, translated to a filename, etc. Q: Can string remapping be disabled? A: Yes, by using the --no-name-remapping option, however this should be considered an advanced option. ---------------- quote ---------------- (well, seems the whole section needs to be rewritten to reflect the new remapping rules [if any]) gert -- USENET is *not* the non-clickable part of WWW! //www.muc.de/~gert/ Gert Doering - Munich, Germany ge...@gr... fax: +49-89-35655025 ge...@ne... 

Hi, On Sat, Feb 04, 2012 at 02:16:13PM +0100, Adriaan de Jong wrote: > Removed done label and cleaned up return values. Boah, that code was ugly... ACK! gert -- USENET is *not* the non-clickable part of WWW! //www.muc.de/~gert/ Gert Doering - Munich, Germany ge...@gr... fax: +49-89-35655025 ge...@ne... 

Hi list, On Saturday 04 February 2012 12:56:24 Heiko Hund wrote: > The UTF-8 support that came with commit 2627335 does allow international > usernames and passwords. This patch introduces UTF-8 support for X.509 DNs. > Additionally, instead of using the legacy openssl format, DNs are now > displayed in RFC 2253 format; "/C=ru/L=\xD0\x9C\xD0\xBE\xD1\x81\xD0\xBA\xD0 > \xB2\xD0\xB0/O=\xD0\x9A\xD1\x80\xD0\xB5\xD0\xBC\xD0\xBB\xD1\x8C/CN=kreml.ru > " becomes "C=ru, L=Москва, O=Кремль, CN=kreml.ru". this is almost an 1:1 repost of this previous one (at http://thread.gmane.org/gmane.network.openvpn.devel/5112). The only thing that was changed is that openssl_get_subject() was moved to ssl_verify_openssl.[ch] as discussed at the FOSDEM meeting. Heiko -- Heiko Hund | Software Engineer | Phone +49-721-25516-237 | Fax -200 Astaro a Sophos Company | Amalienbadstr. 41 Bau 52 | 76227 Karlsruhe | Germany Commercial Register: Mannheim HRA 702710 | Headquarter Location: Karlsruhe Represented by the General Partner Astaro Verwaltungs GmbH Amalienbadstraße 41 Bau 52 | 76227 Karlsruhe | Germany Commercial Register: Mannheim HRB 708248 | Executive Board: Gert Hansen, Markus Hennig, Jan Hichert, Günter Junk, Dr. Frank Nellissen 

Removed done label and cleaned up return values. Signed-off-by: Adriaan de Jong <de...@fo...> --- ssl_verify.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ssl_verify.c b/ssl_verify.c index 326b005..feee124 100644 --- a/ssl_verify.c +++ b/ssl_verify.c @@ -684,14 +684,14 @@ verify_cert(struct tls_session *session, x509_cert_t *cert, int cert_depth) msg (D_HANDSHAKE, "VERIFY OK: depth=%d, %s", cert_depth, subject); session->verified = true; - done: x509_free_subject (subject); - return (session->verified == true) ? SUCCESS : FAILURE; + return SUCCESS; err: tls_clear_error(); session->verified = false; - goto done; + x509_free_subject (subject); + return FAILURE; } /* *************************************************************************** -- 1.7.5.4 

The UTF-8 support that came with commit 2627335 does allow international usernames and passwords. This patch introduces UTF-8 support for X.509 DNs. Additionally, instead of using the legacy openssl format, DNs are now displayed in RFC 2253 format; "/C=ru/L=\xD0\x9C\xD0\xBE\xD1\x81\xD0\xBA\xD0 \xB2\xD0\xB0/O=\xD0\x9A\xD1\x80\xD0\xB5\xD0\xBC\xD0\xBB\xD1\x8C/CN=kreml.ru" becomes "C=ru, L=Москва, O=Кремль, CN=kreml.ru". Since the specific character classes for X.509 names are removed, the "no-name-remapping" configuration option has no use anymore and is removed as well. Signed-off-by: Heiko Hund <hei...@so...> --- openvpn.8 | 23 +---------------------- options.c | 12 +----------- pkcs11_openssl.c | 2 +- sample-scripts/verify-cn | 6 +++--- ssl_verify.c | 45 +++++++++------------------------------------ ssl_verify_openssl.c | 40 ++++++++++++++++++++++++++++++++++++++-- ssl_verify_openssl.h | 2 ++ 7 files changed, 55 insertions(+), 75 deletions(-) diff --git a/openvpn.8 b/openvpn.8 index a6d7567..00acd01 100644 --- a/openvpn.8 +++ b/openvpn.8 @@ -3322,27 +3322,6 @@ the authenticated username as the common name, rather than the common name from the client cert. .\"********************************************************* .TP -.B \-\-no-name-remapping -Allow Common Name, X509 Subject, and username strings to include -any printable character including space, but excluding control -characters such as tab, newline, and carriage-return. - -By default, OpenVPN will remap -any character other than alphanumeric, underbar ('_'), dash -('-'), dot ('.'), and slash ('/') to underbar ('_'). The X509 -Subject string as returned by the -.B tls_id -environmental variable, can additionally contain colon (':') or -equal ('='). - -While name remapping is performed for security reasons to reduce -the possibility of introducing string expansion security vulnerabilities -in user-defined authentication -scripts, this option is provided for those cases where it is desirable to -disable the remapping feature. Don't use this option unless you -know what you are doing! -.\"********************************************************* -.TP .B \-\-port-share host port [dir] When run in TCP server mode, share the OpenVPN port with another application, such as an HTTPS server. If OpenVPN @@ -4463,7 +4442,7 @@ When .B cmd is executed two arguments are appended, as follows: -.B cmd certificate_depth X509_NAME_oneline +.B cmd certificate_depth subject These arguments are, respectively, the current certificate depth and the X509 common name (cn) of the peer. diff --git a/options.c b/options.c index cb9738a..6b8ae22 100644 --- a/options.c +++ b/options.c @@ -601,7 +601,7 @@ static const char usage_message[] = " pending TLS connection that has otherwise passed all other\n" " 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" + " executed as 'cmd certificate_depth subject')\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" @@ -2164,9 +2164,6 @@ options_postprocess_verify_ce (const struct options *options, const struct conne if ((options->ssl_flags & SSLF_AUTH_USER_PASS_OPTIONAL) && !ccnr) msg (M_USAGE, "--auth-user-pass-optional %s", postfix);	} - -	if ((options->ssl_flags & SSLF_NO_NAME_REMAPPING) && script_method == SM_SYSTEM) - msg (M_USAGE, "--script-security method='system' cannot be combined with --no-name-remapping"); } else { @@ -2201,8 +2198,6 @@ options_postprocess_verify_ce (const struct options *options, const struct conne	msg (M_USAGE, "--username-as-common-name requires --mode server"); if (options->ssl_flags & SSLF_AUTH_USER_PASS_OPTIONAL)	msg (M_USAGE, "--auth-user-pass-optional requires --mode server"); - if (options->ssl_flags & SSLF_NO_NAME_REMAPPING) -	msg (M_USAGE, "--no-name-remapping requires --mode server"); if (options->ssl_flags & SSLF_OPT_VERIFY)	msg (M_USAGE, "--opt-verify requires --mode server"); if (options->server_flags & SF_TCP_NODELAY_HELPER) @@ -5581,11 +5576,6 @@ add_option (struct options *options, VERIFY_PERMISSION (OPT_P_GENERAL); options->ssl_flags |= SSLF_AUTH_USER_PASS_OPTIONAL; } - else if (streq (p[0], "no-name-remapping")) - { - VERIFY_PERMISSION (OPT_P_GENERAL); - options->ssl_flags |= SSLF_NO_NAME_REMAPPING; - } else if (streq (p[0], "opt-verify")) { VERIFY_PERMISSION (OPT_P_GENERAL); diff --git a/pkcs11_openssl.c b/pkcs11_openssl.c index e3463dc..aa1eccc 100644 --- a/pkcs11_openssl.c +++ b/pkcs11_openssl.c @@ -129,7 +129,7 @@ pkcs11_certificate_dn (pkcs11h_certificate_t certificate, char *dn, goto cleanup; } - X509_NAME_oneline (X509_get_subject_name (x509), dn, dn_len); + _openssl_get_subject (x509, dn, dn_len); ret = 0; diff --git a/sample-scripts/verify-cn b/sample-scripts/verify-cn index f9fea0f..6e747ef 100755 --- a/sample-scripts/verify-cn +++ b/sample-scripts/verify-cn @@ -3,7 +3,7 @@ # verify-cn -- a sample OpenVPN tls-verify script # # Return 0 if cn matches the common name component of -# X509_NAME_oneline, 1 otherwise. +# subject, 1 otherwise. # # For example in OpenVPN, you could use the directive: # @@ -13,7 +13,7 @@ # the client common name is listed on a line in the # allowed_clients file. -die "usage: verify-cn cnfile certificate_depth X509_NAME_oneline" if (@ARGV != 3); +die "usage: verify-cn cnfile certificate_depth subject" if (@ARGV != 3); # Parse out arguments: # cnfile -- The file containing the list of common names, one per @@ -37,7 +37,7 @@ if ($depth == 0) { # If so, parse out the common name substring in # the X509 subject string. - if ($x509 =~ /\/CN=([^\/]+)/) { + if ($x509 =~ / CN=([^,]+)/) { $cn = $1;	# Accept the connection if the X509 common name	# string matches the passed cn argument. diff --git a/ssl_verify.c b/ssl_verify.c index 326b005..0b2a1fb 100644 --- a/ssl_verify.c +++ b/ssl_verify.c @@ -40,24 +40,9 @@ #include "ssl_verify_openssl.h" #endif -/** Legal characters in an X509 name */ -#define X509_NAME_CHAR_CLASS (CC_ALNUM|CC_UNDERBAR|CC_DASH|CC_DOT|CC_AT|CC_COLON|CC_SLASH|CC_EQUAL) - -/** Legal characters in a common name */ -#define COMMON_NAME_CHAR_CLASS (CC_ALNUM|CC_UNDERBAR|CC_DASH|CC_DOT|CC_AT|CC_SLASH) - /** Maximum length of common name */ #define TLS_USERNAME_LEN 64 -static void -string_mod_sslname (char *str, const unsigned int restrictive_flags, const unsigned int ssl_flags) -{ - if (ssl_flags & SSLF_NO_NAME_REMAPPING) - string_mod (str, CC_PRINT, CC_CRLF, '_'); - else - string_mod (str, restrictive_flags, 0, '_'); -} - /* * Export the untrusted IP address and port to the environment */ @@ -595,7 +580,7 @@ verify_cert(struct tls_session *session, x509_cert_t *cert, int cert_depth) } /* enforce character class restrictions in X509 name */ - string_mod_sslname (subject, X509_NAME_CHAR_CLASS, opt->ssl_flags); + string_mod (subject, CC_PRINT, CC_CRLF, '_'); string_replace_leading (subject, '-', '_'); /* extract the username (default is CN) */ @@ -615,7 +600,7 @@ verify_cert(struct tls_session *session, x509_cert_t *cert, int cert_depth) } /* enforce character class restrictions in common name */ - string_mod_sslname (common_name, COMMON_NAME_CHAR_CLASS, opt->ssl_flags); + string_mod (common_name, CC_PRINT, CC_CRLF, '_'); /* warn if cert chain is too deep */ if (cert_depth >= MAX_CERT_DEPTH) @@ -1005,7 +990,7 @@ verify_user_pass_script (struct tls_session *session, const struct user_pass *up * Verify the username and password using a plugin */ static int -verify_user_pass_plugin (struct tls_session *session, const struct user_pass *up, const char *raw_username) +verify_user_pass_plugin (struct tls_session *session, const struct user_pass *up) { int retval = OPENVPN_PLUGIN_FUNC_ERROR; struct key_state *ks = &session->key[KS_PRIMARY]; /* primary key */ @@ -1014,7 +999,7 @@ verify_user_pass_plugin (struct tls_session *session, const struct user_pass *up if ((session->opt->ssl_flags & SSLF_AUTH_USER_PASS_OPTIONAL) || strlen (up->username)) { /* set username/password in private env space */ - setenv_str (session->opt->es, "username", raw_username); + setenv_str (session->opt->es, "username", up->username); setenv_str (session->opt->es, "password", up->password); /* setenv incoming cert common name for script */ @@ -1038,7 +1023,6 @@ verify_user_pass_plugin (struct tls_session *session, const struct user_pass *up #endif setenv_del (session->opt->es, "password"); - setenv_str (session->opt->es, "username", up->username); } else { @@ -1059,7 +1043,7 @@ verify_user_pass_plugin (struct tls_session *session, const struct user_pass *up #define KMDA_DEF 3 static int -verify_user_pass_management (struct tls_session *session, const struct user_pass *up, const char *raw_username) +verify_user_pass_management (struct tls_session *session, const struct user_pass *up) { int retval = KMDA_ERROR; struct key_state *ks = &session->key[KS_PRIMARY]; /* primary key */ @@ -1068,7 +1052,7 @@ verify_user_pass_management (struct tls_session *session, const struct user_pass if ((session->opt->ssl_flags & SSLF_AUTH_USER_PASS_OPTIONAL) || strlen (up->username)) { /* set username/password in private env space */ - setenv_str (session->opt->es, "username", raw_username); + setenv_str (session->opt->es, "username", up->username); setenv_str (session->opt->es, "password", up->password); /* setenv incoming cert common name for script */ @@ -1081,7 +1065,6 @@ verify_user_pass_management (struct tls_session *session, const struct user_pass	management_notify_client_needing_auth (management, ks->mda_key_id, session->opt->mda_context, session->opt->es); setenv_del (session->opt->es, "password"); - setenv_str (session->opt->es, "username", up->username); retval = KMDA_SUCCESS; } @@ -1105,9 +1088,6 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi, bool s2 = true; struct key_state *ks = &session->key[KS_PRIMARY]; /* primary key */ - struct gc_arena gc = gc_new (); - char *raw_username; - #ifdef MANAGEMENT_DEF_AUTH int man_def_auth = KMDA_UNDEF; @@ -1115,22 +1095,17 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi, man_def_auth = KMDA_DEF; #endif - /* preserve raw username before string_mod remapping, for plugins */ - ALLOC_ARRAY_CLEAR_GC (raw_username, char, USER_PASS_LEN, &gc); - strcpy (raw_username, up->username); - string_mod (raw_username, CC_PRINT, CC_CRLF, '_'); - /* enforce character class restrictions in username/password */ - string_mod_sslname (up->username, COMMON_NAME_CHAR_CLASS, session->opt->ssl_flags); + string_mod (up->username, CC_PRINT, CC_CRLF, '_'); string_mod (up->password, CC_PRINT, CC_CRLF, '_'); /* call plugin(s) and/or script */ #ifdef MANAGEMENT_DEF_AUTH if (man_def_auth == KMDA_DEF) - man_def_auth = verify_user_pass_management (session, up, raw_username); + man_def_auth = verify_user_pass_management (session, up); #endif if (plugin_defined (session->opt->plugins, OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY)) - s1 = verify_user_pass_plugin (session, up, raw_username); + s1 = verify_user_pass_plugin (session, up); if (session->opt->auth_user_pass_verify_script) s2 = verify_user_pass_script (session, up); @@ -1179,8 +1154,6 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi, { msg (D_TLS_ERRORS, "TLS Auth Error: Auth Username/Password verification failed for peer"); } - - gc_free (&gc); } void diff --git a/ssl_verify_openssl.c b/ssl_verify_openssl.c index e82f6f9..e3f2d13 100644 --- a/ssl_verify_openssl.c +++ b/ssl_verify_openssl.c @@ -247,16 +247,52 @@ x509_free_sha1_hash (unsigned char *hash) } char * +_openssl_get_subject (X509 *cert, char *buf, int size) +{ + BIO *subject_bio; + BUF_MEM *subject_mem; + char *subject = buf; + int maxlen = size; + + subject_bio = BIO_new (BIO_s_mem ()); + if (subject_bio == NULL) + goto out; + + X509_NAME_print_ex (subject_bio, X509_get_subject_name (cert), + 0, XN_FLAG_SEP_CPLUS_SPC | XN_FLAG_FN_SN | + ASN1_STRFLGS_UTF8_CONVERT | ASN1_STRFLGS_ESC_CTRL); + + if (BIO_eof (subject_bio)) + goto out_free; + + BIO_get_mem_ptr (subject_bio, &subject_mem); + if (subject == NULL) + { + maxlen = subject_mem->length + 1; + subject = malloc (maxlen); + check_malloc_return (subject); + } + + memcpy (subject, subject_mem->data, maxlen); + subject[maxlen - 1] = '\0'; + +out_free: + BIO_free (subject_bio); +out: + return subject; +} + +char * x509_get_subject (X509 *cert) { - return X509_NAME_oneline (X509_get_subject_name (cert), NULL, 0); + return _openssl_get_subject (cert, NULL, 0); } void x509_free_subject (char *subject) { if (subject) - OPENSSL_free(subject); + free(subject); } diff --git a/ssl_verify_openssl.h b/ssl_verify_openssl.h index 4814d30..9c76d34 100644 --- a/ssl_verify_openssl.h +++ b/ssl_verify_openssl.h @@ -69,4 +69,6 @@ int verify_callback (int preverify_ok, X509_STORE_CTX * ctx); /** @} name Function for authenticating a new connection from a remote OpenVPN peer */ +char *_openssl_get_subject (X509 *cert, char *buf, int size); + #endif /* SSL_VERIFY_OPENSSL_H_ */ -- 1.7.8.3 

On 03/02/12 18:12, Gert Doering wrote: > Hi, > > On Fri, Feb 03, 2012 at 04:19:31PM +0100, Gert Doering wrote: >> On Fri, Feb 03, 2012 at 03:10:37PM +0100, Michael wrote: >>> the patch does not remove >>> >>> +++ b/push.c >>> @@ -245,8 +245,9 @@ send_push_reply (struct context *c) >>> /* TODO: push "/netbits" as well, to allow non-/64 subnet sizes >>> * (needs changes in options.c, options.h, and other >>> places) >>> */ > > Now it does :-) - thanks for pointing this out. v2 patch attached. > > "git --reset" is my friend ;-) ACK. Applied to master branch on -testing and stable. commit c55e9562d64f381ba46b83a02503f6239e23d3ef Author: Gert Doering <ge...@gr...> Date: Fri Feb 3 17:11:03 2012 +0100 Implement IPv6 interface config with non-/64 prefix lengths. Signed-off-by: Gert Doering <ge...@gr...> Acked-by: David Sommerseth <da...@re...> Signed-off-by: David Sommerseth <da...@re...> kind regards, David Sommerseth 

On 03/02/12 18:21, Gert Doering wrote: > Hi, > > last patch for "sudo functionality in t_client.sh" was incomplete, and > for some reason I didn't notice on the test system - fell on my feet on > the freebsd 7.4 test client, and is now fixed for good :-) > > gert ACK. Applied to master branch on -testing and -stable. commit fc3ee19dee6c66e2325a24e864b5328128404e83 Author: Gert Doering <ge...@gr...> Date: Fri Feb 3 19:18:07 2012 +0200 Fix RUN_SUDO functionality for t_client.sh Signed-off-by: Gert Doering <ge...@gr...> Acked-by: David Sommerseth <da...@re...> Signed-off-by: David Sommerseth <da...@re...> kind regards, David Sommerseth