Re: Small patch for the appsession feature
Hi Cyril,
On Sun, Nov 01, 2009 at 12:19:05AM +0100, Cyril Bonté wrote:
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.
OK.
* 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.
I think we must not do this, because one of the reasons for not
checking the equal sign was because of people who have to use IIS
with its ASPSESSION cookies. The end of the cookie name is part
of the value. So it's important that we can check cookie prefixes.
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.
At first glance I'm not too much scared.
However, please avoid variable declaration in the middle of the
code such as below, this is not compatible with compilers before
gcc version 3.0.
@@ -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);
Cheers,
Willy
Re: Small patch for the appsession feature
Le lundi 2 novembre 2009 12:09:43, Willy Tarreau a écrit :
* 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.
I think we must not do this, because one of the reasons for not
checking the equal sign was because of people who have to use IIS
with its ASPSESSION cookies.
Interesting to know about this ASPSESSION cookies, I'm not an IIS user.
The end of the cookie name is part of the value.
Ok, note that the only modification I made is for the url part, not for the
cookie. So maybe this is not a problem.
After some searches, I've seen that IIS does'nt support natively cookieless
persistance : it needs an ISAPI filter to do so (named Cookie munge).
I've read this filter sources and it looks like the URL part for
ASPSESSIONID is -ASP= (without the left side value), which doesn't start
with the semi-colon awaited by the appsession code.
Maybe I'm wrong, I'm really not an ASP developper and the information I found
are maybe outdated (it concerned IIS 4.0/5.0).
So it's important that we can check cookie prefixes.
Sorry but I can't see in the haproxy sources how the cookie prefix can be used
for appsession.
capture cookie allows to find this cookie prefix but it seems there's no code
to then use it in appsession. If it's the case, I can work on it these next
days, if you want.
However, please avoid variable declaration in the middle of the
code such as below, this is not compatible with compilers before
gcc version 3.0.
@@ -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);
Oops, I missed it :)
--
Cyril Bonté
