Opened 6 years ago

Closed 3 years ago

#1079 closed Bug / Defect (fixed)

man page is misleading about --username-as-common-name option

Reported by: jpt Owned by:
Priority: major Milestone: release 2.5
Component: Documentation Version: OpenVPN 2.3.10 (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords:
Cc:

Description

The description for "--username-as-common-name" suggests that the substitution of username for common name will occur only when invoking the authentication script provided in the "--auth-user-pass-verify" option.

This is not true for 2 reasons:
1) the "common_name" environment variable still contains the CN (from certificate) when invoking the auth-user-pass-verify script;
2) the "common_name" environment variable contains the username (from username/password) when the "client-connect" script is invoked.

Suggested change:
as stated in https://sourceforge.net/p/openvpn/mailman/message/35261332/ :

"... just changing "For --auth-user-pass-verify authentication...” to "After --auth-user-pass-verify authentication…” and a note about this affecting the client-(dis)connect and client-config-dir options."

As a side note, this unexpected behavior is hard to sort out by googling, and it seems that it also applies to version 2.4 documentation (though I did not check the actual behavior of that version).

Change History (12)

comment:1 Changed 6 years ago by Gert Döring

Could you re-check with 2.4, please?

2.3 is out of maintenance and will not receive any updates except for security updates - so we won't investigate bug reports for 2.3 and won't issue patches for 2.3 either.

comment:2 Changed 6 years ago by Selva Nair

2.4.x and 2.5_git has the same wording. I had plans to update the man page (https://sourceforge.net/p/openvpn/mailman/message/35261332/) but failed to follow up..

Version 0, edited 6 years ago by Selva Nair (next)

comment:3 Changed 6 years ago by Antonio Quartulli

@selvanair do you have any proposed change for the manpage? My understanding is that the behaviour is now clear and this requires only some small rewording.

comment:4 Changed 6 years ago by Selva Nair

Current text in the man page is:

--username-as-commonname
For --auth-user-pass-verify authentication, use the authenticated username as the common name, rather than the common name from the client cert.

which is confusing and when the replacement happens is unclear. I suggest

"Use the authenticated username as the common name, rather than the common name from the client certificate. Requires that some form of auth-user-pass verification is in effect. As the replacement happens after auth-user-pass verification, the verification script or plugin will still receive the common name from the certificate.

The common_name environment variable passed to scripts and plugins invoked after authentication (e.g, client-connect script) and file names in client-config directory will match the username."

comment:5 Changed 6 years ago by Antonio Quartulli

"Requires that some form of auth-user-pass verification is in effect." Is this entirely true? I thought that the substitution would happen also when no auth-user-pass-verify is specified.

comment:6 Changed 6 years ago by Selva Nair

Then there is no username to substitute with. The replacement call happens from verify_user_pass() in ssl_verify.c

comment:7 Changed 6 years ago by Antonio Quartulli

hmpf..right. Ignore my comment.

comment:8 Changed 4 years ago by tct

CC

comment:9 Changed 4 years ago by tct

Milestone: release 2.5

comment:10 Changed 3 years ago by Gert Döring

Thanks!

commit 66ad8727935a371e237a5bada142c9f5f467c3f8 (master)
commit f9f5b4a307ddd59dd9eddcc869d05cc89dffbeb5 (release/2.5)
Author: Selva Nair
Date: Sun Sep 27 14:46:00 2020 -0400

Improve documentation of --username-as-common-name

Backport to 2.4? Or close?

(I expect us to release a 2.4.10 eventually, and possibly even a 2.4.11, so it's not totally futile)

comment:11 Changed 3 years ago by Selva Nair

2.4 would need a separate patch, not worth the trouble. So close?

comment:12 Changed 3 years ago by Gert Döring

Resolution: fixed
Status: newclosed

then, close. thanks.

Note: See TracTickets for help on using tickets.