auth-token with auth-nocache fix broke --disable-crypto builds

PATCH In review David Sommerseth 4 participants 5 messages Jun 19, 2017

Share & tools

David Sommerseth

author

2017-06-19 13:05 UTC #0 lore raw

PATCH 1/1 auth-token with auth-nocache fix broke --disable-crypto builds

After adding commit 571165360db0392fa83e, it broke builds where the --disable-crypto was used with ./configure. This was due to the delayed_auth_pass_purge() which requires the crypto code paths being called from init.c without the proper #ifdef encapsulation.

Signed-off-by David Sommerseth <[email protected]...>

---
 src/openvpn/init.c | 3 +++
 1 file changed, 3 insertions(+)

src/openvpn/init.c +3 -0

index 0832f1d1..a54307ad 100644
@@ -1382,6 +1382,8 @@ initialization_sequence_completed(struct context *c, const unsigned int flags)
     /* If we delayed UID/GID downgrade or chroot, do it now */
     do_uid_gid_chroot(c, true);
 
+
+#ifdef ENABLE_CRYPTO
     /*
      * In some cases (i.e. when receiving auth-token via
      * push-reply) the auth-nocache option configured on the

@@ -1393,6 +1395,7 @@ initialization_sequence_completed(struct context *c, const unsigned int flags)
     {
         delayed_auth_pass_purge();
     }
+#endif /* ENABLE_CRYPTO */
 
     /* Test if errors */
     if (flags & ISC_ERRORS)

Signature · 1 line

2.11.0

Gert Doering

2017-06-19 13:16 UTC #1 lore raw

HI,

Quoted 24 lines · David Sommerseth

After adding commit 571165360db0392fa83e, it broke builds where the --disable-crypto was used with ./configure. This was due to the delayed_auth_pass_purge() which requires the crypto code paths being called from init.c without the proper #ifdef encapsulation.
Signed-off-by David Sommerseth <[email protected]...>
---
 src/openvpn/init.c | 3 +++
 1 file changed, 3 insertions(+)
src/openvpn/init.c +2 -0
index 0832f1d1..a54307ad 100644
@@ -1382,6 +1382,8 @@ initialization_sequence_completed(struct context *c, const unsigned int flags)
     /* If we delayed UID/GID downgrade or chroot, do it now */
     do_uid_gid_chroot(c, true);
 
+
+#ifdef ENABLE_CRYPTO
     /*
      * In some cases (i.e. when receiving auth-token via
      * push-reply) the auth-nocache option configured on the

src/openvpn/init.c initialization_sequence_completed(struct context *c, const unsigned int flags) · line 1401

Quoted 7 lines

src/openvpn/init.c +1 -0

@@ -1393,6 +1395,7 @@ initialization_sequence_completed(struct context *c, const unsigned int flags)
     {
         delayed_auth_pass_purge();
     }
+#endif /* ENABLE_CRYPTO */
 
     /* Test if errors */
     if (flags & ISC_ERRORS)

ACK.

Somewhat ugly (is there no existing #ifdef ENABLE_CRYPTO block nearby that we could hide this in?) but I need a clean tree tonight, and this will fix things. So, will to go with it.

(ACK-with-space-change for 2.3 :-) - "git cherry-pick -X ignore-all-space" should do nicely)

thanks,

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]...

David Sommerseth

2017-06-19 13:22 UTC #2 lore raw

Quoted 32 lines · Gert Doering

HI,
Quoted 29 lines · David Sommerseth
After adding commit 571165360db0392fa83e, it broke builds where the --disable-crypto was used with ./configure. This was due to the delayed_auth_pass_purge() which requires the crypto code paths being called from init.c without the proper #ifdef encapsulation.
Signed-off-by David Sommerseth <[email protected]...>
---
 src/openvpn/init.c | 3 +++
 1 file changed, 3 insertions(+)
src/openvpn/init.c +3 -0
index 0832f1d1..a54307ad 100644
@@ -1382,6 +1382,8 @@ initialization_sequence_completed(struct context *c, const unsigned int flags)
     /* If we delayed UID/GID downgrade or chroot, do it now */
     do_uid_gid_chroot(c, true);
 
+
+#ifdef ENABLE_CRYPTO
     /*
      * In some cases (i.e. when receiving auth-token via
      * push-reply) the auth-nocache option configured on the
@@ -1393,6 +1395,7 @@ initialization_sequence_completed(struct context *c, const unsigned int flags)
     {
         delayed_auth_pass_purge();
     }
+#endif /* ENABLE_CRYPTO */
 

Quoted 8 lines

src/openvpn/init.c initialization_sequence_completed(struct context *c, const unsigned int flags) · line 1401
Quoted 2 lines
src/openvpn/init.c +0 -0
     /* Test if errors */
     if (flags & ISC_ERRORS)
