zero-to-jupyterhub-k8s icon indicating copy to clipboard operation
zero-to-jupyterhub-k8s copied to clipboard

Ensure secure attribute on cookies in various TLS termination setups

Open consideRatio opened this issue 3 years ago • 4 comments

  • [x] I'd like to verify if the secure attribute is set on our cookies when using proxy.https.type=letsencrypt.
  • [ ] I'd like to understand and document what to consider when using proxy.https.type=offload or proxy.https.enabled=false

It is dictated by how our deployment influence this segment of code in JupyterHub:

    def _set_cookie(self, key, value, encrypted=True, **overrides):
        """Setting any cookie should go through here
        if encrypted use tornado's set_secure_cookie,
        otherwise set plaintext cookies.
        """
        # tornado <4.2 have a bug that consider secure==True as soon as
        # 'secure' kwarg is passed to set_secure_cookie
        kwargs = {'httponly': True}
        if self.request.protocol == 'https':
            kwargs['secure'] = True
        if self.subdomain_host:
            kwargs['domain'] = self.domain

        kwargs.update(self.settings.get('cookie_options', {}))
        kwargs.update(overrides)

Background

This question is raised because @dtaniwaki observed that when offloading TLS termination to a AWS classic load balancer cookies were not set with the secure attribute.

Related conversation from Gitter

@consideratio wrote

What are your thoughts about the jupyterhub-hub-login cookie and the jupyterhub-session-id cookie?

Should either or both be a cookie only sent if HTTPS is used? Should it depend or not on things?

@manics wrote

Are you referring to the Secure attribute? https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#restrict_access_to_cookies

@minrk wrote

I think we should set secure by default if the current request is https. I believe that's what we do already, at least for login. Maybe not for session-id

Looks like we set it for both, here

@consideratio wrote

Ah @manics yepp I meant Secure attribute (send only as part of https:// requests), and I see we always set httponly (tell browsers that javascript access isn't allowed).

@minrk the logic seem to make sense, but how does self.request.protocol evaluate in various situations?

Z2JH configured with autohttps, and traefik terminates TLS. Will JupyterHub consider it to be a HTTPS request or HTTP request via self.request.protocol?

Z2JH configured with proxy.https.type=offload or in practice in a way that make z2jh not inform the hub in any way if it is responding to a mix of HTTP/HTTPS requests or only HTTPS requests.

My guess is this is what makes it so important with the additional headers describing if its a HTTPS or HTTP request originally?

@minrk wrote

It will usually be correct except in weird proxy cases. Those cases can override it explicitly with cookie_options config, applied here.

I don't know if our autohttps setup will do the right thing or not.

We could have our own config to set cookie_options["secure"] = True explicitly if any of our https config is detected. Or we could leave that to users.

consideRatio avatar Feb 25 '21 17:02 consideRatio

In my development environment I found the proxy.https.type=letsencrypt setup with our autohttps pod to do a perfect job.

  1. We were got redirected to https:// if visited using http:// by the autohttps pod running traefik.
  2. We got cookies set with the httponly and secure attribute.

consideRatio avatar Feb 25 '21 17:02 consideRatio

I figure the remaining need is to help those using their own TLS termination, with or without proxy.https.type=offload, should be informed that they should preferably arrange for a redirection from http to https, and for hub.config.JupyterHub.cookie_options = {"secure": true}.

consideRatio avatar Feb 25 '21 17:02 consideRatio

I suspect everyone's favourite header X-Forwarded-Proto and related forwarding headers may also have an effect :laughing:

manics avatar Feb 28 '21 14:02 manics