| Message ID | 20260316224531.315912-1-luca.boccassi@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Openvpn-devel] Fix password usage with ENABLE_PKCS11 | expand |
Am 16.03.2026 um 23:43 schrieb luca.boccassi@gmail.com: > From: Luca Boccassi <luca.boccassi@gmail.com> > > With ENABLE_PKCS11 USER_PASS_LEN is set to 4096 bytes. > > But if a user specifies a large password, as allowed by the limit, > there are two places in the client that fail to handle it: > > - in the TLS handling functions a single read/write is done into/out of > the TLS layer, using a fixed size buffer at 2048 bytes, so the rest > of the password is truncated > - in the management protocol when processing a "password" field another > fixed buffer at 256 bytes is used, so the rest of the password is > truncated > > Use int_max(xyz, USER_PASS_LEN) to define these buffers. In normal builds > the current hardcoded values will stay the same, as they are higher. > In ENABLE_PKCS11 builds the 4096 value will be higher which will allow > longer passwords to work when communicating to the server and in the > management protocol. > > Testing new client/old server shows no issues with a password that follows > the existing 2048 bytes limit. In order to use a > 2048 bytes password, > the server needs to be updated to this change too, otherwise it will truncate > it, but that is the same as status quo: only passwords < 2048 bytes work, so > there should be no regression. > > This enables the use case where JIT use-once tokens are used as passwords. > > Signed-off-by: Luca Boccassi <luca.boccassi@gmail.com> > --- > Alternative to: > > https://sourceforge.net/p/openvpn/mailman/openvpn-devel/thread/20260315230620.1594780-1-luca.boccassi%40gmail.com/#msg59309452 > > Instead of hardcoding larger buffers, simply use USER_PASS_LEN, so that > builds without ENABLE_PKCS11 are not affected. > > With these small changes, it is possible to successfully > connect to an Azure VPN endpoint using the OpenVPN 2.7 client, > using a dummy username, an Entra token as password and the > server-secret from the azvpn XML config that users get as tls-auth > key. > > src/openvpn/common.h | 2 +- > src/openvpn/manage.c | 2 +- > src/openvpn/options.h | 4 ++-- > src/openvpn/options_parse.c | 2 +- > 4 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/src/openvpn/common.h b/src/openvpn/common.h > index aa7b7217..e1096b98 100644 > --- a/src/openvpn/common.h > +++ b/src/openvpn/common.h > @@ -67,7 +67,7 @@ typedef unsigned long ptr_type; > * maximum size of a single TLS message (cleartext). > * This parameter must be >= PUSH_BUNDLE_SIZE > */ > -#define TLS_CHANNEL_BUF_SIZE 2048 > +#define TLS_CHANNEL_BUF_SIZE max_int(2048, USER_PASS_LEN) " This would still mean that builds that use ENABLE_PKCS11 are potientially incompatible with builds without that flag and with older version. Arne
On Mon, Mar 16, 2026 at 6:48 PM <luca.boccassi@gmail.com> wrote: > > From: Luca Boccassi <luca.boccassi@gmail.com> > > With ENABLE_PKCS11 USER_PASS_LEN is set to 4096 bytes. > > But if a user specifies a large password, as allowed by the limit, > there are two places in the client that fail to handle it: > The misunderstanding here is the assumption that ENABLE_PKCS11 builds were supposed to support username/password > 128. They do not -- the 4096 number was added for some local usage (like private key passwords and PINs, or some misunderstanding, I guess), not meant to apply to password data to be passed from client to server or other way -- that never worked. As you have already seen, there are existing limits in multiple places that make larger than 128 byte username/password not to work reliably (or possibly 255). Management and option parsing always had 255 byte limitations, the 2048 tls-channel-buffer limits password+username to about 1600 bytes or thereabout. I'm not saying we should not support larger passwords. In fact I do feel that allowing the management interface to pass longer passwords could be a useful goal if it can be done with minimal disruption (say keep username + password fit in current control TLS buffers). But even there, the "right" way to do it may be to use multi-line base64 as we already do in the daemon -> client direction. If anyone has "passwords" longer than 1k bytes or so, they are doing something wrong and fixing it in the wrong place. "I have no access to the broken piece of software, so let us fix it here" is not going to be acceptable. In any case, this has to be done carefully. There are a number of implicit "protocol" changes in your patch: (i) option parsing size -- are we going to define a new version of config files that support longer lines? Looks like a wrong solution. (ii) management interface -- do we support this only for UI client-versions > x? How would the UI client know whether the daemon will accept long lines? (iii) control channel --- how to ensure backward compatibility? I do not see your patches addressing any of these questions. Hence the pushback. As Arne suggested, consider applying it locally for personal use if it works in your case. Selva PS. This is not the first time this issue has come up -- search openvpn devel list archives for past discussions. Here are some: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16674.html https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg12535.html Selva
diff --git a/src/openvpn/common.h b/src/openvpn/common.h index aa7b7217..e1096b98 100644 --- a/src/openvpn/common.h +++ b/src/openvpn/common.h @@ -67,7 +67,7 @@ typedef unsigned long ptr_type; * maximum size of a single TLS message (cleartext). * This parameter must be >= PUSH_BUNDLE_SIZE */ -#define TLS_CHANNEL_BUF_SIZE 2048 +#define TLS_CHANNEL_BUF_SIZE max_int(2048, USER_PASS_LEN) /* TLS control buffer minimum size * diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c index df72f15f..ea9bd8b0 100644 --- a/src/openvpn/manage.c +++ b/src/openvpn/manage.c @@ -2655,7 +2655,7 @@ man_connection_init(struct management *man) * Allocate helper objects for command line input and * command output from/to the socket. */ - man->connection.in = command_line_new(1024); + man->connection.in = command_line_new(max_int(1024, USER_PASS_LEN)); man->connection.out = buffer_list_new(); /* diff --git a/src/openvpn/options.h b/src/openvpn/options.h index 3d8b5059..a46c0d7c 100644 --- a/src/openvpn/options.h +++ b/src/openvpn/options.h @@ -53,8 +53,8 @@ /* * Max size of options line and parameter. */ -#define OPTION_PARM_SIZE 256 -#define OPTION_LINE_SIZE 256 +#define OPTION_PARM_SIZE max_int(256, USER_PASS_LEN) +#define OPTION_LINE_SIZE max_int(256, USER_PASS_LEN) extern const char title_string[]; diff --git a/src/openvpn/options_parse.c b/src/openvpn/options_parse.c index cb51ad24..1372a5fb 100644 --- a/src/openvpn/options_parse.c +++ b/src/openvpn/options_parse.c @@ -374,7 +374,7 @@ read_config_file(struct options *options, const char *file, int level, const cha int offset = 0; CLEAR(p); ++line_num; - if (strlen(line) == OPTION_LINE_SIZE) + if ((int)strlen(line) == OPTION_LINE_SIZE) { msg(msglevel, "In %s:%d: Maximum option line length (%d) exceeded, line starts with %s",