Skip to contentSkip to conversation

Have the same username/password length regardless of PKCS#11 enablement

PATCH In review David Sommerseth 6 participants 8 messages Sep 22, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Have the same username/password length regardless of PKCS#11 enablement

If running an OpenVPN client with --enable-pkcs11 and a server without and having a username and/or password with more than 128 characters, the authentication will fail as the server truncates the password to 128 bytes.

This makes things easier and more predictable. Username/passwords can be up to 4096 bytes, regardless of the --enable-pkcs11 state.

---
 src/openvpn/misc.h | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
src/openvpn/misc.h +3 -6
index b694096..31ea10e 100644
@@ -195,17 +195,14 @@ const char *hostname_randomize(const char *hostname, struct gc_arena *gc);
  * Get and store a username/password
  */
 
+/* max length of username/password */
+#define USER_PASS_LEN 4096
+
 struct user_pass
 {
   bool defined;
   bool nocache;
 
-/* max length of username/password */
-# ifdef ENABLE_PKCS11
-#   define USER_PASS_LEN 4096
-# else
-#   define USER_PASS_LEN 128
-# endif
   char username[USER_PASS_LEN];
   char password[USER_PASS_LEN];
 };
Signature · 1 line
1.8.3.1
#1 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 11 lines · David Sommerseth

If running an OpenVPN client with --enable-pkcs11 and a server without and having a username and/or password with more than 128 characters, the authentication will fail as the server truncates the password to 128 bytes.

This makes things easier and more predictable. Username/passwords can be up to 4096 bytes, regardless of the --enable-pkcs11 state.

src/openvpn/misc.h | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)

I forgot to add the Trac ticket to the commit message, but this should resolve ticket #712 [1]

[1] <https://community.openvpn.net/openvpn/ticket/712>

- --
kind regards,

David Sommerseth
OpenVPN Technologies, Inc

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJX468PAAoJEIbPlEyWcf3yu6sQAJSOF3f77RCpeFKxJ96C9gJz Ry3msuuzLceiqlISZ1iqRocaDPz6UNN/AEZSoDFA6UXWWnUvDkM4luiU8pjdxe20 dJaO1BBWm7KVVn66/QQApwFOGUzJLMw2KXGAKaKWQl0jEhxjDMpro6KB60cy+439 D7iFtFwSlFekuW5/8INVyVBNQZ925aOolI2i8GFoH4yHxN3fTSHtIwSQUJpk0UuB 2VN7Lkl5UpwrSlU+YyJtS6YoMQsQm3CEKESus3h8hOLNz8WHFg4aJLHu7M7QKXmU ShgdF94vFq2+FvZH465gpICIOgDHRLm+TgJ6rNv2iT2G8KotczpuPXz5XxreS4jf RorDfhnYSw+ZR20cfiFY9et1UgWD3CRHTB+xQh9OXj4qjuxiGrtYG4Y1V1ReYkrw K0TOOtvkUyiEtqrayKQfAQOLZhVy+IW0qw+FMky8GJYqdpwAzFDMqz3ysI04B4CQ hf4i9aYIBuEzu82aacyu890zYjFY6wSoxSjS8MyeeeC2GgNz2JvFb2X4W8oratmd 9dvVQa1UVE0okhX+2z9ka4M0+J8GRy9wH2Lagd5cnKIJxUbMDr98H9GErh10uWjU mIBvCxCnJr01wIEIPagOX5DiLEGMy72F66fm5SZs4NMcdFfpKsf/yhIXNDXtVOhB lkXu10lYZSIUCsj0YA12
=kTQ5
-----END PGP SIGNATURE-----

#2 lore raw
In reply to David Sommerseth

Hi,

Quoted 28 lines · David Sommerseth

If running an OpenVPN client with --enable-pkcs11 and a server without and having a username and/or password with more than 128 characters, the authentication will fail as the server truncates the password to 128 bytes.

This makes things easier and more predictable. Username/passwords can be up to 4096 bytes, regardless of the --enable-pkcs11 state.

---
 src/openvpn/misc.h | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
src/openvpn/misc.h +3 -0
index b694096..31ea10e 100644
@@ -195,17 +195,14 @@ const char *hostname_randomize(const char *hostname, struct gc_arena *gc);
  * Get and store a username/password
  */
 
+/* max length of username/password */
+#define USER_PASS_LEN 4096
+
 struct user_pass
 {
   bool defined;
   bool nocache;

src/openvpn/misc.h const char *hostname_randomize(const char *hostname, struct gc_arena *gc); · line 208

Quoted 10 lines
src/openvpn/misc.h +0 -6
 
-/* max length of username/password */
-# ifdef ENABLE_PKCS11
-#   define USER_PASS_LEN 4096
-# else
-#   define USER_PASS_LEN 128
-# endif
   char username[USER_PASS_LEN];
   char password[USER_PASS_LEN];
 };

ACK.

I've wondered about this before, but never got to sending a patch. Good that you did get to it.

How are user/pass formatted. Are these supposed to be zero-terminated? In that case it would make sense to add a remark to the comment that this length includes the zero-byte.

