search

grafana / k6

A modern load testing tool, using Go and JavaScript - https://k6.io
GNU Affero General Public License v3.0
26.2k stars 1.27k forks source link

Cookies with a colon in their name are ignored #1847

Open theNailz opened 3 years ago

theNailz commented 3 years ago

When a server responds with a Set-Cookie header with a cookie name that contains a colon, the cookie gets ignored and is not stored in the cookie jar.

Environment

Expected Behavior

Cookies with a colon in their name are stored in the cookie jar.

Actual Behavior

Any cookie with a colon (:) in their name are ignored and not stored in the cookie jar.

Steps to Reproduce the Problem

  1. Request a page which responds with a Set-Cookie header that defined a cookie with a colon in its name, such as my:cookie=somevalue
  2. Inspect the cookie jar / do a subsequent request. 2.1. Observe the cookie my:cookie is missing from the jar/subsequent requests.

I do not have a public working example but I can share my received Set-Cookie headers which were not parsed: (tokens are invalid, don't worry)

INFO[0010] Response:
HTTP/2.0 302 Found,
Location: https://..new location..
Set-Cookie: toegang.meldcode=CH3DLAKY%20-%20PP9KS8VS; Path=/; Expires=Mon, 08 Feb 2021 13:40:18 GMT; HttpOnly; Secure
Set-Cookie: toegang.meldcode=CH3DLAKY%20-%20PP9KS8VS; Path=/; Expires=Mon, 08 Feb 2021 13:40:18 GMT; HttpOnly; Secure
Set-Cookie: _interaction_resume=; path=/oidc/auth/Lj6oYyC3oG7sJBZEfHh4R; expires=Thu, 01 Jan 1970 00:00:00 GMT; samesite=none; secure; httponly
Set-Cookie: _interaction_resume.legacy=; path=/oidc/auth/Lj6oYyC3oG7sJBZEfHh4R; expires=Thu, 01 Jan 1970 00:00:00 GMT; secure; httponly
Set-Cookie: toegang:Lj6oYyC3oG7sJBZEfHh4R=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT
Set-Cookie: _session=n6B5bOoCQEBfpxgNl7tT7; path=/; expires=Mon, 22 Mar 2021 13:35:18 GMT; samesite=none; secure; httponly
Set-Cookie: _session.legacy=n6B5bOoCQEBfpxgNl7tT7; path=/; expires=Mon, 22 Mar 2021 13:35:18 GMT; secure; httponly

In this example the following cookie is ignored: toegang:Lj6oYyC3oG7sJBZEfHh4R

(Yes, I noticed there are double headers, this is straight from --http-debug=full.)

na-- commented 3 years ago

Hmm it doesn't seem like colon is a valid character in a cookie name: https://gist.github.com/jeremiahlee/ff7dfb60572211dce306 And k6 is using Go's net/http library, which follows RFCs somewhat strictly, so I'm not sure there's a lot we can do here...

mstoykov commented 3 years ago

from https://github.com/golang/go/issues/12698#issuecomment-143214403 we can try something like https://github.com/StalkR/misc/blob/master/net/http/spacecookies/spacecookies.go, but I would consider this low prior as I have seen a lot of projects who have the same problem, so apparently, it is not only not standard but very poorly supported.

JacoKarsten-Korbicom commented 3 years ago

Hi @MStoykov

We use Azure AD B2C for authentication which uses cookie names with a colon in them. See Azure AD B2C Cookie Definitions. This means I can't run performance tests against any feature on our website requiring a logged in user. This is a major bummer since I really started to love using K6 for protocol level performance testing.

Would you be able advance this up the priority list?

mstoykov commented 3 years ago

Have you tried replacing the : with %3A ?

JacoKarsten-Korbicom commented 3 years ago

We cannot control the name of the cookie as it is predefined by Azure AD B2C. Even if I were able to change the name of the cookie somehow, that would break B2C, as that is what the internal mechanisms of Azure AD B2C rely on when looking for the cookie.

I don't access the cookie by name from my script, I am relying on K6 to propagate the cookie on subsequent requests. However, when I debug and look at all cookies available in the cookie jar, all cookies are present accept the one with the colon in the name.

If I switch on --http-debug, I can see the cookie coming back on the first call, but the cookie is not present from my export default function().

This is what my code looks like currently

     group('Login', () =>
    {
        let b2cSettings = this.RequestAccess();
        if (b2cSettings.success)
        {
            console.info('RequestAccess: Success');
            let submitCredentialsResult = this.SubmitCredentials(b2cSettings.content!, username, password);
            if (submitCredentialsResult.success)
            {
                console.info('SubmitCredentials: Success');
                let selfAssertResult = this.Confirm(b2cSettings.content!);
                if (selfAssertResult.success)
                {
                    console.info('Confirm: Success');
                    let retreiveAuthTokenResult = this.RetreiveAuthToken(b2cSettings.content!);
                    if (retreiveAuthTokenResult.success)
                    {
                        console.info('RetreiveAuthToken: Success');
                    }
                    else
                    {
                        console.error('ERROR => ' + retreiveAuthTokenResult.error);
                    }
                }
                else
                {
                    console.error(selfAssertResult.error);
                }
            }
            else
            {
                console.error(submitCredentialsResult.error);
            }
        }
        else
        {
            console.error(b2cSettings.error);
        }
    });

    sleep(1);
JacoKarsten-Korbicom commented 3 years ago

This is what I get back in the response by using the --http-debug flag when running:

group="::Login" iter=0 name=Login_RequestAccess request_id=a7974197-1e57-44a5-702a-aedfdd2abef5 scenario=default source=http-debug vu=1 INFO[0005] Response: HTTP/1.1 200 OK7 Content-Length: 16972- Allow: OPTIONS,TRACE,GET,HEAD,POST Cache-Control: no-store, must-revalidate, no-cache Content-Encoding: gzip Content-Type: text/html; charset=utf-8 Date: Thu, 18 Feb 2021 16:21:24 GMT Expires: -1 Public: OPTIONS,TRACE,GET,HEAD,POST Set-Cookie: x-ms-cpim-sso:xchangedocsdev.onmicrosoft.com_0=m1.VKws5CBVuLOC+ETX.46S00j2fvvGpdgcn6yBlLg==.0.vB1mkFOMVv+ZtIkM9PKaPAtcF5O5EtpV3p6yXY0ZCStj2rbPlxIwD3KX14UjLsJGidhjkSqT5bDbUk5I9GJggsk+T7PsDBMTUIVMdPhJgse18iNgSTuLoga9EClyqt5VK3SIoDUKvWrVfqQRyaWqM/jOFCa28L1f4Po5f2KgSuKRG/MBAaz1oAU2YbRbW7Whp0VqTdmVdGA7XhRcVv/+KwcAFdlfbvDvc9oAXXLTjREqRiLpa2IyetX3WCFfDwruP5poJBrPejGSkPXwuAIA3y9IGwFx0ufGs25A8ilK3TI=; domain=xchangedocsdev.b2clogin.com; path=/; SameSite=None; secure; HttpOnly
Set-Cookie: x-ms-cpim-csrf=MkZsaUd3SkowUGJ5TFVRZVpic3paTkl2UjJTanNTVjhiTkgxdVRielF0QUdxak9NU04rVHZSWmU4REQzdUVpRGRqS3pTOStlZmc0V3J5VEtuN0tTOWc9PTsyMDIxLTAyLTE4VDE2OjIxOjI0LjI0MTk1MTdaO3ViallkQWFCR2dMZCs1b3JYaXlmZmc9PTt7Ik9yY2hlc3RyYXRpb25TdGVwIjoyfQ==; domain=xchangedocsdev.b2clogin.com; path=/; SameSite=None; secure; HttpOnly Set-Cookie: x-ms-cpim-cache|uzr_94lw0kc9b7niavtuqg_0=m1.1gihidD0ifnvLU2n.7znIGGgSxCTGIP0ZcqD9NA==.0.ayPj+GhJFduZuYKbqYX5ELiPQ/UbhS4+vxWkDcXE3tsU2RqPL26EOCsxrwN8sadJrYca5SpI/FfgidddtdglFQ77nMrnipdWMCpQibNagck5rsktHmEuhGYlgcGPte8iWrT2yQTllSqSPbpdR3NC90Qc/m5kz9mkemqt7Yc12C6m/8DW+G8i/cx6w+gzzi/JqCACiQbkANQcS3xd4u1+sgN46DIGqAkahlPftuWX+7umLT9vkbYaP87odpGjQeSGEWV58PFEia5/7zIAG9109sWjv7l17NWwnmW2NonOvfK4KaKUeOlceCavRGCE4fFMSmrxxLahQZYPKaDcNscrS7ABPqFZpPajo1lzqoIv9rdreht7y1ZaBjx2s6o0qS3PoS13gNeXJnzhm64WxKZKggJ25ts/2lnwlMxW3bFCqssojMH3IgAkwOPmafVznkmjSub9rZs2XTe+MU5P4KDDQeUBmvKszZPqH16t58kSEu4zec4PKjsnf+wWNDPg0BgxqEJm6oV8KgAD1Fkg0YZFKrx3j/gEUN2afdqbmBWgVopQmZRcakysHtwqO+NR/ynHKDuNxM0Ruxo0AnO5tq5+p3fF3fRtOAsZNN8koqBfKegnS4+Y2yNGStnL; domain=xchangedocsdev.b2clogin.com; path=/; SameSite=None; secure; HttpOnly Set-Cookie: x-ms-cpim-trans=eyJUX0RJQyI6W3siSSI6ImY3N2Y5NDUxLTU2ODktNDBkMi1iZDZmLWI5Yzg2YWY0ZDQ0MiIsIlQiOiJ4Y2hhbmdlZG9jc2Rldi5vbm1pY3Jvc29mdC5jb20iLCJQIjoiYjJjXzFhX3NpZ251cHNpZ25pbiIsIkMiOiI4NTQ0YzM5OS1lNDZmLTQ2N2QtYTU1Mi05MGY0OTY2ODk2MGEiLCJTIjoxLCJNIjp7fSwiRCI6MH1dLCJDX0lEIjoiZjc3Zjk0NTEtNTY4OS00MGQyLWJkNmYtYjljODZhZjRkNDQyIn0=; domain=xchangedocsdev.b2clogin.com; path=/; SameSite=None; secure; HttpOnly Strict-Transport-Security: max-age=31536000; includeSubDomains Vary: Accept-Encoding X-Build: 1.0.1910.1 X-Content-Type-Options: nosniff X-Frame-Options: DENY X-Ms-Gateway-Requestid: 2f143361-6eb7-4711-8869-2b2f486de663 X-Request-Id: f77f9451-5689-40d2-bd6f-b9c86af4d442 X-Ua-Compatible: IE=edge X-Xss-Protection: 1; mode=block

However, this is the contents of the cookie jar: [Cookie Jar]{"x-ms-cpim-trans":["eyJUX0RJQyI6W3siSSI6ImY3N2Y5NDUxLTU2ODktNDBkMi1iZDZmLWI5Yzg2YWY0ZDQ0MiIsIlQiOiJ4Y2hhbmdlZG9jc2Rldi5vbm1pY3Jvc29mdC5jb20iLCJQIjoiYjJjXzFhX3NpZ251cHNpZ25pbiIsIkMiOiI4NTQ0YzM5OS1lNDZmLTQ2N2QtYTU1Mi05MGY0OTY2ODk2MGEiLCJTIjoxLCJNIjp7fSwiRCI6MH1dLCJDX0lEIjoiZjc3Zjk0NTEtNTY4OS00MGQyLWJkNmYtYjljODZhZjRkNDQyIn0="],"x-ms-cpim-csrf":["MkZsaUd3SkowUGJ5TFVRZVpic3paTkl2UjJTanNTVjhiTkgxdVRielF0QUdxak9NU04rVHZSWmU4REQzdUVpRGRqS3pTOStlZmc0V3J5VEtuN0tTOWc9PTsyMDIxLTAyLTE4VDE2OjIxOjI0LjI0MTk1MTdaO3ViallkQWFCR2dMZCs1b3JYaXlmZmc9PTt7Ik9yY2hlc3RyYXRpb25TdGVwIjoyfQ=="],"x-ms-cpim-cache|uzr_94lw0kc9b7niavtuqg_0":["m1.1gihidD0ifnvLU2n.7znIGGgSxCTGIP0ZcqD9NA==.0.ayPj+GhJFduZuYKbqYX5ELiPQ/UbhS4+vxWkDcXE3tsU2RqPL26EOCsxrwN8sadJrYca5SpI/FfgidddtdglFQ77nMrnipdWMCpQibNagck5rsktHmEuhGYlgcGPte8iWrT2yQTllSqSPbpdR3NC90Qc/m5kz9mkemqt7Yc12C6m/8DW+G8i/cx6w+gzzi/JqCACiQbkANQcS3xd4u1+sgN46DIGqAkahlPftuWX+7umLT9vkbYaP87odpGjQeSGEWV58PFEia5/7zIAG9109sWjv7l17NWwnmW2NonOvfK4KaKUeOlceCavRGCE4fFMSmrxxLahQZYPKaDcNscrS7ABPqFZpPajo1lzqoIv9rdreht7y1ZaBjx2s6o0qS3PoS13gNeXJnzhm64WxKZKggJ25ts/2lnwlMxW3bFCqssojMH3IgAkwOPmafVznkmjSub9rZs2XTe+MU5P4KDDQeUBmvKszZPqH16t58kSEu4zec4PKjsnf+wWNDPg0BgxqEJm6oV8KgAD1Fkg0YZFKrx3j/gEUN2afdqbmBWgVopQmZRcakysHtwqO+NR/ynHKDuNxM0Ruxo0AnO5tq5+p3fF3fRtOAsZNN8koqBfKegnS4+Y2yNGStnL"]}

mstoykov commented 3 years ago

I find this unlikely to get high enough priority so the core team gets to work on it. It also seems it has the potential to break the handling of cookies, given that the fix seems to be redoing the whole handling of cookies more or less.

We currently mostly use the golang stdlib to handle most of the cookie/headers and starting to handle them ourselves is a big step. One that I don't feel is warranted at this time. So the best case is that https://github.com/golang/go/issues/12698 gets reopened and fixed.

Even if someone decides to make a PR to fix it(which likely will be a lot of work, unless it turns out there is an easy hack) this will have to wait for at least v0.32.0(in 2months+) or 6+ months for the next golang release ;).

