| Message ID | 20181008181618.8976-2-arne@rfc2549.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | None | expand |
Hi, On Mon, Oct 08, 2018 at 08:16:16PM +0200, Arne Schwabe wrote: > There is no user facing way to enable this feature and way that feature > works (username build from MAC of primary net device) is questionable. > > It also does not compile anymore. Feature-ACK, but the patch itself puzzles me. > @@ -455,51 +455,6 @@ get_auth_challenge(const char *auth_challenge, struct gc_arena *gc) > > #endif /* ifdef ENABLE_MANAGEMENT */ > > -#if AUTO_USERID [..] > diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c > index 0a947c6e..5a136d69 100644 > --- a/src/openvpn/ssl.c > +++ b/src/openvpn/ssl.c > @@ -410,9 +410,6 @@ auth_user_pass_setup(const char *auth_file, const struct static_challenge_info * > auth_user_pass_enabled = true; > if (!auth_user_pass.defined && !auth_token.defined) > { > -#if AUTO_USERID > - get_user_pass_auto_userid(&auth_user_pass, auth_file); > -#else > #ifdef ENABLE_MANAGEMENT These hunks have "ENABLE_MANAGEMENT" in the context, while all my branches (2.3, 2.4, master) have "ENABLE_CLIENT_CR" here. This is not really needed to remove the (questionable) feature - and I can just adjust the patch on the fly - but I wonder where these are coming from in your tree, and whether I missed (or messed up) something... gert
Acked-by: Gert Doering <gert@greenie.muc.de> For the reasons given - it's code that has not been activated anywhere in the last 5+ years, there is no way to turn it on by configure, and it's likely not working right on half the platforms. And less #ifdef! I had to whack the patch to make it apply - no changes to the actual code, but the context was full of ENABLE_MANAGEMENT where our tree has ENABLE_CLIENT_CR. I seem to remember that there was a patch to get rid of the letter (because it is auto-defined #if ENABLE_MANAGEMENT) but we seem to have lost track of it... only your tree remembers. Your patch has been applied to the master branch. commit 00d78cd57fa52aa5a65df1707922791e72313663 Author: Arne Schwabe Date: Mon Oct 8 20:16:16 2018 +0200 Remove AUTO_USERID feature Acked-by: Gert Doering <gert@greenie.muc.de> Message-Id: <20181008181618.8976-2-arne@rfc2549.org> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17664.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
Am 08.10.18 um 21:37 schrieb Gert Doering: > Hi, > > On Mon, Oct 08, 2018 at 08:16:16PM +0200, Arne Schwabe wrote: >> There is no user facing way to enable this feature and way that feature >> works (username build from MAC of primary net device) is questionable. >> >> It also does not compile anymore. > > Feature-ACK, but the patch itself puzzles me. > >> @@ -455,51 +455,6 @@ get_auth_challenge(const char *auth_challenge, struct gc_arena *gc) >> >> #endif /* ifdef ENABLE_MANAGEMENT */ >> >> -#if AUTO_USERID > [..] >> diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c >> index 0a947c6e..5a136d69 100644 >> --- a/src/openvpn/ssl.c >> +++ b/src/openvpn/ssl.c >> @@ -410,9 +410,6 @@ auth_user_pass_setup(const char *auth_file, const struct static_challenge_info * >> auth_user_pass_enabled = true; >> if (!auth_user_pass.defined && !auth_token.defined) >> { >> -#if AUTO_USERID >> - get_user_pass_auto_userid(&auth_user_pass, auth_file); >> -#else >> #ifdef ENABLE_MANAGEMENT > > These hunks have "ENABLE_MANAGEMENT" in the context, while all my branches > (2.3, 2.4, master) have "ENABLE_CLIENT_CR" here. > > This is not really needed to remove the (questionable) feature - and I can > just adjust the patch on the fly - but I wonder where these are coming > from in your tree, and whether I missed (or messed up) something... > You are missing patch 1/4: [PATCH 1/4] Remove MANAGMENT_EXTERNAL_KEY, MANAGMENT_IN_EXTRA, ENABLE_CLIENT_CR Is being held until the list moderator can review it for approval. The reason it is being held: Message body is too big: 309191 bytes with a limit of 256 KB But since it now does not apply anymore since 2/4 got in first, I will resend that mail and canceld the message request. Arne
On 08/10/18 21:47, Gert Doering wrote: > Acked-by: Gert Doering <gert@greenie.muc.de> > > For the reasons given - it's code that has not been activated anywhere > in the last 5+ years, there is no way to turn it on by configure, and > it's likely not working right on half the platforms. And less #ifdef! And to add a bit more background; we discussed this with James directly, who also said this AUTO_USERID feature is not used anywhere he was aware of; most likely for specific use case for a side project he was involved in. Anyhow, James didn't think this feature had any value any more. Arne: Lately you've often forgotten to add -s to git commit. Please double check your patches in this regard.
diff --git a/src/openvpn/errlevel.h b/src/openvpn/errlevel.h index 5ca4fa8f..c30284fc 100644 --- a/src/openvpn/errlevel.h +++ b/src/openvpn/errlevel.h @@ -139,7 +139,6 @@ #define D_PACKET_TRUNC_DEBUG LOGLEV(7, 70, M_DEBUG) /* PACKET_TRUNCATION_CHECK verbose */ #define D_PING LOGLEV(7, 70, M_DEBUG) /* PING send/receive messages */ #define D_PS_PROXY_DEBUG LOGLEV(7, 70, M_DEBUG) /* port share proxy debug */ -#define D_AUTO_USERID LOGLEV(7, 70, M_DEBUG) /* AUTO_USERID debugging */ #define D_TLS_KEYSELECT LOGLEV(7, 70, M_DEBUG) /* show information on key selection for data channel */ #define D_ARGV_PARSE_CMD LOGLEV(7, 70, M_DEBUG) /* show parse_line() errors in argv_parse_cmd */ #define D_CRYPTO_DEBUG LOGLEV(7, 70, M_DEBUG) /* show detailed info from crypto.c routines */ diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index 51d539d2..75f4ff47 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -455,51 +455,6 @@ get_auth_challenge(const char *auth_challenge, struct gc_arena *gc) #endif /* ifdef ENABLE_MANAGEMENT */ -#if AUTO_USERID - -void -get_user_pass_auto_userid(struct user_pass *up, const char *tag) -{ - struct gc_arena gc = gc_new(); - struct buffer buf; - uint8_t macaddr[6]; - static uint8_t digest [MD5_DIGEST_LENGTH]; - static const uint8_t hashprefix[] = "AUTO_USERID_DIGEST"; - - const md_kt_t *md5_kt = md_kt_get("MD5"); - md_ctx_t *ctx; - - CLEAR(*up); - buf_set_write(&buf, (uint8_t *)up->username, USER_PASS_LEN); - buf_printf(&buf, "%s", TARGET_PREFIX); - if (get_default_gateway_mac_addr(macaddr)) - { - dmsg(D_AUTO_USERID, "GUPAU: macaddr=%s", format_hex_ex(macaddr, sizeof(macaddr), 0, 1, ":", &gc)); - ctx = md_ctx_new(); - md_ctx_init(ctx, md5_kt); - md_ctx_update(ctx, hashprefix, sizeof(hashprefix) - 1); - md_ctx_update(ctx, macaddr, sizeof(macaddr)); - md_ctx_final(ctx, digest); - md_ctx_cleanup(ctx); - md_ctx_free(ctx); - buf_printf(&buf, "%s", format_hex_ex(digest, sizeof(digest), 0, 256, " ", &gc)); - } - else - { - buf_printf(&buf, "UNKNOWN"); - } - if (tag && strcmp(tag, "stdin")) - { - buf_printf(&buf, "-%s", tag); - } - up->defined = true; - gc_free(&gc); - - dmsg(D_AUTO_USERID, "GUPAU: AUTO_USERID: '%s'", up->username); -} - -#endif /* if AUTO_USERID */ - void purge_user_pass(struct user_pass *up, const bool force) { diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h index 7092685f..fad53de8 100644 --- a/src/openvpn/misc.h +++ b/src/openvpn/misc.h @@ -158,11 +158,6 @@ void configure_path(void); const char *sanitize_control_message(const char *str, struct gc_arena *gc); -#if AUTO_USERID -void get_user_pass_auto_userid(struct user_pass *up, const char *tag); - -#endif - /* * /sbin/ip path, may be overridden */ diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 0a947c6e..5a136d69 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -410,9 +410,6 @@ auth_user_pass_setup(const char *auth_file, const struct static_challenge_info * auth_user_pass_enabled = true; if (!auth_user_pass.defined && !auth_token.defined) { -#if AUTO_USERID - get_user_pass_auto_userid(&auth_user_pass, auth_file); -#else #ifdef ENABLE_MANAGEMENT if (auth_challenge) /* dynamic challenge/response */ { @@ -438,7 +435,6 @@ auth_user_pass_setup(const char *auth_file, const struct static_challenge_info * else #endif /* ifdef ENABLE_MANAGEMENT */ get_user_pass(&auth_user_pass, auth_file, UP_TYPE_AUTH, GET_USER_PASS_MANAGEMENT); -#endif /* if AUTO_USERID */ } } diff --git a/src/openvpn/syshead.h b/src/openvpn/syshead.h index 807f7b9b..d2a50341 100644 --- a/src/openvpn/syshead.h +++ b/src/openvpn/syshead.h @@ -643,15 +643,6 @@ socket_defined(const socket_descriptor_t sd) #define CONNECT_NONBLOCK #endif -/* - * Do we have the capability to support the AUTO_USERID feature? - */ -#if defined(ENABLE_AUTO_USERID) -#define AUTO_USERID 1 -#else -#define AUTO_USERID 0 -#endif - /* * Compression support */