Re: [PATCH] BUG/MINOR: config: Warn cookie domain only if missing embedded dot

2019-10-31 Thread Willy Tarreau
On Wed, Oct 30, 2019 at 09:18:07PM -0300, Joao Morais wrote:
> Yup, attached a new patch. Note however I only know two or three words in
> english beyond "hello" and "good morning", so feel free to update subject to
> a proper one and comment to what is really happening and what the spec is
> really forbiding or not. No need to be reviewed. =)

OK thank you. I've adjusted the patch to:
  - simply remove the test on *argv[arg+1] since it's covered by
strchr() and more closely matches the commit message description
  - adjusted the comment to mention RFC6265 instead
  - adjusted the warning message

I figured I didn't have to reword your message since you apparently made
use of a few extra words beyond "hello" and "good morning" that were
perfectly understandable :-)

Cheers,
Willy



Re: [PATCH] BUG/MINOR: config: Warn cookie domain only if missing embedded dot

2019-10-30 Thread Joao Morais

Hi Willy,

> Em 30 de out de 2019, à(s) 01:41, Willy Tarreau  escreveu:
> 
> Hi Joao,
> 
> On Tue, Oct 29, 2019 at 09:10:11PM -0300, Joao Morais wrote:
>> 
>> What I need to implement is a way to share the sticky session cookie between
>> two distinct but related domains, say haproxy.org and haproxy.com, without
>> its subdomains. I couldn't do that in a way that worked and without a
>> warning.
> 
> Interesting, that's probably what forced browsers to ignore the leading
> dot when in the early 2000s we started to see host-less site names. Then
> I think we can indeed include your patch but then we also need to change
> the comment mentioning RFC2109 and update it to 6265.

Yup, attached a new patch. Note however I only know two or three words in 
english beyond “hello” and “good morning”, so feel free to update subject to a 
proper one and comment to what is really happening and what the spec is really 
forbiding or not. No need to be reviewed. =)

Thanks!

~jm





0001-BUG-MINOR-config-Update-cookie-domain-warn-to-RFC626.patch
Description: Binary data


Re: [PATCH] BUG/MINOR: config: Warn cookie domain only if missing embedded dot

2019-10-29 Thread Willy Tarreau
Hi Joao,

On Tue, Oct 29, 2019 at 09:10:11PM -0300, Joao Morais wrote:
> 
> Hi Willy,
> 
> > Em 29 de out de 2019, à(s) 04:27, Willy Tarreau  escreveu:
> > 
> > No, please look at the RFC again, it's very precise on this :
> > https://tools.ietf.org/html/rfc2109
> 
> Thanks for taking the time to review my patch.
> 
> In fact I read RFC 6265 which doesn't take the leading dot as mandatory. 6265
> obsoletes 2965, which obsoletes 2109.

You're right, it's interesting to see that this part changed in 6265.
Apparently this happened first in 2965 with set-cookie2 that nobody
except Opera and haproxy implemented, then the rule was relaxed in 6265
when trying to merge both specs and document what was really deployed in
browsers.

> What I need to implement is a way to share the sticky session cookie between
> two distinct but related domains, say haproxy.org and haproxy.com, without
> its subdomains. I couldn't do that in a way that worked and without a
> warning.

Interesting, that's probably what forced browsers to ignore the leading
dot when in the early 2000s we started to see host-less site names. Then
I think we can indeed include your patch but then we also need to change
the comment mentioning RFC2109 and update it to 6265.

Willy



Re: [PATCH] BUG/MINOR: config: Warn cookie domain only if missing embedded dot

2019-10-29 Thread Joao Morais


Hi Willy,

> Em 29 de out de 2019, à(s) 04:27, Willy Tarreau  escreveu:
> 
> No, please look at the RFC again, it's very precise on this :
> https://tools.ietf.org/html/rfc2109

Thanks for taking the time to review my patch.

In fact I read RFC 6265 which doesn’t take the leading dot as mandatory. 6265 
obsoletes 2965, which obsoletes 2109.

What I need to implement is a way to share the sticky session cookie between 
two distinct but related domains, say haproxy.org and haproxy.com, without its 
subdomains. I couldn’t do that in a way that worked and without a warning.

~jm




Re: [PATCH] BUG/MINOR: config: Warn cookie domain only if missing embedded dot

2019-10-29 Thread Willy Tarreau
Hi Joao,

On Sun, Oct 27, 2019 at 03:51:18PM -0300, Joao Morais wrote:
> 
> Hi list, the attached patch fixes a warn message if the domain option, from
> cookie keyword, configures a domain without starting with a dot.
> 
> 
> From ee0d7f60d4b0c31e779b639c39e0226cbc920e69 Mon Sep 17 00:00:00 2001
> From: Joao Morais 
> Date: Sun, 27 Oct 2019 15:04:15 -0300
> Subject: [PATCH] BUG/MINOR: config: Warn cookie domain only if missing
>  embedded dot
> 
> The domain option of the cookie keyword allows to define which domain or 
> domains
> should use the the cookie value of a cookie-based server affinity. If the 
> domain
> does not start with a dot, the user agent should only use the cookie on hosts
> that matches the provided domains. If the configured domain starts with a dot,
> the user agent can use the cookie with any host ending with the configured
> domain.
> 
> haproxy config parser helps the admin warning about a potentially buggy 
> config:
> defining a domain without an embedded dot which does not start with a dot, 
> which
> is forbidden by the RFC. However the condition was warning valid domains as
> well.

No, please look at the RFC again, it's very precise on this :
https://tools.ietf.org/html/rfc2109

  | 4.3.2  Rejecting Cookies
  | 
  |To prevent possible security or privacy violations, a user agent
  |rejects a cookie (shall not store its information) if any of the
  |following is true:
  | 
  |* The value for the Path attribute is not a prefix of the request-
  |  URI.
  | 
  |* The value for the Domain attribute contains no embedded dots or
  |  does not start with a dot.

See the "OR" above ? Hence:
  - a domain with no embedded dots is invalid
  - a domain that does not start with a dot is invalid

This is further explained in the last example with "Domain=ajax.com" :

  | 
  |* The value for the request-host does not domain-match the Domain
  |  attribute.
  | 
  |* The request-host is a FQDN (not IP address) and has the form HD,
  |  where D is the value of the Domain attribute, and H is a string
  |  that contains one or more dots.
  | 
  |Examples:
  | 
  |* A Set-Cookie from request-host y.x.foo.com for Domain=.foo.com
  |  would be rejected, because H is y.x and contains a dot.
  | 
  |* A Set-Cookie from request-host x.foo.com for Domain=.foo.com would
  |  be accepted.
  | 
  |* A Set-Cookie with Domain=.com or Domain=.com., will always be
  |  rejected, because there is no embedded dot.
  | 
  |* A Set-Cookie with Domain=ajax.com will be rejected because the
  |  value for Domain does not begin with a dot.

So I could say that we apply exactly what the spec mandates. I know that
browsers tend to be more permissive than the spec to accomodate for old
applications (and for broken ones as well) but as new attacks appear,
they can quickly switch to a stricter mode and break some applications.
Thus the warning should remain to inform the user that there is a risk
that sooner or later a browser will reject such a cookie.

Cheers,
Willy