From 4a3fd4c8df194f530301bf8a7eeee9cffb2025c2 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Thu, 10 May 2012 23:18:26 +0200 Subject: [PATCH] BUG/MAJOR: acl: http_auth_group() must not accept any user from the userlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit http_auth and http_auth_group used to share the same fetch function, while they're doing very different things. The first one only checks whether the supplied credentials are valid wrt a userlist while the second not only checks this but also checks group ownership from a list of patterns. Recent acl/pattern merge caused a simplification here by which the fetch function would always return a boolean, so the group match was always fine if the user:password was valid, regardless of the patterns provided with the ACL. The proper solution consists in splitting the function in two, depending on what is desired. It's also worth noting that check_user() would probably be split, one to check user:password, and the other one to check for group ownership for an already valid user:password combination. At this point it is not certain if the group mask is still useful or not considering that the passwd check is always made. This bug was reported and diagnosed by Cyril Bonté. It first appeared in 1.5-dev9 so it does not need any backporting. --- src/proto_http.c | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/src/proto_http.c b/src/proto_http.c index 39a1fd6a1..1fc18c36e 100644 --- a/src/proto_http.c +++ b/src/proto_http.c @@ -8009,6 +8009,38 @@ acl_fetch_http_auth(struct proxy *px, struct session *l4, void *l7, unsigned int return 1; } +/* Accepts exactly 1 argument of type userlist */ +static int +acl_fetch_http_auth_grp(struct proxy *px, struct session *l4, void *l7, unsigned int opt, + const struct arg *args, struct sample *smp) +{ + + if (!args || args->type != ARGT_USR) + return 0; + + CHECK_HTTP_MESSAGE_FIRST(); + + if (!get_http_auth(l4)) + return 0; + + /* acl_match_auth() will need several information at once */ + smp->ctx.a[0] = args->data.usr; /* user list */ + smp->ctx.a[1] = l4->txn.auth.user; /* user name */ + smp->ctx.a[2] = l4->txn.auth.pass; /* password */ + + /* if the user does not belong to the userlist or has a wrong password, + * report that it unconditionally does not match. Otherwise we return + * a non-zero integer which will be ignored anyway since all the params + * that acl_match_auth() will use are in test->ctx.a[0,1,2]. + */ + smp->type = SMP_T_BOOL; + smp->data.uint = check_user(args->data.usr, 0, l4->txn.auth.user, l4->txn.auth.pass); + if (smp->data.uint) + smp->type = SMP_T_UINT; + + return 1; +} + /* Try to find the next occurrence of a cookie name in a cookie header value. * The lookup begins at . The pointer and size of the next occurrence of * the cookie value is returned into *value and *value_l, and the function @@ -8443,7 +8475,7 @@ static struct acl_kw_list acl_kws = {{ },{ { "hdr_val", acl_parse_int, smp_fetch_hdr_val, acl_match_int, ACL_USE_L7REQ_VOLATILE, ARG2(0,STR,SINT), val_hdr }, { "http_auth", acl_parse_nothing, acl_fetch_http_auth, acl_match_nothing, ACL_USE_L7REQ_VOLATILE, ARG1(0,USR) }, - { "http_auth_group", acl_parse_strcat, acl_fetch_http_auth, acl_match_auth, ACL_USE_L7REQ_VOLATILE, ARG1(0,USR) }, + { "http_auth_group", acl_parse_strcat, acl_fetch_http_auth_grp, acl_match_auth, ACL_USE_L7REQ_VOLATILE, ARG1(0,USR) }, { "http_first_req", acl_parse_nothing, acl_fetch_http_first_req, acl_match_nothing, ACL_USE_L7REQ_PERMANENT, 0 }, { "method", acl_parse_meth, acl_fetch_meth, acl_match_meth, ACL_USE_L7REQ_PERMANENT, 0 },