From: Stephan Bosch Date: Sun, 7 Sep 2025 15:11:51 +0000 (+0200) Subject: lib-sasl: sasl-server-mech-oauth2 - Properly parse kvpairs X-Git-Tag: 2.4.2~162 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=08f273ac2dc8b8c3733b6dd550508b2fba0fd897;p=thirdparty%2Fdovecot%2Fcore.git lib-sasl: sasl-server-mech-oauth2 - Properly parse kvpairs --- diff --git a/src/lib-sasl/Makefile.am b/src/lib-sasl/Makefile.am index 86f9e6e521..9336af922f 100644 --- a/src/lib-sasl/Makefile.am +++ b/src/lib-sasl/Makefile.am @@ -35,6 +35,7 @@ server_mechanisms = \ sasl-server-mech-winbind.c libsasl_la_SOURCES = \ + sasl-oauth2.c \ $(client_mechanisms) \ dsasl-client.c \ $(server_mechanisms) \ @@ -56,6 +57,7 @@ endif headers = \ sasl-common.h \ + sasl-oauth2.h \ dsasl-client.h \ dsasl-client-private.h \ sasl-server.h \ @@ -69,6 +71,7 @@ pkginc_libdir=$(pkgincludedir) pkginc_lib_HEADERS = $(headers) test_programs = \ + test-sasl-oauth2 \ test-sasl-client noinst_PROGRAMS = $(test_programs) @@ -84,6 +87,10 @@ test_libs = \ test_deps = $(test_libs) +test_sasl_oauth2_SOURCES = test-sasl-oauth2.c +test_sasl_oauth2_LDADD = $(test_libs) +test_sasl_oauth2_DEPENDENCIES = $(test_deps) + test_sasl_client_SOURCES = test-sasl-client.c test_sasl_client_LDADD = $(test_libs) test_sasl_client_DEPENDENCIES = $(test_deps) diff --git a/src/lib-sasl/sasl-oauth2.c b/src/lib-sasl/sasl-oauth2.c new file mode 100644 index 0000000000..cfb4ce9915 --- /dev/null +++ b/src/lib-sasl/sasl-oauth2.c @@ -0,0 +1,87 @@ +/* Copyright (c) 2025 Dovecot authors, see the included COPYING file */ + +#include "lib.h" + +#include "sasl-oauth2.h" + +static const unsigned char key_mask = BIT(0); +static const unsigned char value_mask = BIT(1); + +static const unsigned char char_lookup[256] = { + 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 2, 0, 0, 2, 0, 0, // 00 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 20 + 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, // 20 + 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, // 30 + 2, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, // 40 + 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 2, 2, 2, 2, 2, // 50 + 2, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, // 60 + 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 2, 2, 2, 2, 0, // 70 + + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 80 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 90 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // a0 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // b0 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // c0 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // d0 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // e0 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // f0 +}; + + +int sasl_oauth2_kvpair_parse(const unsigned char *data, size_t size, + const char **key_r, const char **value_r, + const unsigned char **end_r, + const char **error_r) +{ + const unsigned char *p = data, *pend = data + size, *poffset; + + i_assert(p < pend); + + /* RFC 7628, Section 3.1: + + kvsep = %x01 + key = 1*(ALPHA) + value = *(VCHAR / SP / HTAB / CR / LF ) + kvpair = key "=" value kvsep + */ + + /* key = 1*(ALPHA) */ + poffset = p; + while (p < pend && (char_lookup[*p] & key_mask) != 0x00) + p++; + + /* "=" */ + if (p == pend) { + *error_r = "Missing value"; + return -1; + } + if (*p != '=') { + *error_r = "Invalid character in key"; + return -1; + } + if (p == poffset) { + *error_r = "Empty key name"; + return -1; + } + *key_r = t_strdup_until(poffset, p); + p++; + + /* value = *(VCHAR / SP / HTAB / CR / LF ) */ + poffset = p; + while (p < pend && (char_lookup[*p] & value_mask) != 0x00) + p++; + + if (p == pend) { + *error_r = "Missing separator (0x01)"; + return -1; + } + if (*p != 0x01) { + *error_r = "Invalid character in value"; + return -1; + } + *value_r = t_strdup_until(poffset, p); + p++; + + *end_r = p; + return 0; +} diff --git a/src/lib-sasl/sasl-oauth2.h b/src/lib-sasl/sasl-oauth2.h new file mode 100644 index 0000000000..a9640b8252 --- /dev/null +++ b/src/lib-sasl/sasl-oauth2.h @@ -0,0 +1,9 @@ +#ifndef SASL_OAUTH2_H +#define SASL_OAUTH2_H + +int sasl_oauth2_kvpair_parse(const unsigned char *data, size_t size, + const char **key_r, const char **value_r, + const unsigned char **end_r, + const char **error_r); + +#endif diff --git a/src/lib-sasl/sasl-server-mech-oauth2.c b/src/lib-sasl/sasl-server-mech-oauth2.c index 6981867f23..8d67094fd1 100644 --- a/src/lib-sasl/sasl-server-mech-oauth2.c +++ b/src/lib-sasl/sasl-server-mech-oauth2.c @@ -7,6 +7,7 @@ #include "auth-gs2.h" #include "oauth2.h" +#include "sasl-oauth2.h" #include "sasl-server-protected.h" #include "sasl-server-oauth2.h" @@ -213,6 +214,11 @@ mech_oauthbearer_auth_continue(struct sasl_server_mech_request *request, struct oauth2_auth_request *oauth2_req = container_of(request, struct oauth2_auth_request, request); + /* RFC 7628, Section 3.1: + + client-resp = (gs2-header kvsep *kvpair kvsep) / kvsep + */ + if (data_size == 0) { oauth2_fail_invalid_request(oauth2_req); return; @@ -222,6 +228,7 @@ mech_oauthbearer_auth_continue(struct sasl_server_mech_request *request, const unsigned char *gs2_header_end; const char *error; + /* gs2-header */ if (auth_gs2_header_decode(data, data_size, FALSE, &gs2_header, &gs2_header_end, &error) < 0) { e_info(request->event, "Invalid gs2-header in request: %s", @@ -264,27 +271,56 @@ mech_oauthbearer_auth_continue(struct sasl_server_mech_request *request, return; } - /* split the data from ^A */ - const char *const *fields = - t_strsplit(t_strndup(gs2_header_end + 1, payload_size - 1), - "\x01"); - const char *const *ptr; - const char *token = NULL, *value; + const unsigned char *in = gs2_header_end; + const unsigned char *in_end = in + payload_size; + const char *token = NULL; - for (ptr = fields; *ptr != NULL; ptr++) { - if (str_begins(*ptr, "auth=", &value)) { + /* kvsep */ + in++; + + /* *kvpair */ + while (*in != 0x01 && in < in_end) { + const char *key, *value; + + if (sasl_oauth2_kvpair_parse(in, in_end - in, &key, &value, + &in, &error) < 0) { + e_info(request->event, + "Invalid response payload: " + "Bad key-value pair: %s", error); + oauth2_fail_invalid_request(oauth2_req); + return; + } + if (strcmp(key, "auth") == 0) { if (str_begins_icase(value, "bearer ", &value) && oauth2_valid_token(value)) { token = value; } else { e_info(request->event, - "Invalid response payload"); + "Invalid response payload: " + "Invalid 'auth' value"); oauth2_fail_invalid_token(oauth2_req); return; } } /* do not fail on unexpected fields */ } + + /* kvsep */ + if (in == in_end) { + e_info(request->event, "Invalid response payload: " + "Missing final key-value separator (0x01)"); + oauth2_fail_invalid_request(oauth2_req); + return; + } + i_assert(*in == 0x01); + in++; + if (in < in_end) { + e_info(request->event, "Invalid response payload: " + "Spurious data at end of response"); + oauth2_fail_invalid_request(oauth2_req); + return; + } + if (token == NULL) { e_info(request->event, "Missing token"); oauth2_fail_invalid_token(oauth2_req); diff --git a/src/lib-sasl/test-sasl-oauth2.c b/src/lib-sasl/test-sasl-oauth2.c new file mode 100644 index 0000000000..1bf43a52fc --- /dev/null +++ b/src/lib-sasl/test-sasl-oauth2.c @@ -0,0 +1,273 @@ +/* Copyright (c) 2025 Dovecot authors, see the included COPYING file */ + +#include "test-lib.h" +#include "str.h" +#include "sasl-oauth2.h" + +struct test_kvpair { + const char *key; + const char *value; +}; + +struct test_kvpair_valid { + const char *in; + + const struct test_kvpair *pairs; +}; + +static const struct test_kvpair_valid kvpair_valid_tests[] = { + { + .in = "key=value\x01", + .pairs = (const struct test_kvpair []){ + { + .key = "key", + .value = "value", + }, + { + .key = NULL, + }, + }, + }, + { + .in = "key=value=frop\x01", + .pairs = (const struct test_kvpair []){ + { + .key = "key", + .value = "value=frop", + }, + { + .key = NULL, + }, + }, + }, + { + .in = "key=value\x01keytwo=value2\x01", + .pairs = (const struct test_kvpair []){ + { + .key = "key", + .value = "value", + }, + { + .key = "keytwo", + .value = "value2", + }, + { + .key = NULL, + }, + }, + }, + { + .in = "host=server.example.com\x01" + "port=143\x01" + "auth=Bearer vF9dft4qmTc2Nvb3RlckBhbHRhdmlzdGEuY29tCg==\x01", + .pairs = (const struct test_kvpair []){ + { + .key = "host", + .value = "server.example.com", + }, + { + .key = "port", + .value = "143", + }, + { + .key = "auth", + .value = "Bearer vF9dft4qmTc2Nvb3RlckBhbHRhdmlzdGEuY29tCg==", + }, + { + .key = NULL, + }, + }, + }, + { + .in = "abcdefghijklmnopqrstuvwxyz=value1\x01" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ=value2\x01", + .pairs = (const struct test_kvpair []){ + { + .key = "abcdefghijklmnopqrstuvwxyz", + .value = "value1", + }, + { + .key = "ABCDEFGHIJKLMNOPQRSTUVWXYZ", + .value = "value2", + }, + { + .key = NULL, + }, + }, + }, + { + .in = "a=abcdefghijklmnopqrstuvwxyz\x01" + "b=ABCDEFGHIJKLMNOPQRSTUVWXYZ\x01" + "c=0123456789\x01" + "d=!\"#$%&'()*+,-./\x01" + "e=:;<=>@\x01" + "f=[/]^`{|}~\x01" + "g= \t\r\n\x01", + .pairs = (const struct test_kvpair []){ + { + .key = "a", + .value = "abcdefghijklmnopqrstuvwxyz", + }, + { + .key = "b", + .value = "ABCDEFGHIJKLMNOPQRSTUVWXYZ", + }, + { + .key = "c", + .value = "0123456789", + }, + { + .key = "d", + .value = "!\"#$%&'()*+,-./", + }, + { + .key = "e", + .value = ":;<=>@", + }, + { + .key = "f", + .value = "[/]^`{|}~", + }, + { + .key = "g", + .value = " \t\r\n", + }, + { + .key = NULL, + }, + }, + }, +}; + +static void +test_kvpair_valid_next(const unsigned char **in, const unsigned char *in_end, + const struct test_kvpair *test) +{ + const char *key, *value; + const char *error; + int ret; + + ret = sasl_oauth2_kvpair_parse(*in, in_end - *in, &key, &value, + in, &error); + test_out_reason_quiet("decode success", ret >= 0, error); + if (ret < 0) + return; + + test_assert_strcmp(key, test->key); + test_assert_strcmp(value, test->value); +} + +static void test_kvpair_valid(void) +{ + unsigned int i; + buffer_t *buf; + + buf = t_buffer_create(128); + for (i = 0; i < N_ELEMENTS(kvpair_valid_tests); i++) { + const struct test_kvpair_valid *test = &kvpair_valid_tests[i]; + const unsigned char *in = (const unsigned char *)test->in; + const unsigned char *in_end = in + strlen(test->in); + const struct test_kvpair *pairs = test->pairs; + + test_begin(t_strdup_printf("sasl oauth2 kvpair valid [%u]", + i + 1)); + + while (!test_has_failed() && + in < in_end && pairs->key != NULL) { + test_kvpair_valid_next(&in, in_end, pairs); + pairs++; + } + + test_assert(test_has_failed() || in == in_end); + test_assert(test_has_failed() || pairs->key == NULL); + + test_end(); + buffer_clear(buf); + } + +} + +struct test_kvpair_invalid { + const char *in; + size_t nul_at; + + bool expect_nonstd; +}; + +static const struct test_kvpair_invalid kvpair_invalid_tests[] = { + { + .in = "=", + }, + { + .in = "a", + }, + { + .in = "key1=a", + }, +{ + .in = "key=a\x01key2=b\x01", + }, + { + .in = "k e y=a", + }, + { + .in = "key=a\x02", + }, + { + .in = "key=a\x02\x01", + }, + { + .in = "key=value", + }, + { + .in = "key=value\x01keytwo=value2", + }, +}; + +static void test_kvpair_invalid(void) +{ + unsigned int i; + int ret; + + for (i = 0; i < N_ELEMENTS(kvpair_invalid_tests); i++) { + const struct test_kvpair_invalid *test = + &kvpair_invalid_tests[i]; + const unsigned char *in = (unsigned char *)test->in; + size_t in_len = strlen(test->in); + const unsigned char *in_end = in + in_len; + const char *key, *value; + const char *error; + + test_begin(t_strdup_printf("sasl oauth2 kvpair invalid [%u]", + i + 1)); + + if (test->nul_at > 0) { + unsigned char *in_nul; + + i_assert((test->nul_at - 1) < in_len); + in_nul = (unsigned char *)t_strdup_noconst(test->in); + in_nul[test->nul_at - 1] = '\0'; + in = in_nul; + } + + ret = 0; + while (ret == 0 && in < in_end) { + ret = sasl_oauth2_kvpair_parse(in, in_end - in, + &key, &value, &in, + &error); + } + test_out_reason("decode failure", ret < 0, error); + + test_end(); + } +} + +int main(void) +{ + static void (*const test_functions[])(void) = { + test_kvpair_valid, + test_kvpair_invalid, + NULL + }; + return test_run(test_functions); +}