Attention is currently required from: Bluca, selvanair.

plaisthos has posted comments on this change by Bluca. ( 
http://gerrit.openvpn.net/c/openvpn/+/1622?usp=email )

Change subject: Add new helpers to handle key exchange (S_SENT_KEY/S_START) 
with large passwords
......................................................................


Patch Set 2: Code-Review-1

(5 comments)

Patchset:

PS2:
I still do not agree with the approach as such but here are additional some 
comments on the patch itself.


File src/openvpn/common.h:

http://gerrit.openvpn.net/c/openvpn/+/1622/comment/a14781fe_894ca37b?usp=email :
PS2, Line 76:  * than the default TLS channel size (e.g. with ENABLE_PKCS11),
So an OpenVPN compiled without --enable-pkcs11 gets a buffer of 2048 + 2 * 128? 
That feels a bit odd.


File src/openvpn/ssl.c:

http://gerrit.openvpn.net/c/openvpn/+/1622/comment/a7bf99f1_be645d0f?usp=email :
PS2, Line 919:     }
there are other reasons why a handshake can fail at this point. Printing a 
message that suggest that probably password is at fault can be misleading.


http://gerrit.openvpn.net/c/openvpn/+/1622/comment/ffbce5ce_262ab2e5?usp=email :
PS2, Line 2645:  * when long passwords/tokens are used.
So this method will basically greedily read as many TLS records as there are 
currently on the wire. This will make future improvements a lot harder where 
there are server/clients out there that basically ignore the framing and 
consider anything that is sent in the same batch to be part of the key method.

Even the original Microsoft patch does not do that. It still uses only one TLS 
record (max 16kB) for key2 method.

The correct approach here would be to try to read a whole TLS record (16 kB) 
and take whatever the SSL_read gives you as the key2 method and not doing 
multiple smaller reads that break the assumption of TLS record framing that 
OpenVPN (currently) uses.

I am surprised that this did not already cause problems for you when the server 
sends push reply immediately after its key2 write and this is being put into 
the key packet. But maybe you never got such unlucky timing.


http://gerrit.openvpn.net/c/openvpn/+/1622/comment/9a4c89e5_f09f9549?usp=email :
PS2, Line 2940:             BLEN(&ks->key_method_send_buf) > 
TLS_CHANNEL_BUF_SIZE;
This can also be triggered by other methods like setting multiple setenv UV_xx 
with push-peerinfo in the configuration and then the user will get the error 
message iwth the long password above.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1622?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I055c64ca8b23066e70eea7d7deddfb14f5354c5f
Gerrit-Change-Number: 1622
Gerrit-PatchSet: 2
Gerrit-Owner: Bluca <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-Reviewer: selvanair <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: Bluca <[email protected]>
Gerrit-Attention: selvanair <[email protected]>
Gerrit-Comment-Date: Wed, 08 Apr 2026 02:07:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to