BUG/MINOR: jwt: enforce the ECDSA signature length in jwt_verify

RFC 7518 section 3.4 states that an ES256 signature MUST be a 64-octet
sequence made of R and S each on 32 bytes, and that validation fails
otherwise (96 bytes for ES384 and 132 for ES512). jwt_verify() and
jwt_verify_cert() accept any even length and split it in two halves, so
that R and S may be prefixed with any number of zero bytes.
This can be reproduced by taking a valid ES256 token, decoding its
signature, inserting a zero byte before each 32-byte half, and encoding it
again: the converter still returns 1.

Let's check that the signature has exactly the length mandated by the
algorithm in convert_ecdsa_sig(), and return JWT_VRFY_INVALID_TOKEN (-3)
otherwise, as was already done for odd lengths. A reg-test is provided.

This fix should be backported to all supported versions.

diff --git a/reg-tests/jwt/jwt_verify_ecdsa_sig_len.vtc b/reg-tests/jwt/jwt_verify_ecdsa_sig_len.vtc
new file mode 100644 (file)
index 0000000..fbd25b1
--- /dev/null
+++ b/reg-tests/jwt/jwt_verify_ecdsa_sig_len.vtc
@@ -0,0 +1,44 @@
+#REGTEST_TYPE=devel
+
+# This reg-test checks that ECDSA signatures are only accepted by jwt_verify
+# and jwt_verify_cert when they are made of R and S each having exactly the
+# size of the curve's order (RFC 7518 section 3.4). The second token carries
+# the same signature as the first one, except that R and S were each prefixed
+# with a zero byte.
+
+varnishtest "jwt_verify_cert must enforce the ECDSA signature length"
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(3.3-dev10)'"
+feature cmd "$HAPROXY_PROGRAM -cc 'feature(OPENSSL)'"
+feature ignore_unknown_macro
+
+haproxy h1 -conf {
+    global
+        default-path origin ${testdir}
+
+    crt-store
+        load crt ec_decrypt.crt key ec_decrypt.key jwt on
+
+    defaults
+        mode http
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    frontend fe
+        bind "fd@${fe}"
+        http-request return status 200 hdr x-res "%[req.hdr(x-tok),jwt_verify_cert(ES256,ec_decrypt.crt)]"
+} -start
+
+client c1 -connect ${h1_fe_sock} {
+    # 64-byte R||S signature
+    txreq -hdr "x-tok: eyJhbGciOiAiRVMyNTYifQ.eA.kTlGlMaCikEC5eAybI3xcORLlwVSkJQBy0ZUYywMsTOGArNCX3pdP2FyYFrhXfmUs1D1Dt6q38F8wkPVtr10QA"
+    rxresp
+    expect resp.status == 200
+    expect resp.http.x-res == "1"
+
+    # same signature with R and S prefixed with a zero (66 bytes)
+    txreq -hdr "x-tok: eyJhbGciOiAiRVMyNTYifQ.eA.AJDh42BizW1dIN3dlGtvXe9IH1asyB_FRQiv4ZL3GEJaAAcreGRqSSFFla9Xl83efThZp63HbyX6gLKMg7tbxV7Z"
+    rxresp
+    expect resp.status == 200
+    expect resp.http.x-res == "-3"
+} -run

diff --git a/src/jwt.c b/src/jwt.c
index 68cfc6967135ed9639d99c6b0474f3789a91b4cf..3848ba52f42f3a88b0e609f2672643283725f39a 100644 (file)
--- a/src/jwt.c
+++ b/src/jwt.c
@@ -315,13 +315,21 @@ static int convert_ecdsa_sig(const struct jwt_ctx *ctx, struct buffer *signature
                goto end;
        }
 
-       if (b_data(signature) % 2) {
+       /* The signature must be made of R and S, each of them being exactly
+        * the size of the curve's order (see section 3.4 of RFC7518).
+        */
+       switch (ctx->alg) {
+       case JWS_ALG_ES256: bignum_len = 32; break;
+       case JWS_ALG_ES384: bignum_len = 48; break;
+       case JWS_ALG_ES512: bignum_len = 66; break;
+       default:            bignum_len = 0;  break;
+       }
+
+       if (!bignum_len || b_data(signature) != 2 * bignum_len) {
                retval = JWT_VRFY_INVALID_TOKEN;
                goto end;
        }
 
-       bignum_len = b_data(signature) / 2;
-
        ec_R = BN_bin2bn((unsigned char*)b_orig(signature), bignum_len, NULL);
        ec_S = BN_bin2bn((unsigned char *)(b_orig(signature) + bignum_len), bignum_len, NULL);