On 30/09/2020 15:13, Arne Schwabe wrote:
> Signed-off-by: Arne Schwabe <arne@rfc2549.org>
> ---
> doc/man-sections/generic-options.rst | 3 +-
> include/openvpn-plugin.h.in | 8 ++
> src/openvpn/ssl.c | 2 +-
> src/openvpn/ssl_common.h | 1 +
> src/openvpn/ssl_verify.c | 165 ++++++++++++++++++++++++---
> src/openvpn/ssl_verify.h | 2 +-
> 6 files changed, 165 insertions(+), 16 deletions(-)
>
So far just glared at the code, but the change below needs to be fixed
first. This patchset has also aged so much it does no longer apply on
top of latest git master.
[...snip...]
> diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
> index e7e62afa..fc3a1116 100644
> --- a/src/openvpn/ssl_verify.c
> +++ b/src/openvpn/ssl_verify.c[...snip...]
> @@ -1067,7 +1196,7 @@ verify_user_pass_script(struct tls_session *session, struct tls_multi *multi,
> struct gc_arena gc = gc_new();
> struct argv argv = argv_new();
> const char *tmp_file = "";
> - bool ret = false;
> + bool ret = OPENVPN_PLUGIN_FUNC_ERROR;
This is wrong. OPENVPN_PLUGIN_FUNC_ERROR is 1, which means "true". I
see this is being corrected again in the next patch; but lets make it
correct from the beginning to avoid making a potential bisect in the
future more confusing than needed.
The rest of the code looks reasonable. I've not tested it yet, as there
are some merge conflicts now. Since the surrounding code has changed a
bit since this patch series , I consider it a bit risky to conclude on
testing this on a older code base without many of the fixes in between
in place.
Most of the merge conflicts is probably related to commit 99d217b20064
(removing --disable-def-auth), but there are other AUTH related changes
as well. This needs to be carefully tested with all these auth changes
in place too.
@@ -409,7 +409,8 @@ which mode OpenVPN is configured as.
* :code:`OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY` plug-in hooks returns
success/failure via :code:`auth_control_file` when using deferred auth
- method
+ method and pending authentification via :code:`pending_auth_file`.
+
* :code:`OPENVPN_PLUGIN_ENABLE_PF` plugin hook to pass filtering rules
via ``pf_file``
@@ -567,6 +567,14 @@ OPENVPN_PLUGIN_DEF openvpn_plugin_handle_t OPENVPN_PLUGIN_FUNC(openvpn_plugin_op
* auth_control_file/client_connect_deferred_file
* in the environmental variable list (envp).
*
+ * Additionally the auth_pending_file can be written, which causes the openvpn
+ * server to send a pending auth request to the client. See doc/management.txt
+ * for more details on this authentication mechanism. The format of the
+ * auth_pending_file is
+ * line 1: timeout in seconds
+ * line 2: Pending auth method the client needs to support (e.g. openurl)
+ * line 3: EXTRA (e.g. OPEN_URL:http://www.example.com)
+ *
* In addition the OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER and
* OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER_V2 are called when OpenVPN tries to
* get the deferred result. For a V2 call implementing this function is
@@ -994,7 +994,7 @@ key_state_free(struct key_state *ks, bool clear)
packet_id_free(&ks->crypto_options.packet_id);
#ifdef PLUGIN_DEF_AUTH
- key_state_rm_auth_control_file(ks);
+ key_state_rm_auth_control_files(ks);
#endif
if (clear)
@@ -216,6 +216,7 @@ struct key_state
unsigned int auth_control_status;
time_t acf_last_mod;
char *auth_control_file;
+ char *auth_pending_file;
#endif
};
@@ -843,13 +843,129 @@ man_def_auth_test(const struct key_state *ks)
#endif /* ifdef MANAGEMENT_DEF_AUTH */
#ifdef PLUGIN_DEF_AUTH
+/**
+ * Removes auth_pending file from the file system
+ * and key_state structure
+ */
+static void
+key_state_rm_auth_pending_file(struct key_state *ks)
+{
+ if (ks && ks->auth_pending_file)
+ {
+ platform_unlink(ks->auth_pending_file);
+ free(ks->auth_pending_file);
+ ks->auth_pending_file = NULL;
+ }
+}
-/*
- * auth_control_file functions
+/**
+ * Check peer_info if the client supports the requested pending auth method
*/
+static bool
+check_auth_pending_method(const char *peer_info, const char *method)
+{
+ struct gc_arena gc = gc_new();
+
+ char *iv_sso = extract_var_peer_info(peer_info, "IV_SSO=", &gc);
+ if (!iv_sso)
+ {
+ gc_free(&gc);
+ return false;
+ }
+
+ const char *client_method = strtok(iv_sso, ",");
+ bool supported = false;
+
+ while (client_method)
+ {
+ if (0 == strcmp(client_method, method))
+ {
+ supported = true;
+ break;
+ }
+ client_method = strtok(NULL, ":");
+ }
+
+ gc_free(&gc);
+ return supported;
+}
+
+/**
+ * Checks if the deferred state should also send auth pending
+ * request to the client. Also removes the auth_pending control file
+ *
+ * @returns true if file was either processed sucessfully or did not
+ * exist at all
+ * @returns false The file had an invlaid format or another error occured
+ */
+static bool
+key_state_check_auth_pending_file(struct key_state *ks,
+ struct tls_multi *multi)
+{
+ bool ret = true;
+ if (ks && ks->auth_pending_file)
+ {
+ struct buffer_list *lines = buffer_list_file(ks->auth_pending_file,
+ 1024);
+ if (lines && lines->head)
+ {
+ /* Must have at least three lines. further lines are ignored for
+ * forward compatibility */
+ if (!lines->head || !lines->head->next || !lines->head->next->next)
+ {
+ msg(M_WARN, "auth pending control file is not at least "
+ "three lines long.");
+ buffer_list_free(lines);
+ return false;
+ }
+ struct buffer *timeout_buf = &lines->head->buf;
+ struct buffer *iv_buf = &lines->head->next->buf;
+ struct buffer *extra_buf = &lines->head->next->next->buf;
+
+ /* Remove newline chars at the end of the lines */
+ buf_chomp(timeout_buf);
+ buf_chomp(iv_buf);
+ buf_chomp(extra_buf);
+
+ long timeout = strtol(BSTR(timeout_buf), NULL, 10);
+ if (timeout == 0)
+ {
+ msg(M_WARN, "could not parse auth pending file timeout");
+ buffer_list_free(lines);
+ return false;
+ }
+
+ const char* pending_method = BSTR(iv_buf);
+ if (!check_auth_pending_method(multi->peer_info, pending_method))
+ {
+ char buf[128];
+ openvpn_snprintf(buf, sizeof(buf),
+ "Authentication failed, required pending auth "
+ "method '%s' not supported", pending_method);
+ auth_set_client_reason(multi, buf);
+ msg(M_INFO, "Client does not supported auth pending method "
+ "'%s'", pending_method);
+ ret = false;
+ }
+ else
+ {
+ send_auth_pending_messages(multi, BSTR(extra_buf), timeout);
+ }
+ }
+
+ buffer_list_free(lines);
+ }
+ key_state_rm_auth_pending_file(ks);
+ return ret;
+}
+
+/**
+ * Removes auth_pending and auth_control files from file system
+ * and key_state structure
+ */
void
-key_state_rm_auth_control_file(struct key_state *ks)
+key_state_rm_auth_control_files(struct key_state *ks)
{
if (ks && ks->auth_control_file)
{
@@ -857,23 +973,34 @@ key_state_rm_auth_control_file(struct key_state *ks)
free(ks->auth_control_file);
ks->auth_control_file = NULL;
}
+ key_state_rm_auth_pending_file(ks);
}
+/**
+ * Generates and creates the control files used for deferred authentification
+ * in the temporary directory.
+ *
+ * @return true if file creation was successful
+ */
static bool
-key_state_gen_auth_control_file(struct key_state *ks, const struct tls_options *opt)
+key_state_gen_auth_control_files(struct key_state *ks, const struct tls_options *opt)
{
struct gc_arena gc = gc_new();
- key_state_rm_auth_control_file(ks);
+ key_state_rm_auth_control_files(ks);
const char *acf = platform_create_temp_file(opt->tmp_dir, "acf", &gc);
- if (acf)
+ const char *apf = platform_create_temp_file(opt->tmp_dir, "apf", &gc);
+
+ if (acf && apf)
{
ks->auth_control_file = string_alloc(acf, NULL);
+ ks->auth_pending_file = string_alloc(apf, NULL);
setenv_str(opt->es, "auth_control_file", ks->auth_control_file);
+ setenv_str(opt->es, "auth_pending_file", ks->auth_pending_file);
}
gc_free(&gc);
- return acf;
+ return (acf && apf);
}
static unsigned int
@@ -905,6 +1032,8 @@ key_state_test_auth_control_file(struct key_state *ks)
return ACF_DISABLED;
}
+
+
#endif /* ifdef PLUGIN_DEF_AUTH */
/*
@@ -1067,7 +1196,7 @@ verify_user_pass_script(struct tls_session *session, struct tls_multi *multi,
struct gc_arena gc = gc_new();
struct argv argv = argv_new();
const char *tmp_file = "";
- bool ret = false;
+ bool ret = OPENVPN_PLUGIN_FUNC_ERROR;
/* Is username defined? */
if ((session->opt->ssl_flags & SSLF_AUTH_USER_PASS_OPTIONAL) || strlen(up->username))
@@ -1172,7 +1301,7 @@ verify_user_pass_plugin(struct tls_session *session, struct tls_multi *multi,
add_session_token_env(session, multi, up);
#ifdef PLUGIN_DEF_AUTH
/* generate filename for deferred auth control file */
- if (!key_state_gen_auth_control_file(ks, session->opt))
+ if (!key_state_gen_auth_control_files(ks, session->opt))
{
msg(D_TLS_ERRORS, "TLS Auth Error (%s): "
"could not create deferred auth control file", __func__);
@@ -1184,10 +1313,20 @@ verify_user_pass_plugin(struct tls_session *session, struct tls_multi *multi,
retval = plugin_call(session->opt->plugins, OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY, NULL, NULL, session->opt->es);
#ifdef PLUGIN_DEF_AUTH
- /* purge auth control filename (and file itself) for non-deferred returns */
- if (retval != OPENVPN_PLUGIN_FUNC_DEFERRED)
+ if (retval == OPENVPN_PLUGIN_FUNC_DEFERRED)
+ {
+ /* Check if we the plugin has written the pending auth control
+ * file and send the pending auth to the client */
+ if(!key_state_check_auth_pending_file(ks, multi))
+ {
+ retval = OPENVPN_PLUGIN_FUNC_ERROR;
+ key_state_rm_auth_control_files(ks);
+ }
+ }
+ else
{
- key_state_rm_auth_control_file(ks);
+ /* purge auth control filename (and file itself) for non-deferred returns */
+ key_state_rm_auth_control_files(ks);
}
#endif
@@ -1256,7 +1395,7 @@ verify_user_pass_management(struct tls_session *session,
#endif /* ifdef MANAGEMENT_DEF_AUTH */
-/*
+/**
* Main username/password verification entry point
*
* Will set session->ks[KS_PRIMARY].authenticated according to
@@ -93,7 +93,7 @@ int tls_authentication_status(struct tls_multi *multi, const int latency);
*
* @param ks The key state the remove the file for
*/
-void key_state_rm_auth_control_file(struct key_state *ks);
+void key_state_rm_auth_control_files(struct key_state *ks);
/**
* Frees the given set of certificate hashes.