Re: [Openvpn-devel] [PATCH v6 4/7] Rewrite auth-token-gen to be based on HMAC based tokens
Hi, On Fri, Aug 16, 2019 at 07:13:14PM +0200, David Sommerseth wrote: > On 08/08/2019 16:51, Arne Schwabe wrote: > > The previous auth-token implementation had a serious problem, especially > > when > > paired with an unpatched OpenVPN client that keeps trying the auth-token > > (commit e61b401a). [..] > Since I've tested and reviewed the rest in earlier rounds and the change from > previous version i sjust changing %lld to PRIu64, I'm giving > this my ... > > Acked-By: David Sommerseth While I'm all happy to see this ACK (and I trust David to have done a full review on the code changes), there is a slight problem with this - it breaks "--disable-server" builds: cc -DHAVE_CONFIG_H -I. -I../../../openvpn/src/openvpn -I../.. -I../../include -I../../../openvpn/include -I../../../openvpn/src/compat -I/usr/local/include -I/usr/local/include -DPLUGIN_LIBDIR=\"/usr/local/lib/openvpn/plugins\" -Wall -Wno-unused-parameter -Wno-unused-function -g -O2 -std=c99 -MT auth_token.o -MD -MP -MF .deps/auth_token.Tpo -c -o auth_token.o ../../../openvpn/src/openvpn/auth_token.c ../../../openvpn/src/openvpn/auth_token.c:100:16: error: no member named 'auth_token' in 'struct tls_multi' if (multi->auth_token) ~ ^ ../../../openvpn/src/openvpn/auth_token.c:112:42: error: no member named 'auth_token' in 'struct tls_multi' char *old_tsamp_initial = multi->auth_token + strlen(SESSION_ID_PREFIX); ~ ^ ../../../openvpn/src/openvpn/auth_token.c:119:21: error: no member named 'auth_token' in 'struct tls_multi' free(multi->auth_token); ~ ^ ... so, can I have a v7 of this, plus an ACK...? Sorry for being a spoilsport here. gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v6 4/7] Rewrite auth-token-gen to be based on HMAC based tokens
On 08/08/2019 16:51, Arne Schwabe wrote: > The previous auth-token implementation had a serious problem, especially when > paired with an unpatched OpenVPN client that keeps trying the auth-token > (commit e61b401a). > > The auth-token-gen implementation forgot the auth-token on reconnect, this > lead to reconnect with auth-token never working. > > This new implementation implements the auth-token in a stateles variant. By > using HMAC to sign the auth-token the server can verify if a token has been > authenticated and by checking the embedded timestamp in the token it can > also verify that the auth-token is still valid. > > Using the new config directive auth-gen-token-secret instead of > extending auth-gen-token (--auth-gen-token [lifetime] [secret-key]) was > chosen to allow inlinening the secret key. > > Patch V2: cleaned up code, use refactored read_pem_key_file function > Patch V3: clarify some design decision in the commit message > Patch V4: Use ephermal_generate_key > Patch V5: Use C99 PRIu64 instead of %lld int printf like statement, > fix strict aliasing > --- > doc/openvpn.8| 25 > src/openvpn/Makefile.am | 1 + > src/openvpn/auth_token.c | 273 +++ > src/openvpn/auth_token.h | 116 + > src/openvpn/init.c | 30 - > src/openvpn/openvpn.h| 1 + > src/openvpn/options.c| 22 +++- > src/openvpn/options.h| 4 + > src/openvpn/push.c | 70 -- > src/openvpn/push.h | 8 ++ > src/openvpn/ssl.c| 7 +- > src/openvpn/ssl_common.h | 36 -- > src/openvpn/ssl_verify.c | 182 +++--- > 13 files changed, 640 insertions(+), 135 deletions(-) > create mode 100644 src/openvpn/auth_token.c > create mode 100644 src/openvpn/auth_token.h > Hi, Thanks a lot. This now only leaves the following warning when using gcc-4.8.5 and gcc-6.3.1 (both on RHEL 7.7) auth_token.c: In function ‘generate_auth_token’: auth_token.c:115:9: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] initial_timestamp = *((uint64_t *)(old_tstamp_decode)); ^ This warning is not present when compiling with gcc-7.3.1, gcc-8.3.1, clang-3.4.2 nor clang-5.0.1. So I'm blaming buggy/confused older GCC compilers for this one. Since I've tested and reviewed the rest in earlier rounds and the change from previous version i sjust changing %lld to PRIu64, I'm giving this my ... Acked-By: David Sommerseth -- kind regards, David Sommerseth OpenVPN Inc ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
