Hello Willy and Aleksandar,
If you agree, I would like to apply this new patch to add some more integrity
checking on appsession.
* the session value (provided by the URL or by the request/response cookie) is
now well delimited :
currently, setting "len 52" on a 32 chars value has a bad effect on load
balancing (for example, a part of the query string is considered as the session
id ; same thing happens with the cookie).
=> with the patch, "len 52" is now a maximum length.
* it adds a verification on the '=' char :
currently (with appsession JSESSIONID for example), an URL like
http://<haproxy>/path;jsessionidfake=0123... matches the session id
"ake=0123..."
=> with the patch, jsessionidfake won't be recognized.
Tell me if you're OK for the loop added in get_srv_from_appsession(). This can
be a little slower but I think this will help to eliminate some troubles in
production.
Note : the patch is based on haproxy 1.4 snapshot "20091030".
--
Cyril Bonté
diff -Naur a/include/proto/proto_http.h b/include/proto/proto_http.h
--- a/include/proto/proto_http.h 2009-10-29 12:00:11.000000000 +0100
+++ b/include/proto/proto_http.h 2009-10-31 21:53:06.000000000 +0100
@@ -75,7 +75,7 @@
int apply_filter_to_req_line(struct session *t, struct buffer *req, struct hdr_exp *exp);
int apply_filters_to_request(struct session *t, struct buffer *req, struct hdr_exp *exp);
int apply_filters_to_response(struct session *t, struct buffer *rtr, struct hdr_exp *exp);
-void manage_client_side_appsession(struct session *t, const char *buf);
+void manage_client_side_appsession(struct session *t, const char *buf, int len);
void manage_client_side_cookies(struct session *t, struct buffer *req);
void manage_server_side_cookies(struct session *t, struct buffer *rtr);
void check_response_for_cacheability(struct session *t, struct buffer *rtr);
diff -Naur a/src/proto_http.c b/src/proto_http.c
--- a/src/proto_http.c 2009-10-29 12:00:11.000000000 +0100
+++ b/src/proto_http.c 2009-10-31 23:40:39.000000000 +0100
@@ -2479,7 +2479,7 @@
/* It needs to look into the URI */
if (s->be->appsession_name) {
- get_srv_from_appsession(s, &req->data[msg->som], msg->sl.rq.l);
+ get_srv_from_appsession(s, &req->data[msg->som + msg->sl.rq.u], msg->sl.rq.u_l);
}
/*
@@ -3778,7 +3778,7 @@
* Try to retrieve the server associated to the appsession.
* If the server is found, it's assigned to the session.
*/
-void manage_client_side_appsession(struct session *t, const char *buf) {
+void manage_client_side_appsession(struct session *t, const char *buf, int len) {
struct http_txn *txn = &t->txn;
appsess *asession = NULL;
char *sessid_temp = NULL;
@@ -3796,8 +3796,8 @@
return;
}
- memcpy(t->sessid, buf, t->be->appsession_len);
- t->sessid[t->be->appsession_len] = 0;
+ memcpy(t->sessid, buf, len);
+ t->sessid[len] = 0;
}
if ((sessid_temp = pool_alloc2(apools.sessid)) == NULL) {
@@ -3806,8 +3806,8 @@
return;
}
- memcpy(sessid_temp, buf, t->be->appsession_len);
- sessid_temp[t->be->appsession_len] = 0;
+ memcpy(sessid_temp, buf, len);
+ sessid_temp[len] = 0;
asession = appsession_hash_lookup(&(t->be->htbl_proxy), sessid_temp);
/* free previously allocated memory */
@@ -4080,7 +4080,11 @@
/* first, let's see if the cookie is our appcookie*/
/* Cool... it's the right one */
- manage_client_side_appsession(t, p3);
+ int value_len = p4 - p3;
+ if (value_len > t->be->appsession_len) {
+ value_len = t->be->appsession_len;
+ }
+ manage_client_side_appsession(t, p3, value_len);
#if defined(DEBUG_HASH)
Alert("manage_client_side_cookies\n");
appsession_hash_dump(&(t->be->htbl_proxy));
@@ -4508,8 +4512,12 @@
send_log(t->be, LOG_ALERT, "Not enough Memory process_srv():asession->sessid:malloc().\n");
return;
}
- memcpy(t->sessid, p3, t->be->appsession_len);
- t->sessid[t->be->appsession_len] = 0;
+ int value_len = p4 - p3;
+ if (value_len > t->be->appsession_len) {
+ value_len = t->be->appsession_len;
+ }
+ memcpy(t->sessid, p3, value_len);
+ t->sessid[value_len] = 0;
}/* end if ((t->proxy->appsession_name != NULL) ... */
break; /* we don't want to loop again since there cannot be another cookie on the same line */
} /* we're now at the end of the cookie value */
@@ -4653,25 +4661,35 @@
*/
void get_srv_from_appsession(struct session *t, const char *begin, int len)
{
- char *request_line;
+ char *end_params, *first_param, *cur_param, *next_param;
if (t->be->appsession_name == NULL ||
(t->txn.meth != HTTP_METH_GET && t->txn.meth != HTTP_METH_POST) ||
- (request_line = memchr(begin, ';', len)) == NULL ||
- ((1 + t->be->appsession_name_len + 1 + t->be->appsession_len) > (begin + len - request_line)))
+ ((first_param = memchr(begin, ';', len)) == NULL))
return;
- /* skip ';' */
- request_line++;
-
- /* look if we have a jsessionid */
- if (strncasecmp(request_line, t->be->appsession_name, t->be->appsession_name_len) != 0)
- return;
+ if ((end_params = memchr(first_param, '?', len - (begin - first_param))) == NULL) {
+ end_params = begin + len;
+ }
- /* skip jsessionid= */
- request_line += t->be->appsession_name_len + 1;
-
- manage_client_side_appsession(t, request_line);
+ cur_param = next_param = end_params;
+ while (cur_param > first_param) {
+ cur_param--;
+ if (cur_param[0] == ';') {
+ if ((cur_param + t->be->appsession_name_len + 1 < next_param) &&
+ (cur_param[t->be->appsession_name_len + 1] == '=') &&
+ (strncasecmp(cur_param + 1, t->be->appsession_name, t->be->appsession_name_len) == 0)) {
+ cur_param += t->be->appsession_name_len + 2;
+ int value_len = next_param - cur_param;
+ if (value_len > t->be->appsession_len) {
+ value_len = t->be->appsession_len;
+ }
+ manage_client_side_appsession(t, cur_param, value_len);
+ break;
+ }
+ next_param = cur_param;
+ }
+ }
#if defined(DEBUG_HASH)
Alert("get_srv_from_appsession\n");
appsession_hash_dump(&(t->be->htbl_proxy));