I agree that this is not ideal, but the specification is clear, and the fact that Microsoft is (again) not following it and even specifically breaking it is ... reminiscent of something ;)

As a workaround sending the header Cookie seems to work in k6:

import http from "k6/http";

export default function() {
    var res = http.get("https://httpbin.org/get", {headers: {"Cookie": "on:e=two"}})
    console.log(JSON.stringify(res, null, "  "))
}

Obviously, in your case you will also need to parse the Set-Cookie header when it comes in. This can probably be done fairly easy with some amount of wrappers and would expect will be fairly non-intrusive as long as that Cookie isn't needed everywhere ... but I haven't tried it so YMMV :man_shrugging:

JacoKarsten-Korbicom commented 3 years ago

Thanks @MStoykov , I'll capture the cookie from the header and resubmit on subsequent requests. I suspect it is only needed till I receive the authorization code.

Thanks for the quick assistance.

JacoKarsten-Korbicom commented 3 years ago

Unfortunately this workaround didn't work for me. I tried setting the cookie like this: 

        let response = http.post<'text'>(url, data,
            {
                headers: { 'Cookie': `x-ms-cpim-sso:xchangedocsdev.onmicrosoft.com_0=${b2cSettings.ssoCookie}`},
                tags: { name: 'Login_SubmitCredentials' }
            });

