security/openvpn: reliability fixes cherry-picked from upstream

Arne Schwabe's OpenSSL fix for Debian Bug#958296
"Fix tls_ctx_client/server_new leaving error on OpenSSL error stack"
<https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=958296> [1]

Selva Nair's auth-pam fixes
"Parse static challenge response in auth-pam plugin"
"Accept empty password and/or response in auth-pam plugin"

Re-diff (with make makepatch) older patches.

Reported by:	Jonas Andradas via Debian BTS
Obtained from:	Arne Schwabe, Selva Nair <https://github.com/OpenVPN/openvpn/tree/release/2.4>
MFH:		2020Q2 (blanket for backporting reliability fixes)
This commit is contained in:
Matthias Andree 2020-05-07 16:28:42 +00:00
parent c552da0a20
commit ec578cb332
Notes: svn2git 2021-03-31 03:12:20 +00:00
svn path=/head/; revision=534272
6 changed files with 329 additions and 6 deletions

View File

@ -3,7 +3,7 @@
PORTNAME= openvpn
DISTVERSION= 2.4.9
PORTREVISION?= 0
PORTREVISION?= 1
CATEGORIES= security net net-vpn
MASTER_SITES= https://swupdate.openvpn.org/community/releases/ \
https://build.openvpn.net/downloads/releases/ \

View File

@ -1,6 +1,6 @@
--- configure.orig 2019-02-20 12:28:34 UTC
--- configure.orig 2020-04-16 13:26:53 UTC
+++ configure
@@ -18120,8 +18120,6 @@ fi
@@ -18226,8 +18226,6 @@ fi
$as_echo "!! WARNING !! The cmoka git submodule has not been initialized or updated. Unit testing cannot be performed." >&6; }
fi
else

View File

