Author: brane Date: Wed Aug 6 05:13:16 2025 New Revision: 1927628 Log: SERF-207: The Digest authn provider verifies received parameters.
* serf_private.h (serf__find_token): New prototype. * src/syntax.c (skip_not_space): New; skips non-space characters. (serf__find_token): New; finds a token in a space-separated list. * auth/auth_digest.c (serf__handle_digest_auth): Verify that incoming authn parameters are supported: we only support the (default) MD5 hash and the "auth" qop. * test/test_auth.c (digest_check_parameters, test_digest_valid_params, test_digest_invalid_params): New test functions. (test_auth): Register the new test cases. * test/test_internal.c (test_find_token): New test for serf__find_token(). (test_internal): Register the new test case. Modified: serf/trunk/auth/auth_digest.c serf/trunk/serf_private.h serf/trunk/src/syntax.c serf/trunk/test/test_auth.c serf/trunk/test/test_internal.c Modified: serf/trunk/auth/auth_digest.c ============================================================================== --- serf/trunk/auth/auth_digest.c Wed Aug 6 04:59:43 2025 (r1927627) +++ serf/trunk/auth/auth_digest.c Wed Aug 6 05:13:16 2025 (r1927628) @@ -288,6 +288,23 @@ serf__handle_digest_auth(const serf__aut return SERF_ERROR_AUTHN_MISSING_ATTRIBUTE; } + /* We only support the MD5 hash, fail early if it's anything else. */ + if (algorithm && strcmp(algorithm, "MD5")) { + apr_pool_destroy(scratch_pool); + return SERF_ERROR_AUTHN_NOT_SUPPORTED; + } + + /* The qop parameter must contain "auth", as that's the only value we + support. Fail early if it's not one of the requested qop modes. */ + if (qop) { + qop = serf__find_token("auth", 4, qop); + if (!qop) { + apr_pool_destroy(scratch_pool); + return SERF_ERROR_AUTHN_NOT_SUPPORTED; + } + qop = "auth"; /* qop must NUL-terminated. */ + } + realm = serf__construct_realm(SERF__PEER_FROM_CODE(code), conn, realm_name, pool); Modified: serf/trunk/serf_private.h ============================================================================== --- serf/trunk/serf_private.h Wed Aug 6 04:59:43 2025 (r1927627) +++ serf/trunk/serf_private.h Wed Aug 6 05:13:16 2025 (r1927628) @@ -788,6 +788,16 @@ void serf__tolower_inplace(char *dst, ap /* Like serf__tolower_inplace, but allocates a new string from the pool. */ const char *serf__tolower(const char *src, apr_pool_t *pool); +/* Find a given TOKEN in string of whitespace-delimited tokens SRC. + If LEN > 0, it is the length of TOKEN; otherwise this function will + call strlen() to find the length. All comparisons are case-sensitive. + Whitespace is either space ('\x20') or horizontal tab ('\x09'). + + NOTE: This function does not modify SRC, so there's no guarantee that + the returned token is properly NUL-terminated, as it may have + been found somewhere in the middle of the string. */ +const char *serf__find_token(const char *token, apr_size_t len, const char *src); + /* from context.c */ void serf__context_progress_delta(void *progress_baton, apr_off_t read, apr_off_t written); Modified: serf/trunk/src/syntax.c ============================================================================== --- serf/trunk/src/syntax.c Wed Aug 6 04:59:43 2025 (r1927627) +++ serf/trunk/src/syntax.c Wed Aug 6 05:13:16 2025 (r1927628) @@ -87,6 +87,12 @@ static const char *skip_space(const char return src + strspn(src, " \t"); } +/* Skip non-space characters. */ +static const char *skip_not_space(const char *src) +{ + return src + strcspn(src, " \t"); +} + /* Skip token68 */ static const char *skip_token68(const char *src) { @@ -244,3 +250,19 @@ const char *serf__tolower(const char *sr dst[i] = ct_tolower(src[i]); /* The NUL byte is copied, too. */ return dst; } + + +const char *serf__find_token(const char *token, apr_size_t len, const char *src) +{ + if (len == 0) + len = strlen(token); + + src = skip_space(src); + while (*src) { + const char *end = skip_not_space(src); + if (end - src == len && 0 == strncmp(token, src, len)) + return src; + src = skip_space(end); + } + return NULL; +} Modified: serf/trunk/test/test_auth.c ============================================================================== --- serf/trunk/test/test_auth.c Wed Aug 6 04:59:43 2025 (r1927627) +++ serf/trunk/test/test_auth.c Wed Aug 6 05:13:16 2025 (r1927628) @@ -309,6 +309,75 @@ static void test_digest_authentication_k digest_authentication(tc, 1); } +static void digest_check_parameters(CuTest *tc, + apr_status_t expected_status, + const char *alg, + const char *qop) +{ + test_baton_t *tb = tc->testBaton; + handler_baton_t handler_ctx[2]; + const char *hdr; + apr_status_t status; + + /* fprintf(stderr, "alg=%s qop=%s\n", alg, qop); */ + + /* Set up a test context with a server */ + setup_test_mock_server(tb); + status = setup_test_client_context(tb, NULL, tb->pool); + CuAssertIntEquals(tc, APR_SUCCESS, status); + + serf_config_authn_types(tb->context, SERF_AUTHN_DIGEST); + serf_config_credentials_callback(tb->context, digest_authn_callback); + + create_new_request(tb, &handler_ctx[0], "GET", "/", 1); + + /* Construct the response header. */ + alg = alg ? apr_psprintf(tb->pool, ",algorithm=\"%s\"", alg) : ""; + qop = qop ? apr_psprintf(tb->pool, ",qop=\"%s\"", qop) : ""; + hdr = apr_psprintf(tb->pool, "Digest realm=\"Test Suite\"," + "nonce=\"ABCDEF1234567890\",opaque=\"myopaque\"" + "%s%s", alg, qop); + /* fprintf(stderr, "hdr=%s\n", hdr); */ + + Given(tb->mh) + GETRequest(URLEqualTo("/"), HeaderNotSet("Authorization")) + Respond(WithCode(401), WithChunkedBody("1"), + WithHeader("www-Authenticate", hdr)) + GETRequest(URLEqualTo("/"), HeaderSet("Authorization")) + Respond(WithCode(200), WithChunkedBody("")) + Expect + AllRequestsReceivedInOrder + EndGiven + + status = run_client_and_mock_servers_loops(tb, 1, handler_ctx, tb->pool); + CuAssertIntEquals(tc, expected_status, status); +} + +static void test_digest_valid_params(CuTest *tc) +{ + digest_check_parameters(tc, APR_SUCCESS, NULL, NULL); + digest_check_parameters(tc, APR_SUCCESS, "MD5", NULL); + digest_check_parameters(tc, APR_SUCCESS, NULL, "auth"); + digest_check_parameters(tc, APR_SUCCESS, "MD5", "auth"); + digest_check_parameters(tc, APR_SUCCESS, "MD5", "auth-int auth"); + digest_check_parameters(tc, APR_SUCCESS, "MD5", "crumple auth auth-int"); +} + +static void test_digest_invalid_params(CuTest *tc) +{ + static const apr_status_t expected = SERF_ERROR_AUTHN_NOT_SUPPORTED; + digest_check_parameters(tc, expected, "", NULL); + digest_check_parameters(tc, expected, "MD5-sess", NULL); + digest_check_parameters(tc, expected, "SHA-256", NULL); + digest_check_parameters(tc, expected, "SHA-256-sess", NULL); + digest_check_parameters(tc, expected, "SHA-512-256", NULL); + digest_check_parameters(tc, expected, "SHA-512-256-sess", NULL); + digest_check_parameters(tc, expected, NULL, ""); + digest_check_parameters(tc, expected, NULL, "auth-int"); + digest_check_parameters(tc, expected, NULL, "crumple"); + digest_check_parameters(tc, expected, NULL, "crumple auth-int"); +} + static apr_status_t switched_realm_authn_callback(char **username, char **password, @@ -1020,6 +1089,8 @@ CuSuite *test_auth(void) SUITE_ADD_TEST(suite, test_basic_authentication_keepalive_off); SUITE_ADD_TEST(suite, test_digest_authentication); SUITE_ADD_TEST(suite, test_digest_authentication_keepalive_off); + SUITE_ADD_TEST(suite, test_digest_valid_params); + SUITE_ADD_TEST(suite, test_digest_invalid_params); SUITE_ADD_TEST(suite, test_basic_switch_realms); SUITE_ADD_TEST(suite, test_digest_switch_realms); SUITE_ADD_TEST(suite, test_auth_on_HEAD); Modified: serf/trunk/test/test_internal.c ============================================================================== --- serf/trunk/test/test_internal.c Wed Aug 6 04:59:43 2025 (r1927627) +++ serf/trunk/test/test_internal.c Wed Aug 6 05:13:16 2025 (r1927628) @@ -557,6 +557,22 @@ static void test_parameter_case_folding( parse_parameters(tc, "01234ABCDEFGHIJKLMNOPQRSTUVWXYZ56789=Val", expected); } +static void test_find_token(CuTest *tc) +{ + CuAssertPtrNotNull(tc, serf__find_token("foo", 0, "foo")); + CuAssertPtrNotNull(tc, serf__find_token("foo", 3, "foo")); + CuAssertPtrNotNull(tc, serf__find_token("foo", 3, " foo")); + CuAssertPtrNotNull(tc, serf__find_token("foo", 3, "foo ")); + CuAssertPtrNotNull(tc, serf__find_token("foo", 3, "bar\tfoo")); + CuAssertPtrNotNull(tc, serf__find_token("foo", 3, "foo\tbar")); + CuAssertPtrNotNull(tc, serf__find_token("foo", 3, "bar\t #$@*&^! foo qux")); + CuAssertPtrEquals(tc, NULL, serf__find_token("foo", 2, "foo")); + CuAssertPtrEquals(tc, NULL, serf__find_token("foo", 3, "\vfoo")); + CuAssertPtrEquals(tc, NULL, serf__find_token("foo", 3, "foobar")); + CuAssertPtrEquals(tc, NULL, serf__find_token("foo", 3, " qux foobar baz")); + CuAssertPtrEquals(tc, NULL, serf__find_token("foo", 3, " qux bar")); +} + CuSuite *test_internal(void) { @@ -577,6 +593,7 @@ CuSuite *test_internal(void) SUITE_ADD_TEST(suite, test_parse_repeated_parameters); SUITE_ADD_TEST(suite, test_parse_single_token_parameters); SUITE_ADD_TEST(suite, test_parameter_case_folding); + SUITE_ADD_TEST(suite, test_find_token); return suite; }