and like this:

        let response = http.post<'text'>(url, data,
            {
                cookies: { 'x-ms-cpim-sso:xchangedocsdev.onmicrosoft.com_0': b2cSettings.ssoCookie },
                headers: headers,
                tags: { name: 'Login_SubmitCredentials' }
            });

Even when I use your example of "on:e=two" the cookie is not sent with the request. As soon as I drop the ":" or replace it with a "|", the cookie goes through. But then B2C doesn't recognize the cookie.

I get this B2C error: User does not have an existing session and request prompt parameter has a value of 'None'.

mstoykov commented 3 years ago

Running against httpbin seems to work ... I just double-checked.

How do you find out the cookie doesn't get to the server? Do you happen to have a firewall or some kind of corporate gateway/proxy in the middle, and does it work with any other tool for example curl?

JacoKarsten-Korbicom commented 3 years ago

I look at the request's cookie after submitting the call:

        let response = http.get<'text'>(url,
            {
                cookies: { 'x-ms-cpim-sso_xchangedocsdev.onmicrosoft.com_0': b2cSettings.ssoCookie },
                headers: headers,
                redirects: 0,
                tags: { name: 'Login_Confirm' }
            });

        check(response, { 'status was 302': r => r.status === 302 });

        Object.keys(response.request.cookies).forEach(key => {
            console.log(`[KEY]${key}`);
          });