@ -0,0 +1,214 @@
From b89e48b015e581a4a0f5c306e2ab20da34c862ea Mon Sep 17 00:00:00 2001
From: Selva Nair <selva.nair@gmail.com>
Date: Tue, 24 Jul 2018 22:34:53 -0400
Subject: [PATCH] Parse static challenge response in auth-pam plugin
If static challenge is in use, the password passed to the plugin by openvpn
is of the form "SCRV1:base64-pass:base64-response". Parse this string to
separate it into password and response and use them to respond to queries
in the pam conversation function.
On the plugin parameters line the substitution keyword for the static
challenge response is "OTP". For example, for pam config named "test" that
prompts for "user", "password" and "pin", use
plugin openvpn-auth-pam.so "test user USERNAME password PASSWORD pin OTP"
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1532486093-24793-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17307.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 7369d01bf360bcfa02f26c05b86dde5496d120f6)
---
src/plugins/auth-pam/README.auth-pam | 15 ++++--
src/plugins/auth-pam/auth-pam.c | 75 +++++++++++++++++++++++++++-
2 files changed, 84 insertions(+), 6 deletions(-)
diff --git a/src/plugins/auth-pam/README.auth-pam b/src/plugins/auth-pam/README.auth-pam
index e12369021..908156542 100644
--- a/src/plugins/auth-pam/README.auth-pam
+++ ./src/plugins/auth-pam/README.auth-pam
@@ -36,19 +36,20 @@ pairs to answer PAM module queries.
For example:
- plugin openvpn-auth-pam.so "login login USERNAME password PASSWORD"
+ plugin openvpn-auth-pam.so "login login USERNAME password PASSWORD pin OTP"
tells auth-pam to (a) use the "login" PAM module, (b) answer a
-"login" query with the username given by the OpenVPN client, and
-(c) answer a "password" query with the password given by the
-OpenVPN client. This provides flexibility in dealing with the different
+"login" query with the username given by the OpenVPN client,
+(c) answer a "password" query with the password, and (d) answer a
+"pin" query with the OTP given by the OpenVPN client.
+This provides flexibility in dealing with different
types of query strings which different PAM modules might generate.
For example, suppose you were using a PAM module called
"test" which queried for "name" rather than "login":
plugin openvpn-auth-pam.so "test name USERNAME password PASSWORD"
-While "USERNAME" "COMMONNAME" and "PASSWORD" are special strings which substitute
+While "USERNAME" "COMMONNAME" "PASSWORD" and "OTP" are special strings which substitute
to client-supplied values, it is also possible to name literal values
to use as PAM module query responses. For example, suppose that the
login module queried for a third parameter, "domain" which
@@ -61,6 +62,10 @@ the operation of this plugin:
client-cert-not-required
username-as-common-name
+ static-challenge
+
+Use of --static challenege is required to pass a pin (represented by "OTP" in
+parameter substituion) or a second password.
Run OpenVPN with --verb 7 or higher to get debugging output from
this plugin, including the list of queries presented by the
diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth-pam.c
index 5ba4dc4cb..1324307f1 100644
--- a/src/plugins/auth-pam/auth-pam.c
+++ ./src/plugins/auth-pam/auth-pam.c
@@ -6,6 +6,7 @@
* packet compression.
*
* Copyright (C) 2002-2018 OpenVPN Inc <sales@openvpn.net>
+ * Copyright (C) 2016-2018 Selva Nair <selva.nair@gmail.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
@@ -64,6 +65,7 @@
/* Pointers to functions exported from openvpn */
static plugin_secure_memzero_t plugin_secure_memzero = NULL;
+static plugin_base64_decode_t plugin_base64_decode = NULL;
/*
* Plugin state, used by foreground
@@ -87,6 +89,7 @@ struct auth_pam_context
* "USERNAME" -- substitute client-supplied username
* "PASSWORD" -- substitute client-specified password
* "COMMONNAME" -- substitute client certificate common name
+ * "OTP" -- substitute static challenge response if available
*/
#define N_NAME_VALUE 16
@@ -111,6 +114,7 @@ struct user_pass {
char username[128];
char password[128];
char common_name[128];
+ char response[128];
const struct name_value_list *name_value_list;
};
@@ -276,6 +280,66 @@ name_value_match(const char *query, const char *match)
return strncasecmp(match, query, strlen(match)) == 0;
}
+/*
+ * Split and decode up->password in the form SCRV1:base64_pass:base64_response
+ * into pass and response and save in up->password and up->response.
+ * If the password is not in the expected format, input is not changed.
+ */
+static void
+split_scrv1_password(struct user_pass *up)
+{
+ const int skip = strlen("SCRV1:");
+ if (strncmp(up->password, "SCRV1:", skip) != 0)
+ {
+ return;
+ }
+
+ char *tmp = strdup(up->password);
+ if (!tmp)
+ {
+ fprintf(stderr, "AUTH-PAM: out of memory parsing static challenge password\n");
+ goto out;
+ }
+
+ char *pass = tmp + skip;
+ char *resp = strchr(pass, ':');
+ if (!resp) /* string not in SCRV1:xx:yy format */
+ {
+ goto out;
+ }
+ *resp++ = '\0';
+
+ int n = plugin_base64_decode(pass, up->password, sizeof(up->password)-1);
+ if (n > 0)
+ {
+ up->password[n] = '\0';
+ n = plugin_base64_decode(resp, up->response, sizeof(up->response)-1);
+ if (n > 0)
+ {
+ up->response[n] = '\0';
+ if (DEBUG(up->verb))
+ {
+ fprintf(stderr, "AUTH-PAM: BACKGROUND: parsed static challenge password\n");
+ }
+ goto out;
+ }
+ }
+
+ /* decode error: reinstate original value of up->password and return */
+ plugin_secure_memzero(up->password, sizeof(up->password));
+ plugin_secure_memzero(up->response, sizeof(up->response));
+ strcpy(up->password, tmp); /* tmp is guaranteed to fit in up->password */
+
+ fprintf(stderr, "AUTH-PAM: base64 decode error while parsing static challenge password\n");
+
+out:
+ if (tmp)
+ {
+ plugin_secure_memzero(tmp, strlen(tmp));
+ free(tmp);
+ }
+}
+
OPENVPN_EXPORT int
openvpn_plugin_open_v3(const int v3structver,
struct openvpn_plugin_args_open_in const *args,
@@ -316,6 +380,7 @@ openvpn_plugin_open_v3(const int v3structver,
/* Save global pointers to functions exported from openvpn */
plugin_secure_memzero = args->callbacks->plugin_secure_memzero;
+ plugin_base64_decode = args->callbacks->plugin_base64_decode;
/*
* Make sure we have two string arguments: the first is the .so name,
@@ -599,6 +664,10 @@ my_conv(int n, const struct pam_message **msg_array,
{
aresp[i].resp = searchandreplace(match_value, "COMMONNAME", up->common_name);
}
+ else if (strstr(match_value, "OTP"))
+ {
+ aresp[i].resp = searchandreplace(match_value, "OTP", up->response);
+ }
else
{
aresp[i].resp = strdup(match_value);
@@ -787,6 +856,9 @@ pam_server(int fd, const char *service, int verb, const struct name_value_list *
#endif
}
+ /* If password is of the form SCRV1:base64:base64 split it up */
+ split_scrv1_password(&up);
+
if (pam_auth(service, &up)) /* Succeeded */
{
if (send_control(fd, RESPONSE_VERIFY_SUCCEEDED) == -1)
@@ -818,10 +890,11 @@ pam_server(int fd, const char *service, int verb, const struct name_value_list *
command);
goto done;
}
+ plugin_secure_memzero(up.response, sizeof(up.response));
}
done:
-
plugin_secure_memzero(up.password, sizeof(up.password));
+ plugin_secure_memzero(up.response, sizeof(up.response));
#ifdef USE_PAM_DLOPEN
dlclose_pam();
#endif

View File

@ -0,0 +1,40 @@
From cab48ad43eaba51c54fa23e55b0b2eb436dd921f Mon Sep 17 00:00:00 2001
From: Selva Nair <selva.nair@gmail.com>
Date: Tue, 7 Aug 2018 22:44:31 -0400
Subject: [PATCH] Accept empty password and/or response in auth-pam plugin
In the auth-pam plugin correctly parse the static challenge string
even when password or challenge response is empty.
Whether an empty user input is an error is determined by the PAM
conversation function depending on whether the PAM module queries
for it or not.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1533696271-21799-2-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17382.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 7a8109023f4c345fe12f23421c5fa7e88e1ea85b)
---
src/plugins/auth-pam/auth-pam.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth-pam.c
index 1324307f1..88b53204b 100644
--- a/src/plugins/auth-pam/auth-pam.c
+++ ./src/plugins/auth-pam/auth-pam.c
@@ -310,11 +310,11 @@ split_scrv1_password(struct user_pass *up)
*resp++ = '\0';
int n = plugin_base64_decode(pass, up->password, sizeof(up->password)-1);
- if (n > 0)
+ if (n >= 0)
{
up->password[n] = '\0';
n = plugin_base64_decode(resp, up->response, sizeof(up->response)-1);
- if (n > 0)
+ if (n >= 0)
{
up->response[n] = '\0';
if (DEBUG(up->verb))

View File

@ -1,6 +1,6 @@
--- src/openvpn/openssl_compat.h.orig 2019-02-20 12:28:23 UTC
--- src/openvpn/openssl_compat.h.orig 2020-04-16 13:26:45 UTC
+++ src/openvpn/openssl_compat.h
@@ -735,7 +735,7 @@ SSL_CTX_get_max_proto_version(SSL_CTX *ctx)
@@ -747,7 +747,7 @@ SSL_CTX_get_max_proto_version(SSL_CTX *ctx)
}
#endif /* SSL_CTX_get_max_proto_version */
@ -9,7 +9,7 @@
/** Mimics SSL_CTX_set_min_proto_version for OpenSSL < 1.1 */
static inline int
SSL_CTX_set_min_proto_version(SSL_CTX *ctx, long tls_ver_min)
@@ -764,7 +764,7 @@ SSL_CTX_set_min_proto_version(SSL_CTX *ctx, long tls_v
@@ -776,7 +776,7 @@ SSL_CTX_set_min_proto_version(SSL_CTX *ctx, long tls_v
}
#endif /* SSL_CTX_set_min_proto_version */

View File

@ -0,0 +1,69 @@
In the corner case that the global OpenSSL has an invalid command like
MinProtocol = TLSv1.0
(Due to OpenSSL's idiosyncrasies MinProtocol = TLSv1 would be correct)
the SSL_ctx_new function leaves the errors for parsing the config file
on the stack.
OpenSSL: error:14187180:SSL routines:ssl_do_config:bad value
Since the later functions, especially the one of loading the
certificates expected a clean error this error got reported at the
wrong place.
Print the warnings with crypto_msg when we detect that we are in this
situation (this also clears the stack).
---
src/openvpn/ssl_openssl.c | 10 ++++++++++
1 file changed, 10 insertions(+)
Acked-by: Gert Doering <gert@greenie.muc.de>
"Explanation and Code make sense, Debian testing confirmed it fixes
the problem observed" (which was a user error in the end, but led to an
unexpected error in openvpn).
Basic client test run with openssl 1.1.1 on Linux/Gentoo.
Your patch has been applied to the master and release/2.4 branch.
commit 75aa88af774abaa168bf72e43e1dbb57be14c044 (master)
commit 125654bfa6f99a251b581522182e85748dd8043a (release/2.4)
Author: Arne Schwabe
Date: Tue Apr 21 12:11:22 2020 +0200
Fix tls_ctx_client/server_new leaving error on OpenSSL error stack
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200421101122.24284-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19802.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--- src/openvpn/ssl_openssl.c.orig 2020-04-16 13:26:45 UTC
+++ src/openvpn/ssl_openssl.c
@@ -110,6 +110,11 @@ tls_ctx_server_new(struct tls_root_ctx *ctx)
{
crypto_msg(M_FATAL, "SSL_CTX_new SSLv23_server_method");
}
+ if (ERR_peek_error() != 0)
+ {
+ crypto_msg(M_WARN, "Warning: TLS server context initialisation "
+ "has warnings.");
+ }
}
void
@@ -122,6 +127,11 @@ tls_ctx_client_new(struct tls_root_ctx *ctx)
if (ctx->ctx == NULL)
{
crypto_msg(M_FATAL, "SSL_CTX_new SSLv23_client_method");
+ }
+ if (ERR_peek_error() != 0)
+ {
+ crypto_msg(M_WARN, "Warning: TLS client context initialisation "
+ "has warnings.");
}
}