| Message ID | 20260315230620.1594780-2-luca.boccassi@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Two small fixes for auth via tokens | expand |
Am 16.03.26 um 00:05 schrieb luca.boccassi@gmail.com: > From: Luca Boccassi <luca.boccassi@gmail.com> > > When authenticating via a JWT token 2048 bytes are not enough, which > breaks the auth process. In my local case the token is ~2100 bytes. > Bump the maximum harcoded size from 2k to 8k to leave some headroom. > > Signed-off-by: Luca Boccassi <luca.boccassi@gmail.com> > --- > src/openvpn/common.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/openvpn/common.h b/src/openvpn/common.h > index aa7b7217..fbe6239a 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 8192 > > /* TLS control buffer minimum size > * This gets a definitive NACK from me. This will cause compatibility problems. It might work for your small setup where you can easily patch both sides but it is not a good when compatibility with other clients is important. We understand that user+pass is limited but this is not the right approach to overcome this limitation. Arne
On Mon, 16 Mar 2026 at 12:00, Arne Schwabe <arne@rfc2549.org> wrote: > > Am 16.03.26 um 00:05 schrieb luca.boccassi@gmail.com: > > From: Luca Boccassi <luca.boccassi@gmail.com> > > > > When authenticating via a JWT token 2048 bytes are not enough, which > > breaks the auth process. In my local case the token is ~2100 bytes. > > Bump the maximum harcoded size from 2k to 8k to leave some headroom. > > > > Signed-off-by: Luca Boccassi <luca.boccassi@gmail.com> > > --- > > src/openvpn/common.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/openvpn/common.h b/src/openvpn/common.h > > index aa7b7217..fbe6239a 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 8192 > > > > /* TLS control buffer minimum size > > * > > This gets a definitive NACK from me. This will cause compatibility > problems. It might work for your small setup where you can easily patch > both sides but it is not a good when compatibility with other clients is > important. > > We understand that user+pass is limited but this is not the right > approach to overcome this limitation. Hi, Could you please clarify what you mean by compatibility problems? And what would be the right approach instead to let the client process larger passwords? Thanks
> > Could you please clarify what you mean by compatibility problems? And > what would be the right approach instead to let the client process > larger passwords? Thanks By compatibility problem I mean that a peer that is modified like you are suggesting will not be able to communicate to other peers without that patch. This should be pretty obvious from the way you are changing the code that you allow a send buffer of 8k but do not take into account that peers without that patch only have 2k to receive. Arne
On Mon, 16 Mar 2026 at 12:05, Arne Schwabe <arne@rfc2549.org> wrote: > > Could you please clarify what you mean by compatibility problems? And > > what would be the right approach instead to let the client process > > larger passwords? Thanks > > By compatibility problem I mean that a peer that is modified like you > are suggesting will not be able to communicate to other peers without > that patch. > > This should be pretty obvious from the way you are changing the code > that you allow a send buffer of 8k but do not take into account that > peers without that patch only have 2k to receive. Thanks, first time I work on this repo so trying to get familiar with these details, I'll look into it. The issue here is that the handshake fails, as the azvpn server expects a full token to be received, but it gets truncated. I have no control over the server, just the client. Do you have a suggestion on how to solve this in a way that would be acceptable?
> > The issue here is that the handshake fails, as the azvpn server > expects a full token to be received, but it gets truncated. I have no > control over the server, just the client. Do you have a suggestion on > how to solve this in a way that would be acceptable? I have no Idea what the azvpn server is and what they are doing but it looks like they are using some hacks that were never submitted upstream and also had no communication with our project. So it is not surprising that this does not work with the normal open source client. Arne
On Mon, 16 Mar 2026 at 12:11, Arne Schwabe <arne@rfc2549.org> wrote: > > The issue here is that the handshake fails, as the azvpn server > > expects a full token to be received, but it gets truncated. I have no > > control over the server, just the client. Do you have a suggestion on > > how to solve this in a way that would be acceptable? > > I have no Idea what the azvpn server is and what they are doing but it > looks like they are using some hacks that were never submitted upstream > and also had no communication with our project. So it is not surprising > that this does not work with the normal open source client. Oh it's even worse than that, I assure you :-) It's an internal reimplementation, it does not use the open source server sadly. But it is the openvpn protocol, and it works once the allowance for a large password is made. I am trying to get the standard network-manager + openvpn setup to work with it, to at least be spared from having to use the absolutely horrendous proprietary client that comes with it. In case you are curious, the network-manager-openvpn side is here: https://gitlab.gnome.org/bluca/NetworkManager-openvpn/-/commits/entra_auth?ref_type=heads With these changes, it works nicely. Of course I'm more than happy to get the client changes here in a form that is acceptable to you, but would really appreciate guidance, as you can tell I am completely new to this project. The only limitation I have is that I can't change how the server works (I wish...), but on the client side I am open to anything that works. Thanks for the feedback!
Am 16.03.26 um 13:17 schrieb Luca Boccassi: > > Oh it's even worse than that, I assure you :-) It's an internal > reimplementation, it does not use the open source server sadly. But it > is the openvpn protocol, and it works once the allowance for a large > password is made. > > I am trying to get the standard network-manager + openvpn setup to > work with it, to at least be spared from having to use the absolutely > horrendous proprietary client that comes with it. > In case you are curious, the network-manager-openvpn side is here: > https://gitlab.gnome.org/bluca/NetworkManager-openvpn/-/commits/entra_auth?ref_type=heads > With these changes, it works nicely. > > Of course I'm more than happy to get the client changes here in a form > that is acceptable to you, but would really appreciate guidance, as > you can tell I am completely new to this project. The only limitation > I have is that I can't change how the server works (I wish...), but on > the client side I am open to anything that works. Yeah the problem is that they made changes that are not really acceptable in general for our project. You basically would need a special azvpn mode in OpenVPN that is only activated via a configuration option or similar and that would also have a lot of code overhead to enable/disable this mode in various parts of OpenVPN. And we need some strong reason to include such a hack mode that we need to maintain then forever in our code base. I am afraid that for now using a specially patched version is probably easier/better to do. Arne
On Mon, 16 Mar 2026 at 12:30, Arne Schwabe <arne@rfc2549.org> wrote: > > Am 16.03.26 um 13:17 schrieb Luca Boccassi: > > > > Oh it's even worse than that, I assure you :-) It's an internal > > reimplementation, it does not use the open source server sadly. But it > > is the openvpn protocol, and it works once the allowance for a large > > password is made. > > > > I am trying to get the standard network-manager + openvpn setup to > > work with it, to at least be spared from having to use the absolutely > > horrendous proprietary client that comes with it. > > In case you are curious, the network-manager-openvpn side is here: > > https://gitlab.gnome.org/bluca/NetworkManager-openvpn/-/commits/entra_auth?ref_type=heads > > With these changes, it works nicely. > > > > Of course I'm more than happy to get the client changes here in a form > > that is acceptable to you, but would really appreciate guidance, as > > you can tell I am completely new to this project. The only limitation > > I have is that I can't change how the server works (I wish...), but on > > the client side I am open to anything that works. > > Yeah the problem is that they made changes that are not really > acceptable in general for our project. You basically would need a > special azvpn mode in OpenVPN that is only activated via a configuration > option or similar and that would also have a lot of code overhead to > enable/disable this mode in various parts of OpenVPN. And we need some > strong reason to include such a hack mode that we need to maintain then > forever in our code base. > > I am afraid that for now using a specially patched version is probably > easier/better to do. Unfortunately a specially patched version cannot really work, as this really needs to work with the packaged client in normal distros. Can't tell other people to do their own building and patching and security maintenance and so on... But I'm not sure why a special mode would be needed in the first place, with additional burden to support? There are only 2 changes needed, and both have to do with ensuring the password size that can already be supported (4096) actually works through the stack. Right now the client you can install on Ubuntu or Fedora or any other distro, where it is built with PKCS11 enabled, takes 4k passwords already, so that part works, as the second patch of the series is really a cleanup. The issue is that there are 2 places in the client where this breaks: 1) The TLS buffer as per the first patch, as the client doesn't do multiple passes, but instead expect a single write to the TLS layer to be sufficient, but it supports 2k instead of the 4k that is the max password size 2) The management protocol that network-manager uses to configure the password at runtime, which takes 256 bytes at most in a command, as per the third patch So the goal here would be simply to ensure the client works with the existing max password size. Please let me know if I am missing something, but as far as I can tell, I have tried to check for compatibility issues, but I could not see one with the TLS buffer increase. I ran an unmodified 2.7.0 server, and a modified client, using TLS, and the client could connect just fine to the unmodified server, even if the TLS buffer is larger - it's just a buffer after all. The only scenario I can think of that won't be compatible, is if someone tries to send a password larger than 2K - but that is already broken today? So in what way would that be a compatibility issue, given it already doesn't work, despite the max password size already being 4K? Is there a situation in which this would be a problem in a way that it isn't today that I am missing? This is the config I tested with, server: dev tun topology subnet server 10.99.99.0 255.255.255.0 proto tcp-server port 11940 ca /tmp/ovpn-compat-test/ca.crt cert /tmp/ovpn-compat-test/server.crt key /tmp/ovpn-compat-test/server.key tls-auth /tmp/ovpn-compat-test/ta.key 0 cipher AES-256-GCM auth SHA256 verb 4 keepalive 10 60 script-security 2 auth-user-pass-verify /bin/true via-env username-as-common-name verify-client-cert none client: client dev tun proto tcp-client remote 127.0.0.1 11940 ca /tmp/ovpn-compat-test/ca.crt cert /tmp/ovpn-compat-test/client.crt key /tmp/ovpn-compat-test/client.key tls-auth /tmp/ovpn-compat-test/ta.key 1 cipher AES-256-GCM auth SHA256 auth-user-pass /tmp/ovpn-compat-test/userpass.txt verb 4 nobind connect-timeout 10 For the management protocol, if increasing the options size is not ok, how about adding a new 'password-long' or somesuch option to the protocol, that allows receiving a password that is up to the current max of 4k? Happy to work on this if it is preferable. Thanks for your feedback!
On Mon, 16 Mar 2026 at 20:50, Luca Boccassi <luca.boccassi@gmail.com> wrote: > > On Mon, 16 Mar 2026 at 12:30, Arne Schwabe <arne@rfc2549.org> wrote: > > > > Am 16.03.26 um 13:17 schrieb Luca Boccassi: > > > > > > Oh it's even worse than that, I assure you :-) It's an internal > > > reimplementation, it does not use the open source server sadly. But it > > > is the openvpn protocol, and it works once the allowance for a large > > > password is made. > > > > > > I am trying to get the standard network-manager + openvpn setup to > > > work with it, to at least be spared from having to use the absolutely > > > horrendous proprietary client that comes with it. > > > In case you are curious, the network-manager-openvpn side is here: > > > https://gitlab.gnome.org/bluca/NetworkManager-openvpn/-/commits/entra_auth?ref_type=heads > > > With these changes, it works nicely. > > > > > > Of course I'm more than happy to get the client changes here in a form > > > that is acceptable to you, but would really appreciate guidance, as > > > you can tell I am completely new to this project. The only limitation > > > I have is that I can't change how the server works (I wish...), but on > > > the client side I am open to anything that works. > > > > Yeah the problem is that they made changes that are not really > > acceptable in general for our project. You basically would need a > > special azvpn mode in OpenVPN that is only activated via a configuration > > option or similar and that would also have a lot of code overhead to > > enable/disable this mode in various parts of OpenVPN. And we need some > > strong reason to include such a hack mode that we need to maintain then > > forever in our code base. > > > > I am afraid that for now using a specially patched version is probably > > easier/better to do. > > Unfortunately a specially patched version cannot really work, as this > really needs to work with the packaged client in normal distros. Can't > tell other people to do their own building and patching and security > maintenance and so on... > > But I'm not sure why a special mode would be needed in the first > place, with additional burden to support? > > There are only 2 changes needed, and both have to do with ensuring the > password size that can already be supported (4096) actually works > through the stack. Right now the client you can install on Ubuntu or > Fedora or any other distro, where it is built with PKCS11 enabled, > takes 4k passwords already, so that part works, as the second patch of > the series is really a cleanup. The issue is that there are 2 places > in the client where this breaks: > > 1) The TLS buffer as per the first patch, as the client doesn't do > multiple passes, but instead expect a single write to the TLS layer to > be sufficient, but it supports 2k instead of the 4k that is the max > password size > 2) The management protocol that network-manager uses to configure the > password at runtime, which takes 256 bytes at most in a command, as > per the third patch > > So the goal here would be simply to ensure the client works with the > existing max password size. > > Please let me know if I am missing something, but as far as I can > tell, I have tried to check for compatibility issues, but I could not > see one with the TLS buffer increase. I ran an unmodified 2.7.0 > server, and a modified client, using TLS, and the client could connect > just fine to the unmodified server, even if the TLS buffer is larger - > it's just a buffer after all. > > The only scenario I can think of that won't be compatible, is if > someone tries to send a password larger than 2K - but that is already > broken today? So in what way would that be a compatibility issue, > given it already doesn't work, despite the max password size already > being 4K? Is there a situation in which this would be a problem in a > way that it isn't today that I am missing? > > This is the config I tested with, server: > > dev tun > topology subnet > server 10.99.99.0 255.255.255.0 > proto tcp-server > port 11940 > ca /tmp/ovpn-compat-test/ca.crt > cert /tmp/ovpn-compat-test/server.crt > key /tmp/ovpn-compat-test/server.key > tls-auth /tmp/ovpn-compat-test/ta.key 0 > cipher AES-256-GCM > auth SHA256 > verb 4 > keepalive 10 60 > script-security 2 > auth-user-pass-verify /bin/true via-env > username-as-common-name > verify-client-cert none > > client: > > client > dev tun > proto tcp-client > remote 127.0.0.1 11940 > ca /tmp/ovpn-compat-test/ca.crt > cert /tmp/ovpn-compat-test/client.crt > key /tmp/ovpn-compat-test/client.key > tls-auth /tmp/ovpn-compat-test/ta.key 1 > cipher AES-256-GCM > auth SHA256 > auth-user-pass /tmp/ovpn-compat-test/userpass.txt > verb 4 > nobind > connect-timeout 10 > > For the management protocol, if increasing the options size is not ok, > how about adding a new 'password-long' or somesuch option to the > protocol, that allows receiving a password that is up to the current > max of 4k? > Happy to work on this if it is preferable. > > Thanks for your feedback! I've sent a simplified and reduced revision that instead of hardcoding new sizes, simply uses the existing max(<existing>, USER_PASS_LEN) (without changing it) itself to define those 3 buffers. That way there are no changes unless USER_PASS_LEN is set to a large value. Hopefully this is more palatable - but as before, I am more than happy to work on alternative implementations, as mentioned above, if there are any suggestions. Thanks!
diff --git a/src/openvpn/common.h b/src/openvpn/common.h index aa7b7217..fbe6239a 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 8192 /* TLS control buffer minimum size *