-Steffan

#4 lore raw
In reply to David Sommerseth
Quoted 7 lines · David Sommerseth

If running an OpenVPN client with --enable-pkcs11 and a server without and having a username and/or password with more than 128 characters, the authentication will fail as the server truncates the password to 128 bytes.

This makes things easier and more predictable. Username/passwords can be up to 4096 bytes, regardless of the --enable-pkcs11 state.

Hi David,

1. Minor quibble: "Characters" is a bit misleading because (I think) it is actually the number of bytes that is limited -- a UTF-8 string of 256 bytes may represent fewer than 256 characters.

2. The management interface limits usernames, passwords, and private keys to 255 or 256 bytes. The following error is sent by over the management interface in response to a 275 byte password ("a-y" repeated 11 times):

ERROR: Options error: Parameter at TCP:0 is too long (256 chars max):
abcdefghijklmnopqrstuvwxyabcdefghijklmnopqrstuvwxyabcdefghijklmnopqrstuvwxyabcdefghijklmnopqrstuvwxyabcdefghijklmnopqrstuvwxyabcdefghijklmnopqrstuvwxyabcdefghijklmnopqrstuvwxyabcdefghijklmnopqrstuvwxyabcdefghijklmnopqrstuvwxyabcdefghijklmnopqrstuvwxyabcde

Note that although the error message says 256 characters is the maximum, there are only 255 characters/bytes in the password it sends back. Perhaps an off-by-one error? (And I am pretty sure that "bytes" is what is limited, not "characters", as per above.)

Best regards,

Jon Bullard

#3 lore raw
In reply to Steffan Karger

Hi,

Quoted 19 lines · Steffan Karger
Quoted 6 lines
src/openvpn/misc.h +2 -0
+#define USER_PASS_LEN 4096
+
 struct user_pass
 {
   bool defined;
   bool nocache;

src/openvpn/misc.h const char *hostname_randomize(const char *hostname, struct gc_arena *gc); · line 208

Quoted 10 lines
src/openvpn/misc.h +0 -6
 
-/* max length of username/password */
-# ifdef ENABLE_PKCS11
-#   define USER_PASS_LEN 4096
-# else
-#   define USER_PASS_LEN 128
-# endif
   char username[USER_PASS_LEN];
   char password[USER_PASS_LEN];
 };

ACK.

Quoted 2 lines

I've wondered about this before, but never got to sending a patch. Good that you did get to it.

I'm not exactly happy about this. struct user_pass is used as a global instance in quite a few places, and this sucks up 7.5 kbytes extra for each of them.

As Jonathan wrote, management interface can only handle 256 bytes, so I think this should be done as part of a larger cleanup.

But it's good that you got involved :-) - what is the reasoning behind 4k for username and password here? It doesn't really make sense to me, but since this struct is used in pkcs#11 context, something seems to need it *there*.

(Will it work at all, to send a 4k username and password in the OpenVPN handshake? James tells us the space for --push-peer-info is "limited", and username+password share the very same space...)

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
#5 lore raw
In reply to David Sommerseth

Hi,

Quoted 8 lines · David Sommerseth

If running an OpenVPN client with --enable-pkcs11 and a server without and having a username and/or password with more than 128 characters, the authentication will fail as the server truncates the password to 128 bytes.

This makes things easier and more predictable. Username/passwords can be up to 4096 bytes, regardless of the --enable-pkcs11 state.

In spite of this change, in most usage scenarios, username, password and even pkcs11-id are limited to a much smaller length (more like ~256 bytes) for other reasons:

The pkcs11-id is the only string that probably needs such a long length, but its limited to ~240 bytes when read from the config file because of max line length of 256 bytes + 1 for NUL (in some code paths 255?). Even if the max line length is increased, the max size of an option parameter is 256 bytes (+ 1 for NUL).

The 256 byte limit on parameters in response received from management (pointed out by Jonathan) is also somewhat related to the above: Input from management goes through the same parsing code as the lines read from the config file for handling escapes, embedded whitespace etc. and a limit of 256 bytes per option parameter is imposed

Also usernames > 64 bytes will break --username-as-common-name as CN is limited to 64 bytes (could 64 code points, but it seems openvpn interprets that as 64 bytes).

So for most practical purposes, pkcs11-id appears to be limited to ~240 bytes characters in the current code. Changing USER_PASS_LEN to, say, 256 may be more sensible. That shouldn't break any existing configs but may break some use cases where a long id is supplied from the command line.

Selva

#6 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 35 lines · Selva Nair

Hi,

On Thu, Sep 22, 2016 at 6:04 AM, David Sommerseth <[email protected]... <mailto:[email protected]...>> wrote:

If running an OpenVPN client with --enable-pkcs11 and a server without and having a username and/or password with more than 128 characters, the authentication will fail as the server truncates the password to 128 bytes.

This makes things easier and more predictable. Username/passwords can be up to 4096 bytes, regardless of the --enable-pkcs11 state.

