Thanks Willy and Tim for your feedback.

You can find attached the updated patches with fixed coding style (now
set correctly in my editor), updated commit message, entry doc in sorted
order, size_t instead of int in both enc/dec  and corresponding reg-test.

Only part unclear:
On 02/04/2021 15:04, Tim Düsterhus wrote:
>> +int base64urldec(const char *in, size_t ilen, char *out, size_t olen) {
>> +    char conv[ilen+2];
>
> This looks like a remotely triggerable stack overflow.

You mean in case ilen is too big? in such case should we rather use
dynamic allocation ?

-- 
Moemen MHEDHBI
>From bae8d3890be6d2f5a58697bf7b8e9f01f4589d3b Mon Sep 17 00:00:00 2001
From: Moemen MHEDHBI <mmhed...@haproxy.com>
Date: Thu, 1 Apr 2021 20:53:59 +0200
Subject: [PATCH 1/2] MINOR: sample: add ub64dec and ub64enc converters

ub64dec and ub64enc are the base64url equivalent of b64dec and base64
converters. base64url encoding is the "URL and Filename Safe Alphabet"
variant of base64 encoding. It is also used in in JWT (JSON Web Token)
standard.
RFC1421 mention in base64.c file is deprecated so it was replaced with
RFC4648 to which existing converters, base64/b64dec, still apply.

Example:
  HAProxy:
    http-request return content-type text/plain lf-string %[req.hdr(Authorization),word(2,.),ub64dec]
  Client:
    Token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1c2VyIjoiZm9vIiwia2V5IjoiY2hhZTZBaFhhaTZlIn0.5VsVj7mdxVvo1wP5c0dVHnr-S_khnIdFkThqvwukmdg
    $ curl -H "Authorization: Bearer ${TOKEN}" http://haproxy.local
    {"user":"foo","key":"chae6AhXai6e"}
---
 doc/configuration.txt                | 12 ++++++
 include/haproxy/base64.h             |  2 +
 reg-tests/sample_fetches/ubase64.vtc | 24 +++++++++++
 src/base64.c                         | 59 +++++++++++++++++++++++++++-
 src/sample.c                         | 38 ++++++++++++++++++
 5 files changed, 134 insertions(+), 1 deletion(-)
 create mode 100644 reg-tests/sample_fetches/ubase64.vtc

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 7048fb63e..c7fe416e5 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -16393,6 +16393,18 @@ table_trackers(<table>)
   connections there are from a given address for example. See also the
   sc_trackers sample fetch keyword.
 
+ub64dec
+  This converter is the base64url variant of b64dec converter. base64url
+	encoding is the "URL and Filename Safe Alphabet" variant of base64 encoding.
+	It is also the encoding used in JWT (JSON Web Token) standard.
+
+	Example:
+	  # Decoding a JWT payload:
+	  http-request set-var(txn.token_payload) req.hdr(Authorization),word(2,.),ub64dec
+
+ub64enc
+  This converter is the base64url variant of base64 converter.
+
 upper
   Convert a string sample to upper case. This can only be placed after a string
   sample fetch function or after a transformation keyword returning a string
diff --git a/include/haproxy/base64.h b/include/haproxy/base64.h
index 1756bc058..532c46a44 100644
--- a/include/haproxy/base64.h
+++ b/include/haproxy/base64.h
@@ -17,7 +17,9 @@
 #include <haproxy/api.h>
 
 int a2base64(char *in, int ilen, char *out, int olen);
+int a2base64url(char *in, size_t ilen, char *out, size_t olen);
 int base64dec(const char *in, size_t ilen, char *out, size_t olen);
+int base64urldec(const char *in, size_t ilen, char *out, size_t olen);
 const char *s30tob64(int in, char *out);
 int b64tos30(const char *in);
 
diff --git a/reg-tests/sample_fetches/ubase64.vtc b/reg-tests/sample_fetches/ubase64.vtc
new file mode 100644
index 000000000..a273321d2
--- /dev/null
+++ b/reg-tests/sample_fetches/ubase64.vtc
@@ -0,0 +1,24 @@
+varnishtest "ub64dec sample fetche Test"
+
+#REQUIRE_VERSION=2.4
+
+feature ignore_unknown_macro
+
+haproxy h1 -conf {
+    defaults
+        mode http
+        timeout connect 1s
+        timeout client  1s
+        timeout server  1s
+
+    frontend fe
+        bind "fd@${fe}"
+        http-request return content-type text/plain hdr encode %[hdr(input),ub64enc] lf-string %[req.hdr(Authorization),word(2,.),ub64dec]
+} -start
+
+client c1 -connect ${h1_fe_sock} {
+    txreq -url "/" -hdr "input: biduule" -hdr "Authorization: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1c2VyIjoiZm9vIiwia2V5IjoiY2hhZTZBaFhhaTZlIn0.5VsVj7mdxVvo1wP5c0dVHnr-S_khnIdFkThqvwukmdg"
+    rxresp
+    expect resp.http.encode == "YmlkdXVsZQ"
+    expect resp.body == "{\"user\":\"foo\",\"key\":\"chae6AhXai6e\"}"
+} -run
diff --git a/src/base64.c b/src/base64.c
index 53e4d65b2..c53c8b076 100644
--- a/src/base64.c
+++ b/src/base64.c
@@ -1,5 +1,5 @@
 /*
- * ASCII <-> Base64 conversion as described in RFC1421.
+ * ASCII <-> Base64 conversion as described in RFC4648.
  *
  * Copyright 2006-2010 Willy Tarreau <w...@1wt.eu>
  * Copyright 2009-2010 Krzysztof Piotr Oledzki <o...@ans.pl>
@@ -138,6 +138,63 @@ int base64dec(const char *in, size_t ilen, char *out, size_t olen) {
 	return convlen;
 }
 
+/* url variant of a2base64 */
+int a2base64url(char *in, size_t ilen, char *out, size_t olen)
+{
+	int convlen, i;
+
+	convlen = a2base64(in, ilen, out, olen);
+	while (out[convlen - 1] == '=') {
+		convlen--;
+		out[convlen] = '\0';
+	}
+	for (i = 0; i < convlen; i++) {
+		if (out[i] == '+')
+			out[i] = '-';
+		if (out[i] == '/')
+			out[i] = '_';
+	}
+	return convlen;
+}
+
+/* url variant of base64dec */
+int base64urldec(const char *in, size_t ilen, char *out, size_t olen)
+{
+	int i;
+	char conv[ilen + 2];
+
+	for (i = 0; i < ilen; i++) {
+		switch (in[i]) {
+			case '-':
+				conv[i] = '+';
+				break;
+			case '_':
+				conv[i] = '/';
+				break;
+			default:
+				conv[i] = in[i];
+		}
+	}
+	switch (ilen % 4) {
+		case 0:
+			break;
+		case 2:
+			conv[ilen] = '=';
+			conv[ilen + 1] = '=';
+			conv[ilen + 2] = '\0';
+			ilen += 2;
+			break;
+		case 3:
+			conv[ilen] = '=';
+			conv[ilen + 1] = '\0';
+			ilen += 1;
+			break;
+		default:
+			return -1;
+	}
+	return base64dec(conv, ilen, out, olen);
+}
+
 
 /* Converts the lower 30 bits of an integer to a 5-char base64 string. The
  * caller is responsible for ensuring that the output buffer can accept 6 bytes
diff --git a/src/sample.c b/src/sample.c
index 835a18115..e36e2e2ec 100644
--- a/src/sample.c
+++ b/src/sample.c
@@ -1567,6 +1567,24 @@ static int sample_conv_base642bin(const struct arg *arg_p, struct sample *smp, v
 	return 1;
 }
 
+static int sample_conv_base64url2bin(const struct arg *arg_p, struct sample *smp, void *private)
+{
+	struct buffer *trash = get_trash_chunk();
+	int bin_len;
+
+	trash->data = 0;
+	bin_len = base64urldec(smp->data.u.str.area, smp->data.u.str.data,
+			    trash->area, trash->size);
+	if (bin_len < 0)
+		return 0;
+
+	trash->data = bin_len;
+	smp->data.u.str = *trash;
+	smp->data.type = SMP_T_BIN;
+	smp->flags &= ~SMP_F_CONST;
+	return 1;
+}
+
 static int sample_conv_bin2base64(const struct arg *arg_p, struct sample *smp, void *private)
 {
 	struct buffer *trash = get_trash_chunk();
@@ -1585,6 +1603,24 @@ static int sample_conv_bin2base64(const struct arg *arg_p, struct sample *smp, v
 	return 1;
 }
 
+static int sample_conv_bin2base64url(const struct arg *arg_p, struct sample *smp, void *private)
+{
+	struct buffer *trash = get_trash_chunk();
+	int b64_len;
+
+	trash->data = 0;
+	b64_len = a2base64url(smp->data.u.str.area, smp->data.u.str.data,
+			   trash->area, trash->size);
+	if (b64_len < 0)
+		return 0;
+
+	trash->data = b64_len;
+	smp->data.u.str = *trash;
+	smp->data.type = SMP_T_STR;
+	smp->flags &= ~SMP_F_CONST;
+	return 1;
+}
+
 static int sample_conv_sha1(const struct arg *arg_p, struct sample *smp, void *private)
 {
 	blk_SHA_CTX ctx;
@@ -4096,6 +4132,8 @@ static struct sample_conv_kw_list sample_conv_kws = {ILH, {
 	{ "debug",  sample_conv_debug,     ARG2(0,STR,STR), smp_check_debug, SMP_T_ANY,  SMP_T_ANY },
 	{ "b64dec", sample_conv_base642bin,0,            NULL, SMP_T_STR,  SMP_T_BIN  },
 	{ "base64", sample_conv_bin2base64,0,            NULL, SMP_T_BIN,  SMP_T_STR  },
+	{ "ub64dec", sample_conv_base64url2bin,0,        NULL, SMP_T_STR,  SMP_T_BIN  },
+	{ "ub64enc", sample_conv_bin2base64url,0,        NULL, SMP_T_BIN,  SMP_T_STR  },
 	{ "upper",  sample_conv_str2upper, 0,            NULL, SMP_T_STR,  SMP_T_STR  },
 	{ "lower",  sample_conv_str2lower, 0,            NULL, SMP_T_STR,  SMP_T_STR  },
 	{ "length", sample_conv_length,    0,            NULL, SMP_T_STR,  SMP_T_SINT },
-- 
2.31.1

>From 8b6941f1529038c184754a23bcd199d7746f8715 Mon Sep 17 00:00:00 2001
From: Moemen MHEDHBI <mmhed...@haproxy.com>
Date: Fri, 2 Apr 2021 01:05:07 +0200
Subject: [PATCH 2/2] CLEANUP: align samples list in sample.c

---
 src/sample.c | 54 ++++++++++++++++++++++++++--------------------------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/src/sample.c b/src/sample.c
index e36e2e2ec..9dd05f648 100644
--- a/src/sample.c
+++ b/src/sample.c
@@ -4129,33 +4129,33 @@ INITCALL1(STG_REGISTER, sample_register_fetches, &smp_kws);
 
 /* Note: must not be declared <const> as its list will be overwritten */
 static struct sample_conv_kw_list sample_conv_kws = {ILH, {
-	{ "debug",  sample_conv_debug,     ARG2(0,STR,STR), smp_check_debug, SMP_T_ANY,  SMP_T_ANY },
-	{ "b64dec", sample_conv_base642bin,0,            NULL, SMP_T_STR,  SMP_T_BIN  },
-	{ "base64", sample_conv_bin2base64,0,            NULL, SMP_T_BIN,  SMP_T_STR  },
-	{ "ub64dec", sample_conv_base64url2bin,0,        NULL, SMP_T_STR,  SMP_T_BIN  },
-	{ "ub64enc", sample_conv_bin2base64url,0,        NULL, SMP_T_BIN,  SMP_T_STR  },
-	{ "upper",  sample_conv_str2upper, 0,            NULL, SMP_T_STR,  SMP_T_STR  },
-	{ "lower",  sample_conv_str2lower, 0,            NULL, SMP_T_STR,  SMP_T_STR  },
-	{ "length", sample_conv_length,    0,            NULL, SMP_T_STR,  SMP_T_SINT },
-	{ "hex",    sample_conv_bin2hex,   0,            NULL, SMP_T_BIN,  SMP_T_STR  },
-	{ "hex2i",  sample_conv_hex2int,   0,            NULL, SMP_T_STR,  SMP_T_SINT },
-	{ "ipmask", sample_conv_ipmask,    ARG2(1,MSK4,MSK6), NULL, SMP_T_ADDR, SMP_T_IPV4 },
-	{ "ltime",  sample_conv_ltime,     ARG2(1,STR,SINT), NULL, SMP_T_SINT, SMP_T_STR },
-	{ "utime",  sample_conv_utime,     ARG2(1,STR,SINT), NULL, SMP_T_SINT, SMP_T_STR },
-	{ "crc32",  sample_conv_crc32,     ARG1(0,SINT), NULL, SMP_T_BIN,  SMP_T_SINT  },
-	{ "crc32c", sample_conv_crc32c,    ARG1(0,SINT), NULL, SMP_T_BIN,  SMP_T_SINT  },
-	{ "djb2",   sample_conv_djb2,      ARG1(0,SINT), NULL, SMP_T_BIN,  SMP_T_SINT  },
-	{ "sdbm",   sample_conv_sdbm,      ARG1(0,SINT), NULL, SMP_T_BIN,  SMP_T_SINT  },
-	{ "wt6",    sample_conv_wt6,       ARG1(0,SINT), NULL, SMP_T_BIN,  SMP_T_SINT  },
-	{ "xxh3",   sample_conv_xxh3,      ARG1(0,SINT), NULL, SMP_T_BIN,  SMP_T_SINT  },
-	{ "xxh32",  sample_conv_xxh32,     ARG1(0,SINT), NULL, SMP_T_BIN,  SMP_T_SINT  },
-	{ "xxh64",  sample_conv_xxh64,     ARG1(0,SINT), NULL, SMP_T_BIN,  SMP_T_SINT  },
-	{ "json",   sample_conv_json,      ARG1(1,STR),  sample_conv_json_check, SMP_T_STR,  SMP_T_STR },
-	{ "bytes",  sample_conv_bytes,     ARG2(1,SINT,SINT), NULL, SMP_T_BIN,  SMP_T_BIN },
-	{ "field",  sample_conv_field,     ARG3(2,SINT,STR,SINT), sample_conv_field_check, SMP_T_STR,  SMP_T_STR },
-	{ "word",   sample_conv_word,      ARG3(2,SINT,STR,SINT), sample_conv_field_check, SMP_T_STR,  SMP_T_STR },
-	{ "regsub", sample_conv_regsub,    ARG3(2,REG,STR,STR), sample_conv_regsub_check, SMP_T_STR, SMP_T_STR },
-	{ "sha1",   sample_conv_sha1,      0,            NULL, SMP_T_BIN,  SMP_T_BIN  },
+	{ "debug",   sample_conv_debug,        ARG2(0,STR,STR),       smp_check_debug,          SMP_T_ANY,  SMP_T_ANY  },
+	{ "b64dec",  sample_conv_base642bin,   0,                     NULL,                     SMP_T_STR,  SMP_T_BIN  },
+	{ "base64",  sample_conv_bin2base64,   0,                     NULL,                     SMP_T_BIN,  SMP_T_STR  },
+	{ "ub64enc", sample_conv_bin2base64url,0,                     NULL,                     SMP_T_BIN,  SMP_T_STR  },
+	{ "ub64dec", sample_conv_base64url2bin,0,                     NULL,                     SMP_T_STR,  SMP_T_BIN  },
+	{ "upper",   sample_conv_str2upper,    0,                     NULL,                     SMP_T_STR,  SMP_T_STR  },
+	{ "lower",   sample_conv_str2lower,    0,                     NULL,                     SMP_T_STR,  SMP_T_STR  },
+	{ "length",  sample_conv_length,       0,                     NULL,                     SMP_T_STR,  SMP_T_SINT },
+	{ "hex",     sample_conv_bin2hex,      0,                     NULL,                     SMP_T_BIN,  SMP_T_STR  },
+	{ "hex2i",   sample_conv_hex2int,      0,                     NULL,                     SMP_T_STR,  SMP_T_SINT },
+	{ "ipmask",  sample_conv_ipmask,       ARG2(1,MSK4,MSK6),     NULL,                     SMP_T_ADDR, SMP_T_IPV4 },
+	{ "ltime",   sample_conv_ltime,        ARG2(1,STR,SINT),      NULL,                     SMP_T_SINT, SMP_T_STR  },
+	{ "utime",   sample_conv_utime,        ARG2(1,STR,SINT),      NULL,                     SMP_T_SINT, SMP_T_STR  },
+	{ "crc32",   sample_conv_crc32,        ARG1(0,SINT),          NULL,                     SMP_T_BIN,  SMP_T_SINT },
+	{ "crc32c",  sample_conv_crc32c,       ARG1(0,SINT),          NULL,                     SMP_T_BIN,  SMP_T_SINT },
+	{ "djb2",    sample_conv_djb2,         ARG1(0,SINT),          NULL,                     SMP_T_BIN,  SMP_T_SINT },
+	{ "sdbm",    sample_conv_sdbm,         ARG1(0,SINT),          NULL,                     SMP_T_BIN,  SMP_T_SINT },
+	{ "wt6",     sample_conv_wt6,          ARG1(0,SINT),          NULL,                     SMP_T_BIN,  SMP_T_SINT },
+	{ "xxh3",    sample_conv_xxh3,         ARG1(0,SINT),          NULL,                     SMP_T_BIN,  SMP_T_SINT },
+	{ "xxh32",   sample_conv_xxh32,        ARG1(0,SINT),          NULL,                     SMP_T_BIN,  SMP_T_SINT },
+	{ "xxh64",   sample_conv_xxh64,        ARG1(0,SINT),          NULL,                     SMP_T_BIN,  SMP_T_SINT },
+	{ "json",    sample_conv_json,         ARG1(1,STR),           sample_conv_json_check,   SMP_T_STR,  SMP_T_STR  },
+	{ "bytes",   sample_conv_bytes,        ARG2(1,SINT,SINT),     NULL,                     SMP_T_BIN,  SMP_T_BIN  },
+	{ "field",   sample_conv_field,        ARG3(2,SINT,STR,SINT), sample_conv_field_check,  SMP_T_STR,  SMP_T_STR  },
+	{ "word",    sample_conv_word,         ARG3(2,SINT,STR,SINT), sample_conv_field_check,  SMP_T_STR,  SMP_T_STR  },
+	{ "regsub",  sample_conv_regsub,       ARG3(2,REG,STR,STR),   sample_conv_regsub_check, SMP_T_STR,  SMP_T_STR  },
+	{ "sha1",    sample_conv_sha1,         0,                     NULL,                     SMP_T_BIN,  SMP_T_BIN  },
 #ifdef USE_OPENSSL
 	{ "sha2",   sample_conv_sha2,      ARG1(0, SINT), smp_check_sha2, SMP_T_BIN,  SMP_T_BIN  },
 #ifdef EVP_CIPH_GCM_MODE
-- 
2.31.1

Reply via email to