mstoykov commented 3 years ago

they won't be there, as golang doesn't like the fact that there is : in them(this whole issue). But the header is sent from at least what I can gather as that doesn't get processed(that much).

That's why using httpbin.org as I did above is so good as it will return you what headers you sent in the body so you can check that

JacoKarsten-Korbicom commented 3 years ago

Looks like you are right. Here is what I get from the response:

  "request": {
      "method": "GET",
      "url": "https://httpbin.org/get",
      "headers": {
        "Accept": [
          "*/*"
        ],
        "Accept-Encoding": [
          "gzip, deflate, br"
        ],
        "Connection": [
          "keep-alive"
        ],
        "Host": [
          "xchangedocsdev.b2clogin.com"
        ],
        "Referer": [
          "https://xchangedocsdev.b2clogin.com/xchangedocsdev.onmicrosoft.com/b2c_1a_signupsignin/oauth2/v2.0/authorize?response_type=id_token&scope=[REDACTED]%20openid%20profile&client_id=[REDACTED]&redirect_uri=https%3A%2F%2Fxd-dev-webapp-6ami23sdirtd2.azurewebsites.net%2F&prompt=login&response_mode=fragment"
        ],
        "Cookie": [
          "x-ms-cpim-sso:xchangedocsdev.onmicrosoft.com_0=m1[REDACTED]="
        ],
        "User-Agent": [
          "k6/0.30.0 (https://k6.io/)"
        ]
      },
      "body": "",
      "cookies":
mstoykov commented 3 years ago

@JacoKarsten-Korbicom seems fine to me so that seems to work :shrug:

Also, I hope these credentials that you are posting are testing ones and you revoke them before posting? As otherwise, someone can now use those to gain access ;)