In spite of this change, in most usage scenarios, username, password and even pkcs11-id are limited to a much smaller length (more like ~256 bytes) for other reasons:

The pkcs11-id is the only string that probably needs such a long length, but its limited to ~240 bytes when read from the config file because of max line length of 256 bytes + 1 for NUL (in some code paths 255?). Even if the max line length is increased, the max size of an option parameter is 256 bytes (+ 1 for NUL).

The 256 byte limit on parameters in response received from management (pointed out by Jonathan) is also somewhat related to the above: Input from management goes through the same parsing code as the lines read from the config file for handling escapes, embedded whitespace etc. and a limit of 256 bytes per option parameter is imposed

Also usernames > 64 bytes will break --username-as-common-name as CN is limited to 64 bytes (could 64 code points, but it seems openvpn interprets that as 64 bytes).

Quoted 5 lines

So for most practical purposes, pkcs11-id appears to be limited to ~240 bytes characters in the current code. Changing USER_PASS_LEN to, say, 256 may be more sensible. That shouldn't break any existing configs but may break some use cases where a long id is supplied from the command line.

Thank you for a thorough check-up!

I have spotted a few places where 'struct userpass' have been declared as global variables, like in socks.c, proxy.c and ssl.c. For simplicity this can be okay, code wise. But considering these buffers is not be used in all configurations (probably not even the majority of configurations), this seems like wasted memory. I also do not have complete overview of the management code yet, as struct user_pass is used inside some other management related structs, where some might also be global.

I have no issues to set USER_PASS_LEN to 256 or 1024 or in between as long as we are confident we don't break anything. But I think we could clean up several of these global variables too.

- --
kind regards,

David Sommerseth
OpenVPN Technologies, Inc

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJX5BhuAAoJEIbPlEyWcf3yO8QP/2TJzxlf1nE9XuLZmWuZYLnu 76wUBd+MsPnSGu0Jv8bAqBA2O7kLPH+G6n265x11FLerJk7cbAJqq8j/PXoHP51U lUSPVuWv2TKdizPjy3zoCxP30YKgvs7mWg4JCyyWvcrC2L/MDLRABFALa7PvhyOQ OtjIi+9PW5gP3BQ207b1bIXyVCFUgPzlU4V+tE09OXFI2vT7/k+gAkFai0jxI4fb +6pEhIui3S20CdZlsIUj4jrHWk/bwLPH8JZ7JP+EKxu3XNDN28exP84ngOKjnntt A/SaKLp2pbtQGeyB1h2Pv07u7vmVwp6bXmxYUhuc6E3POEh1mg+uL2e3Dd3f+2oW BcZkMN75LpCTQYk3PqwV6UpjJ6jLurTMzAEnQ0Rf6EUFf6Vzeti2QyAoiTynGjGh zer7lYY918K3+kiFUJ/fvdMhFbnIFcBc3579Hpgahp1V5KhKnGN5t1fnWKAy+7sm se4pq4Be+SPV/EI6nFvab6dik9MJorSqmWRrfZeTyIV0+3BKtZc8g415litglh6x gT/Atf4G4BLxe4I3G66flSZBG7rLGi8DZnnsPz2SYcPMQyEz9PbkRA+Dumi7QFRg i1ghdI6bkT8Fnw3wx4/hHcIr1Tu76xxl6PGKqCp8u3SeXhXv9kzWX29q65Ie1hLB VNbRKmRCJFMEnV/9g7uG
=Cjnn
-----END PGP SIGNATURE-----

#7 lore raw

Hi,

Quoted 12 lines · David Sommerseth
Quoted 10 lines · Selva Nair

Also usernames > 64 bytes will break --username-as-common-name as CN is limited to 64 bytes (could 64 code points, but it seems openvpn interprets that as 64 bytes).

So for most practical purposes, pkcs11-id appears to be limited to ~240 bytes characters in the current code. Changing USER_PASS_LEN to, say, 256 may be more sensible. That shouldn't break any existing configs but may break some use cases where a long id is supplied from the command line.

Quoted 5 lines

Thank you for a thorough check-up!

I have spotted a few places where 'struct userpass' have been declared as global variables, like in socks.c, proxy.c and ssl.c. For

One could replace those static structs with static pointers to the struct and allocate at first use.

I have no issues to set USER_PASS_LEN to 256 or 1024 or in between as

Quoted 2 lines

long as we are confident we don't break anything. But I think we could clean up several of these global variables too.

Code-wise, I do not think this will break anything. But any number less than 4096 could potentially break some rare (?) use cases -- e.g., passwords/usernames/ids of length > 256 supplied from a file or command line on clients (and matching server) with USER_PASS_LEN = 4096. At the same time, anything more than 256 should also increase length of option lines and option parameters.

I vote for 256.

That said I have no idea how long are typical pkcs11-ids. And, if pkcs11 URI is adopted, the string could get arbitrarily large depending on how many attributes are included in the URI. In my view increasing the size to 4096 to support pkcs11-ids was the wrong approach in the first place. Anything longer than OPTION_LINE_SIZE is better input as multiple lines and parsed without using get_user_pass.

Selva