BUG/MINOR: jwt: don't truncate binary HMAC secrets at the first NULL byte

When jwt_verify() is used with an HMAC algorithm and its secret is passed
via a variable, the secret is truncated at its first NUL byte. RFC 7518
recommends random HMAC keys at least as long as the hash, which are
commonly distributed base64-encoded and fed to haproxy through something
such as "str(<b64>),b64dec" into a variable. About one random 32-byte key
out of eight contains a NUL byte. With such a key, all legitimately signed
tokens are rejected (traffic impact for the site), and tokens signed with
only the bytes located before the first NUL are accepted instead, which
for a NUL in the first bytes of the key means that a client can forge
valid tokens after a trivial brute force. Configurations passing the
secret as a string constant are not affected since the config parser
cannot produce a NUL byte there.

This can be reproduced with this config:

    http-request set-var(txn.key) str(YQBiY2RlZg==),b64dec   # "a\0bcdef"
    http-request return status 200 hdr x-res "%[req.hdr(x-tok),jwt_verify(HS256,txn.key)]"

A token signed with "a\0bcdef" returns 0 and one signed with "a" returns 1.

This fix should be backported to all supported versions.

diff --git a/reg-tests/jwt/jws_verify.vtc b/reg-tests/jwt/jws_verify.vtc
index 466109aa9eb450df8b4b88158bd9f12a2e53de71..2577b87e725f1122efeb0ac8e2edd4f44bb67faa 100644 (file)
--- a/reg-tests/jwt/jws_verify.vtc
+++ b/reg-tests/jwt/jws_verify.vtc
@@ -50,6 +50,13 @@ haproxy h1 -conf {
         use_backend auth_bearer_be if { path /auth_bearer }
         default_backend dflt_be
 
+    # Frontend used only to check binary HMAC secrets containing '\0' characters
+    frontend bin_hmac_fe
+        bind "fd@${binhmacfe}"
+        http-request set-var(txn.key) str(YQBiY2RlZg==),b64dec
+        http-request set-var(txn.ikey) int(12345)
+        http-request return status 200 hdr x-res "%[req.hdr(x-tok),jwt_verify(HS256,txn.key)]" hdr x-int "%[req.hdr(x-tok),jwt_verify(HS256,txn.ikey)]"
+
     crt-store named_store
         load crt "${testdir}/cert.rsa.pem" jwt on
 
@@ -600,3 +607,23 @@ client c29 -connect ${h1_mainfe_sock} {
     expect resp.http.x-jwt-verify == "-3"
 } -run
 
+# Check binary HMAC secrets
+client c30 -connect ${h1_binhmacfe_sock} {
+    # signed with the full secret "a\0bcdef"
+    txreq -hdr "x-tok: eyJhbGciOiAiSFMyNTYiLCAidHlwIjogIkpXVCJ9.eyJzdWIiOiJ4In0.x8ao0qtdSK7Gp8GOzciqyucdsYIupoT46NutNBuvnsk"
+    rxresp
+    expect resp.status == 200
+    expect resp.http.x-res == "1"
+
+    # signed with the truncated secret "a"
+    txreq -hdr "x-tok: eyJhbGciOiAiSFMyNTYiLCAidHlwIjogIkpXVCJ9.eyJzdWIiOiJ4In0.rtjwa3Fd19oC-izY5CrSva-VlzmxyLsYDVy0gtkWqkQ"
+    rxresp
+    expect resp.status == 200
+    expect resp.http.x-res == "0"
+
+    # signed with "12345", the integer variable's string form
+    txreq -hdr "x-tok: eyJhbGciOiAiSFMyNTYiLCAidHlwIjogIkpXVCJ9.eyJzdWIiOiJ4In0.uigQcqlOQfXIx6y3FZoQiyA_vnvq6jCU0DKT33UXW1Q"
+    rxresp
+    expect resp.status == 200
+    expect resp.http.x-int == "1"
+} -run

diff --git a/src/sample.c b/src/sample.c
index 7f2a93df6f2372391b9251aab089402ebdf4e7ba..efdd8138db470fa3030f3eada6c6271ae025800f 100644 (file)
--- a/src/sample.c
+++ b/src/sample.c
@@ -4972,11 +4972,27 @@ static int sample_conv_jwt_verify(const struct arg *args, struct sample *smp, vo
        if (chunk_printf(alg, "%.*s", (int)b_data(&alg_smp.data.u.str), b_orig(&alg_smp.data.u.str)) <= 0)
                goto end;
 
+       /* The key may be a binary HMAC secret, which must be retrieved and
+        * copied as-is since both a cast to string and printf() would stop at
+        * the first NUL byte. Variables of other types are still converted to
+        * strings. The key may also be a public key path, hence the trailing
+        * zero.
+        */
        smp_set_owner(&key_smp, smp->px, smp->sess, smp->strm, smp->opt);
-       if (!sample_conv_var2smp_str(&args[1], &key_smp))
+       if (args[1].type == ARGT_VAR) {
+               if (!vars_get_by_desc(&args[1].data.var, &key_smp, NULL))
+                       goto end;
+               if (key_smp.data.type != SMP_T_BIN &&
+                   (!sample_casts[key_smp.data.type][SMP_T_STR] ||
+                    !sample_casts[key_smp.data.type][SMP_T_STR](&key_smp)))
+                       goto end;
+       }
+       else if (!sample_conv_var2smp_str(&args[1], &key_smp))
                goto end;
-       if (chunk_printf(key, "%.*s", (int)b_data(&key_smp.data.u.str), b_orig(&key_smp.data.u.str)) <= 0)
+       if (!b_data(&key_smp.data.u.str) || b_data(&key_smp.data.u.str) >= b_size(key) ||
+           !chunk_cpy(key, &key_smp.data.u.str))
                goto end;
+       key->area[key->data] = 0;
 
        ret = jwt_verify(input, alg, key, 0);