I don't know what User does not have an existing session and request prompt parameter has a value of 'None'. could mean ... but I would argue that you should try using curl to replicate a request that passes and then try to make it work in k6.

Maybe something else is missing from the request? or it is expecting a specific User-Agent ? no idea

JacoKarsten-Korbicom commented 3 years ago

Yep, Just test accounts.

Thanks for your help. Must be something else that cause B2C to return the error then.

JacoKarsten-Korbicom commented 3 years ago

Hey @MStoykov

I have found my problem. I was capturing the sso cookie on the first call thinking that I can just pass it to all 3 subsequent calls. I had to discard this sso cookie in favour of a new sso cookie I get back on the third call (function Confirm) to submit with the last (4th) call to get the authorization code back.

I'm so glad I got this working. Your advice helped a lot!

leandrodotec commented 1 year ago

In case anyone is looking for a quick solution, this is the function to extract the cookie. 

import http from "k6/http";

export default function() {
    var resWithCookies = http.get(url_with_bad_cookies)
    var badCookies = parseBadCookie(resWithCookies.headers);

    var res = http.get(url_that_now_receives_cookies_correctly, {headers: {"Cookie": badCookies}})
    console.log(JSON.stringify(res, null, "  "))
}

export function parseBadCookie(setCookieHeader) {
  const cookieLines = setCookieHeader.split(",");
  // gests all cookies with colons in the name
  const cookiesWithColons = cookieLines.filter((cookie) => {
    const cookieName = cookie.split("=");
    return cookieName[0].indexOf(":") >= 0;
  });
  // returns comma separated string, ready to be added as a cookie header
  return cookiesWithColons.join(",");
}
leandrodotec commented 1 year ago

Btw, Azure AD still has cookies with colons in it, and K6 still ignores them.

leandrodotec commented 1 year ago

Hi again, this issue is quite an important one. I have been working with Jmeter for quite some time and just recently started migrating things to K6. My first big issue was this one, that took a lot of debugging and Fiddler to find out that 1 cookie had a ";" in the name and was ignored.

I understand that we have RFCs, but no modern browser today follows them strictly because technology grows fast and the RFCs don't follow the same pace. The cookie spec is from April 2011 and the spec that defines "token" is from 1999.

I tested this cookie in LoadRunner, JMeter and NeoLoad. All of them just work.

mstoykov commented 1 year ago

Hi @leandrodotec, thanks for writing in with such a nice example [^1] :bow:.

[^1]: it seems to me there is no typescript syntax in this, so it is 100% javascript just ... I think :shrug:

As mentioned in the links from the old comments this comes from the go stdlib directly. There reasoning for not supporting this is - "it is not RFC compliant and is not as commonly broken as ".

AFAIK and managed to google - nothing has changed about this. So the go stdlib will not make this any easier for us.

This does mean that at this point this will likely need to be fixed in multiple places multiple times or for us to fix this.

To be honest I am not a fan of the current cookiejar implementation which is direct copy of the go one.

https://github.com/grafana/k6/issues/3026 and https://github.com/grafana/k6/issues/1923 are two issues which make it confusing and hard to use already.

But the current k6/http is in maintaince mode until we manage to ship a new http API which should try to address all of these issues

It now has a design doc and is planned to be worked on.

I don't think this will be shipped with enough features to be usable for at least 2+ releases (2 months each) and this issue is not in the ones that should be fixed.

I will add it to a list to be discussed, but even if we do add it to the list this will still mean this will not ship soon. Sorry.

At this point my recommendation is to wrap the k6/http API and do the changes in that - hopefully they will work for you. Posting the wrapper will also be nice and likely useful for other users.

Hopefully this explains why this hasn't been fixed and makes it more clear on when it might be ... if at all.

leandrodotec commented 1 year ago

HI @mstoykov thanks for taking the time to answer me. And yeah, the first version of my example had a lot of typescript, I removed them and I forgot to remove my comment :-).