[Openvpn-devel,2/4] Remove AUTO_USERID feature
Commit Message
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.
---
src/openvpn/errlevel.h | 1 -
src/openvpn/misc.c | 45 ------------------------------------------
src/openvpn/misc.h | 5 -----
src/openvpn/ssl.c | 4 ----
src/openvpn/syshead.h | 9 ---------
5 files changed, 64 deletions(-)
Comments
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.
@@ -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 */
@@ -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)
{
@@ -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
*/
@@ -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 */
}
}
@@ -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
*/