Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set-Cookie silently no longer sent without domain #6723

Closed
martindorey opened this issue Mar 11, 2021 · 9 comments
Closed

Set-Cookie silently no longer sent without domain #6723

martindorey opened this issue Mar 11, 2021 · 9 comments
Assignees
Labels

Comments

@martindorey
Copy link

@martindorey martindorey commented Mar 11, 2021

I did this

$ cat /tmp/cookie
Set-Cookie: key=value
$ curl --verbose --cookie /tmp/cookie http://blank.org/

With 7.60.0, I get no cookie sent:

martind@pizzagate:/tmp/curl-7.60.0$ ./src/curl --verbose --cookie /tmp/cookie http://blank.org/
*   Trying 18.217.80.105...
* TCP_NODELAY set
* Connected to blank.org (18.217.80.105) port 80 (#0)
> GET / HTTP/1.1
> Host: blank.org
> User-Agent: curl/7.60.0
> Accept: */*
> 
< HTTP/1.1 200 OK
...

I expected the following

Cookie: key=value

... as seen in:

martind@pizzagate:/tmp/curl-7.59.0$ ./src/curl --verbose --cookie /tmp/cookie http://blank.org/
*   Trying 18.217.80.105...
* TCP_NODELAY set
* Connected to blank.org (18.217.80.105) port 80 (#0)
> GET / HTTP/1.1
> Host: blank.org
> User-Agent: curl/7.59.0
> Accept: */*
> Cookie: key=value
> 
< HTTP/1.1 200 OK
...

curl/libcurl version

martind@pizzagate:/tmp/curl-7.60.0$ ./src/curl -V
curl 7.60.0 (x86_64-pc-linux-gnu) libcurl/7.60.0 OpenSSL/1.1.1d zlib/1.2.11 libidn2/2.0.5 libpsl/0.20.2 (+libidn2/2.0.5) nghttp2/1.36.0 librtmp/2.3
Release-Date: 2018-05-16
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtmp rtsp smb smbs smtp smtps telnet tftp 
Features: AsynchDNS IDN IPv6 Largefile NTLM NTLM_WB SSL libz TLS-SRP HTTP2 UnixSockets HTTPS-proxy PSL 
martind@pizzagate:/tmp/curl-7.60.0$ 

operating system

martind@pizzagate:/tmp/curl-7.60.0$ uname -a
Linux pizzagate 4.19.0-9-amd64 #1 SMP Debian 4.19.118-2+deb10u1 (2020-06-07) x86_64 GNU/Linux
martind@pizzagate:/tmp/curl-7.60.0$ 

If I add an appropriate domain to the Set-Cookie file, then all is well:

martind@pizzagate:/tmp/curl-7.60.0$ cat /tmp/cookie
Set-Cookie: key=value; domain=blank.org
martind@pizzagate:/tmp/curl-7.60.0$ ./src/curl --verbose --cookie /tmp/cookie http://blank.org/
*   Trying 18.217.80.105...
* TCP_NODELAY set
* Connected to blank.org (18.217.80.105) port 80 (#0)
> GET / HTTP/1.1
> Host: blank.org
> User-Agent: curl/7.60.0
> Accept: */*
> Cookie: key=value
> 
< HTTP/1.1 200 OK

Why am I sending you a bug report on a version from 3 years ago? Because I suspect that the cause of my pain is a change in this version, specifically b8d5036, which I will attempt to demonstrate by nobbling part of it and showing that the cookie returns:

martind@pizzagate:/tmp/curl-7.60.0$ diff -u ./lib/cookie.c{,.martind}
--- ./lib/cookie.c	2021-03-10 23:45:36.309320486 -0800
+++ ./lib/cookie.c.martind	2021-03-10 23:45:11.113604028 -0800
@@ -292,7 +292,7 @@
     return 0;
 
   top = get_top_domain(domain, &len);
-  return cookie_hash_domain(top, len);
+  return cookie_hash_domain(top, len) * 0;
 }
 
 /*
martind@pizzagate:/tmp/curl-7.60.0$ cp ./lib/cookie.c{.martind,}
martind@pizzagate:/tmp/curl-7.60.0$ make -j8
...
martind@pizzagate:/tmp/curl-7.60.0$ cat /tmp/cookie
Set-Cookie: key=value
martind@pizzagate:/tmp/curl-7.60.0$ ./src/curl --verbose --cookie /tmp/cookie http://blank.org/
*   Trying 18.217.80.105...
* TCP_NODELAY set
* Connected to blank.org (18.217.80.105) port 80 (#0)
> GET / HTTP/1.1
> Host: blank.org
> User-Agent: curl/7.60.0
> Accept: */*
> Cookie: key=value
> 
< HTTP/1.1 200 OK
...

I have no problem with the Set-Cookie file format requiring a domain. It would be nice if the requirement were documented, though the documentation for this file format is... skimpy. I suspect that this was an accidental change in behavior, a side-effect of a doubtless worthy performance improvement. If it's a change that trips up others, then they might appreciate finding a bug report that nudges them in the direction of adding a domain.

@bagder
Copy link
Member

@bagder bagder commented Mar 11, 2021

(This may have been an inadvertent change, I don't remember but I also don't find any explicit commit saying this.)

The problem is that without a specific domain for the cookie, curl won't know for what sites to use the cookie and it would then presume it is for all domains you specify on the command line, and that's probably not many users want. The internal cookie engine needs a (single specific) domain to "attach" the cookie to.

The solution is to store cookies with -c as a cookie-jar instead of just saving HTTP headers with partial information.

This is a behavior change and as such it is a bug, but I don't think this is one that we'll benefit a lot by fixing. Especially not if it has been this way for 2.5 years already...

@bagder bagder added the HTTP label Mar 11, 2021
@danielgustafsson
Copy link
Member

@danielgustafsson danielgustafsson commented Mar 11, 2021

Actually, I think we have to fix it. Cookies with the __Host- prefix are not allowed to have a domain attribute.

@danielgustafsson
Copy link
Member

@danielgustafsson danielgustafsson commented Mar 11, 2021

I'm taking a crack at this, along with some other cookie related things I had on my TODO while in there.

@martindorey
Copy link
Author

@martindorey martindorey commented Mar 11, 2021

I doubt I'm adding any value here, but I was changing code that had been passing the cookie like this:

martind@pizzagate:/tmp/curl-7.60.0$ ./src/curl --silent --verbose --cookie key=value http://blank.org/ 2>&1 | grep Cookie
> Cookie: key=value
martind@pizzagate:/tmp/curl-7.60.0$ 

That works in all versions of curl that I tried, so there must already be some code path that's happy with at least one cookie without a domain (perhaps that's what @danielgustafsson told us already). My goal was to hide the cookie's value from the process list. I did read:

As a convenience, curl also supports a cookie file being a set of HTTP headers that set cookies. It's an inferior format but may be the only thing you have.

... from https://everything.curl.dev/http/cookies but the cookiejar format from https://curl.se/docs/http-cookies.html made me think "surely I don't need that complexity". I was sad not to be able to use <filename with --cookie like I was using with --form.

@bagder
Copy link
Member

@bagder bagder commented Mar 11, 2021

I was sad not to be able to use <filename with --cookie like I was using with --form.

You mean by using that specific syntax? The command line tool is unfortunately wildly inconsistent in how you specify a file to read vs plain content, when the same option can do both.

@martindorey
Copy link
Author

@martindorey martindorey commented Mar 11, 2021

You mean by using that specific syntax?

I don't think I even tried that before disabusing myself of that hope from the man page. The man page did hint that I might need a "Set-Cookie: " prefix in the file, but it was subtle enough that I don't think I took the hint without StackOverflow's help. A comment on StackOverflow then misled me into thinking I needed a semi-colon on the end. I'm not sure where I learned that I did need a newline on the end - otherwise no cookie is sent, though there's no warning to say "I spurn your half-baked --cookie!". Ooh, I see the man page does clearly state that omitting the domain is allowed:

If you use the NAME1=VALUE1; format, or in a file use the Set-Cookie format and don't specify a domain, then the cookie is sent for any domain

@bagder
Copy link
Member

@bagder bagder commented Mar 11, 2021

The man page did hint that I might need a "Set-Cookie: " prefix in the file

...

I'm not sure where I learned that I did need a newline on the end

The man page says:

The file format of the file to read cookies from should be plain HTTP headers (Set-Cookie style) or the Netscape/Mozilla cookie file format.

... and cookie headers ("Set-Cookie style") of course need to be headers and a HTTP header always ends with a newline. In my mind that is clear but perhaps that should be clarified better.

I now assume that when you talked about <filename you meant that the file in question would only contain "name=value" ?

If you only did this to hide the cookie from PS output you could instead go with:

echo "-b name=value" | curl -K - $URL
@martindorey
Copy link
Author

@martindorey martindorey commented Mar 12, 2021

you meant that the file in question would only contain "name=value" ?

Indeed. I'd value a single example in the man page, one that stopped me fretting over whether a Set-Cookie: header includes the Set-Cookie: bit or the trailing punctuation, one that would help users of existing versions too, over that ~suggestion.

echo "-b name=value" | curl -K - $URL

And because echo is a shell builtin, it won't appear in ps, even fleetingly - how cunning.

@bagder
Copy link
Member

@bagder bagder commented May 10, 2021

At the point in time when libcurl reads the cookie file from disk it hasn't parsed the provided URL yet so it doesn't even know what host name it will use from there and therefore it cannot easily use that as for example a default domain for headers stored without it.

I'll keep my position that I think we should rather document this behavior and urge people to not load cookies from stored HTTP headers but instead use the more "appropriate" cookie jar format for that purpose. Or provide the domain property if the HTTP headers are generated by something else.

bagder added a commit that referenced this issue May 16, 2021
... or the cookies won't get sent. Push users to using the "Netscape"
format instead, which curl uses when saving a cookie "jar".

Reported-by: Martin Dorey
Fixes #6723
@bagder bagder closed this in 5dfa4c0 May 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants