diff mbox series

[Openvpn-devel] Fix binary and used instead auth-token check instead of logical and

Message ID 20210331155508.19423-1-arne@rfc2549.org
State Changes Requested
Headers
Series [Openvpn-devel] Fix binary and used instead auth-token check instead of logical and |

Commit Message

Arne Schwabe March 31, 2021, 4:55 a.m. UTC 
  AUTH_TOKEN_HMAC_OK is 1, so the first term is always 0/1 and the bool
from the second part is also 0/1, so the & does the same in this instance
as &&.

In this specific case & instead && does not change behaviour but using
&& is the intended semantic behaviour.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/auth_token.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Antonio Quartulli March 31, 2021, 10:19 a.m. UTC | #1 
Hi,

I would reword the commit subject, because the way it is now fails to
highlight that we are talking about a bitwise-and operator (&).

On 31/03/2021 17:55, Arne Schwabe wrote:
> AUTH_TOKEN_HMAC_OK is 1, so the first term is always 0/1 and the bool
> from the second part is also 0/1, so the & does the same in this instance
> as &&.
> 
> In this specific case & instead && does not change behaviour but using
> && is the intended semantic behaviour.
> 
> Signed-off-by: Arne Schwabe <arne@rfc2549.org>
> ---
>  src/openvpn/auth_token.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c
> index cc70c06c3..d571b686e 100644
> --- a/src/openvpn/auth_token.c
> +++ b/src/openvpn/auth_token.c
> @@ -99,7 +99,7 @@ add_session_token_env(struct tls_session *session, struct tls_multi *multi,
>      /* We had a valid session id before */
>      const char *session_id_source;
>      if (multi->auth_token_state_flags & AUTH_TOKEN_HMAC_OK
> -        &!(multi->auth_token_state_flags & AUTH_TOKEN_EXPIRED))
> +        && !(multi->auth_token_state_flags & AUTH_TOKEN_EXPIRED))
>      {
>          session_id_source = up->password;
>      }
> 

Compile-tested and I stared at the change long enough that I can feel
confident it does not change any functional behaviour.

The commit subject did not pass the compile test :) therefore I Will
wait for a new revision before leaving my ACK.

Cheers,
Gert Doering May 3, 2021, 3:54 a.m. UTC | #2 
Acked-by: Gert Doering <gert@greenie.muc.de>

Your patch has been applied to the master branch.

Release/2.5 has this fix already, as part of commit 3aca477a1b58.

Antonio complained about the patch subject:, and asked for a v2 - which
I did not see anywhere, so I rewrote the commit subject and now it's 
out of my work queue :-)

commit 0cbfa10e6aac01831bebe42ab33dc56c4704c1a6
Author: Arne Schwabe
Date:   Wed Mar 31 17:55:08 2021 +0200

     Fix binary and (&) used in auth-token check instead of logical and (&&)

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20210331155508.19423-1-arne@rfc2549.org>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21911.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c
index cc70c06c3..d571b686e 100644
--- a/src/openvpn/auth_token.c
+++ b/src/openvpn/auth_token.c
@@ -99,7 +99,7 @@  add_session_token_env(struct tls_session *session, struct tls_multi *multi,
     /* We had a valid session id before */
     const char *session_id_source;
     if (multi->auth_token_state_flags & AUTH_TOKEN_HMAC_OK
-        &!(multi->auth_token_state_flags & AUTH_TOKEN_EXPIRED))
+        && !(multi->auth_token_state_flags & AUTH_TOKEN_EXPIRED))
     {
         session_id_source = up->password;
     }