ACK.
Somewhat ugly (is there no existing #ifdef ENABLE_CRYPTO block nearby that we could hide this in?) but I need a clean tree tonight, and this will fix things. So, will to go with it.

There are no other ENABLE_CRYPTO blocks in initialization_sequence_completed() [init.c:1375]. The alternative could be to put this #ifdef inside the delayed_auth_pass_purge() and make that function be "empty" without ENABLE_CRYPTO. But as init.c is the only place this is called outside the ENABLE_CRYPTO scope, this is where my dice rolled.

Signature · 4 lines

kind regards,

David Sommerseth
OpenVPN Technologies, Inc

David Sommerseth

author

2017-06-19 13:28 UTC #4 lore raw

In reply to David Sommerseth

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Your patch has been applied to the following branches

commit 5bde5b6d1875fd87b116c943084df0d2f6aee6d0  (master)
commit df5efe7e2cd23b2282526f2c41be2063d941dff1  (release/2.4)
commit 888b0dcee0e03a80f202005914b81ccb17a27eab  (release/2.3)
Author: David Sommerseth
Date:   Mon Jun 19 15:05:07 2017 +0200

auth-token with auth-nocache fix broke --disable-crypto builds

Signed-off-by: David Sommerseth
Acked-by: Gert Doering
Message-Id: <[email protected]...>
URL: https://www.mail-archive.com/[email protected]/msg14883.html

- --
kind regards,

David Sommerseth

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

iQIcBAEBCAAGBQJZR9FtAAoJEIbPlEyWcf3yKxcP/1k6mfayyFHT49v/AFduuWqm EYsX+q1d+RQvJi/0PopAcdnD9DHmQiR6f3/slRPWa1YlP1UJKx7SCYSuxJJoQ4aJ 4lSGresernxIl5LAzlzZo3ohRR3VJZhWM7fyeuFCm5kcw7PRsxQoHHz9yfS29F7O Jzpg8nA6ZJ0pL1QX/T1GapGx0oFY3h/H1vxIkhcZnA0d9rkjqPHX4kUiXSLs49Cq iycUIr75+fMrIia3GMhoF/OjGiGLnxo251ig//YjnEcEnM1MLKlfCL6y2pkpj9ic ff6t79uO7vmMP9iCSoI+C4JESBCoyjWtDEmTx4jF/dyy2Dq8wz81wuImrTkk6EGY LkWZ6dPFxPAFD3wxU0vli9GM8TviuECGppyqjiMreLM/0O4wMHfIkS00TNi1+WYF /AxpyzRiNwLmBbVq+UD+DZhf5X02LErtAYIk2Hlfo/e/Rn819NVCpB+w6fBl1Hf7 7Bb3dI7nWlWMRQoqUelpQ7nnC4f6FGU8ALMM1TzgHCaczAqoh31gRpyXuDkO6+k+ nJAouJLOrMB4Zg4QxsYNoYU/tw80dnO5VnGj7Qi8GrBdyKOiJsnD5zpGYJlPyu5o P0ocMCwnac3GtCiui4N34fb/eVd6LkQaf+CjyiK0+THHyGRUrKkTvLJlHZC7fjQm sG10Hp4LW4IQF+ELL0LP
=NWJC
-----END PGP SIGNATURE-----

Antonio Quartulli

2017-06-19 21:47 UTC #3 lore raw

In reply to David Sommerseth

Quoted 9 lines · David Sommerseth

Quoted 4 lines · Gert Doering
Somewhat ugly (is there no existing #ifdef ENABLE_CRYPTO block nearby that we could hide this in?) but I need a clean tree tonight, and this will fix things. So, will to go with it.

Quoted 6 lines

There are no other ENABLE_CRYPTO blocks in initialization_sequence_completed() [init.c:1375]. The alternative could be to put this #ifdef inside the delayed_auth_pass_purge() and make that function be "empty" without ENABLE_CRYPTO. But as init.c is the only place this is called outside the ENABLE_CRYPTO scope, this is where my dice rolled.

Something I have been doing in the kernel code was to put this kind of blocks in the .h file where the function is declared: in case of CONSTANT not-defined the function would be defined inline within the .h file with an empty body. The assumption is that the corresponding .c file is not compiled in this case. This is usually quite clean and avoids having more and more ifdefs inside the code. For example in this case it would result in:

#ifdef ENABLE_CRYPTO
void delayed_auth_pass_purge(void);
#else
void delayed_auth_pass_purge(void)
{
}
#endif

Obviously you’ll want to group multiple functions in the same ifdef.

This might be helpful to beautify (->make easy to read) several parts of the openvpn code.

my 2 cents.

Cheers,

--
Antonio Quartulli