cURL looses Cookie information when CURLOPT_FOLLOWLOCATION is true #697
Comments
|
The cookie engine has no knowledge of redirects and never had, which makes this sound strange to me. The cookie jar is only ever saved on the closing of the easy handle. Is there are chance that your code exits before that in the redirect case? |
|
I glanced at Cookie.c briefly and read the following comment "The 'lineptr' parameter is a full "Set-cookie:" line as received from a server." That makes me think that the parameter is initialized fresh upon redirect and any previous Cookie information is now gone. Perhaps previous versions did not init this parameter again upon redirect, saving the Cookie information. Regardless, the old combo of php5-curl 5.3 and libcurl 7.26 saved the Cookie. Now I'll just have to find out why. The application we built remotely logs into the Siemens DECT base stations we administer, and we also configure them this way using a PHP script for our SIP PBX. We've been using this application for quite a while now, as early as Debian 6.0 and it's worked with redirects since that time :) I'm now setting up a VM to see if I can track down where the changes came from. I'm not entirely sure if this is caused in changes from php5-curl 5.3 to php5-curl 5.6 or from the change from curl 7.26 to 7.38. Completely rewriting the curl Class I use in our code to save the Cookie manually and scrape the headers from the redirect seems really backwards to me. I think cURL was made explicitly to not have to do such a thing ;) |
|
I've uploaded some transferlogs for stdout and stderr from Debian 7 and Debian 8. If you look at the err log from Debian 8 with regards to the POST request on the login form I see that it picks up on the Cookie. It says it adds the cookie apparently with the 303 reponse. The next request is for home.html, but it performs this as a GET request. This one is performed without the cookie and any subsequent page is for login.html. At this point the base station is locked and you can't get in anymore, you have to powercycle the base station to make it forget the session. transferlog.debian8.err.txt |
|
Backported 7.38.0 from Jessie onto Wheezy and that is enough to break it. (With PHP staying the same) I'm trying to build intermediate versions but this is my first time building Debian packages. |
|
Confirmed |
|
I came as far today as 7.28.0 before the workday ended. Fastest method I found for building a debian package with an older version is clone the git tree from http://anonscm.debian.org/cgit/collab-maint/curl.git/log/?ofs=100 and then check out different points in time and build from that using I can then build a new package using I can probably downgrade our production box by building a older package this way without entirely trashing the entire system :) Unfortunately the base station got buggered again so I can test over the weekend. |
|
I think trying to find or setup a way we can reproduce this problem with the current version is a better spent time. |
|
Bisected to 4cfbb20, I'm on it |
|
Oh, the expire string is indeed funny looking. Does it fail to parse that and then expire the cookie? |
|
It's the date, 21-12-2037 doesn't work but 21-Dec-2037 does, that is why expiry is 0 and is being purged. I'll consult the RFC and see if this is correct, however would there be any disadvantage to adding a fix for this even if it's not complaint? |
|
Our date parser is already accepting lots of non compliant formats. I won't mind if we support this one too, if we can do that without breaking something else... |
|
Basically anything that could possibly relate to this (netscape cookie spec and date specs in RFCs 822, 850, 1036, 1123, 2965, 6265) However the date syntax spec in 822 on which some of the others based off of (and also suggest compliance with) has dd mm yy in the comment. I know I'm really reaching here but mm is the shorthand for a two digit month (as opposed to mmm), so maybe someone misread that RFC. IE 9/10 and FF 38/47.0a1 parse the date but Chrome 49 won't and treats it as a session cookie. It seems to me there could be two issues here, first we aren't sending this session cookie on redirect and second we aren't parsing this cookie's date format which turns it into a session cookie. As far as relaxing the date parser it looks like if I change it without being specific to cookies then situations like 01-02-38 it could be Feb 1 or Jan 2. However |
|
Making sure it gets treated as a session cookie when the date parser fails seems like a decent first priority. (I'm on vacation atm so I only cheer from the sidelines right now) |
|
It is treated as a session cookie but not in the function that purges expired cookies so that's a problem. That function will check against diff --git a/lib/cookie.c b/lib/cookie.c
index 1fd97c0..558b6a7 100644
--- a/lib/cookie.c
+++ b/lib/cookie.c
@@ -304,17 +304,17 @@ static void remove_expired(struct CookieInfo *cookies)
{
struct Cookie *co, *nx, *pv;
curl_off_t now = (curl_off_t)time(NULL);
co = cookies->cookies;
pv = NULL;
while(co) {
nx = co->next;
- if((co->expirestr || co->maxage) && co->expires < now) {
+ if(co->expires && co->expires < now) {
if(co == cookies->cookies) {
cookies->cookies = co->next;
}
else {
pv->next = co->next;
}
cookies->numcookies--;
freecookie(co);
The date thing as I noted in my edit is more complicated than I originally thought. The RFCs do not explicitly spell out the order as day month year although it's implied various places. And the latest RFC 6265 allows the tokens in more than one order and gives an algorithm to process them. The dd-mm-yyyy format is not compliant. And I read up on curl_getdate. I could either add the dd-mm-yyyy format to that function as acceptable always or make a flag and a stub or a new function that can only be used internally and is specific to cookies, this way the public api will not be affected. |
|
Uh that side effect statement is wrong. It seems that by the current logic cookies that were read in the Netscape format cannot be purged by |
|
It looks correct to me! |
Prior to this change cookies with an expiry date that failed parsing and were converted to session cookies could be purged in remove_expired. Bug: #697 Reported-by: Seth Mos
|
Unfortunately this seems to break Test 46 removing the
For more detail please see the autobuilds. |
|
I can confirm that. That is of course because those are expired cookies and the newly modified expiry function will now (correctly) expire them! I propose the fix for this is to bump the numerical to the 2 billion range. |
|
Fixed in commit e6293cf |
|
I just verified that this fix works for me, I put a patched version on our production box and it's doing it's thing again. Now, to contact people to get 250 base stations power cycled. :) |
|
Thanks for the update Seth. Please be aware there are two issues here:
I've only solved Right now because Edit 2017-11-21: Though we decided last year not to fix part |
|
I consider this issue fixed, as it was brought back to the state it was before. Fixing the parsing of that non-compliant date format could be considered an added bonus but nothing to lose any sleep over since it has to be a very small fraction of the world's cookies using that. |
In that version was fixed the bug, which looses Cookie information when CURLOPT_FOLLOWLOCATION is true. curl/curl#697
The expiry format used by Siemens: [day, ]dd-mm-yy[yy] hh:mm:ss zzz The problem is the dd-mm-yy[yy] which is not a correct format. This fix replaces the dd-mm-yy[yy] with dd-mmm-yy[yy], which is valid. In other words internally the month digits (mm) are replaced with the abbreviated 3 letter name (mmm) to make it parseable. And the day-of-week is ignored since it doesn't matter for this purpose. eg 12 => Dec ------------------------------------------------------------------------ Set-Cookie: key=14683720; path=/; expires=Sat, 21-12-2037 00:00:00 GMT parsing fails: Sat, 21-12-2037 00:00:00 GMT fix applied: 21-Dec-2037 00:00:00 GMT parsed ok * Added cookie key="14683720" for domain 127.0.0.1, path /, expire 2144966400 < Set-Cookie: key=14683720; path=/; expires=Sat, 21-12-2037 00:00:00 GMT ------------------------------------------------------------------------ Set-Cookie: key=14683720; path=/; expires=Sat, 11-12-37 00:00:00 GMT parsing fails: Sat, 11-12-37 00:00:00 GMT fix applied: 11-Dec-37 00:00:00 GMT parsed ok * Added cookie key="14683720" for domain 127.0.0.1, path /, expire 2144102400 < Set-Cookie: key=14683720; path=/; expires=Sat, 11-12-37 00:00:00 GMT ------------------------------------------------------------------------ Fixes curl#697
I did this
On Debian 7.9 with cURL 7.26.0-1+wheezy13 and PHP 5.3.3-7+squeeze19 we can succesfully login into Siemens C470/N300 DECT base stations. After upgrading to Debian 8 and cURL 7.38.0 this no longer works. I've also tested this on Debian 9 (Stretch/sid) with cURL 7.47.0 and the problem exists there still.
No cookies were saved to the Cookie JAR on either of these occasions.
After debugging somewhat I appear to have found the issue, the authentication is succesful, and the host returns a cookie with a 303 redirect. This cookie is then lost for any subsequent request, nor is it written to the cookie jar.
Apparently there has been a change since cURL 7.26 that no longer saves the cookie on redirect. Unfortunate. It appears I was not the only one that ran into this issue.
I expected the following
I expected a successful login where the cookie is saved to the jar when the login page redirects as 7.26.0 previously did. Perhaps this was changed for security reasons but with a side effect. I could not find a option for cURL to save this cookie on redirect.
curl/libcurl version
seth@lsautom:~$ curl --version
curl 7.47.0 (x86_64-pc-linux-gnu) libcurl/7.47.0 GnuTLS/3.4.9 zlib/1.2.8 libidn/1.32 libssh2/1.5.0 nghttp2/1.7.1 librtmp/2.3
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtmp rtsp scp sftp smb smbs smtp smtps telnet tftp
Features: AsynchDNS IDN IPv6 Largefile GSS-API Kerberos SPNEGO NTLM NTLM_WB SSL libz TLS-SRP HTTP2 UnixSockets
operating system
Debian Stretch/sid ~02-03-2016
Example output:
Login not succesful after redirect. The cookie file is empty.
Example PHP code
The text was updated successfully, but these errors were encountered: