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

SNI FAQ page subtly incorrect and implementation confusing #4275

Closed
jeteon opened this issue Aug 28, 2018 · 3 comments
Closed

SNI FAQ page subtly incorrect and implementation confusing #4275

jeteon opened this issue Aug 28, 2018 · 3 comments

Comments

@jeteon
Copy link

jeteon commented Aug 28, 2018

Title: The FAQ page that documents SNI configuration is slightly incorrect and misleading

Description:

The SNI configuration example shown in the FAQ section is incorrect because it shows supplying a string value instead of a list of strings for the server_names parameter. This took some time for me to debug since:

  • the server itself accepts this configuration without issue (though it won't work so I guess it does something else with it)
  • server_names is a strange label for a single string but I thought maybe that's just how the reference example does it and it seems to be implied that you'd duplicate the entire block containing it for SNI so I thought it might be an implementation detail that formed from that.
  • it's my first rodeo (okay, it might be my 3rd)

The particular example is also confusing since according to the documentation on the server_names parameter both "www.example.com" and "example.com" would have been matched by the "wildcard" matching that is supposedly done for "www.example.com" so the second block would therefore never do anything. Turbine Labs, for instance, propagates this confusion in an article of theirs so it seems not to be something that comes from lack of familiarity with the product. Would you consider us making the second example something like "api.example.com" instead? Then a note could be added about the wildcard matching feature taking care of parent domains.

@cmluciano
Copy link
Member

cc @PiotrSikora

PiotrSikora added a commit to PiotrSikora/envoy that referenced this issue Aug 28, 2018
Fixes envoyproxy#4275.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
@PiotrSikora
Copy link
Contributor

The SNI configuration example shown in the FAQ section is incorrect because it shows supplying a string value instead of a list of strings for the server_names parameter.

It's correct, server_names: value is the same as server_names: [value] when dealing with single-element lists in YAML.

the server itself accepts this configuration without issue (though it won't work so I guess it does something else with it)

It works just fine (it's missing a few required fields in HTTP connection manager, so the config doesn't load as-is, but that's unrelated).

The particular example is also confusing since according to the documentation on the server_names parameter both "www.example.com" and "example.com" would have been matched by the "wildcard" matching that is supposedly done for "www.example.com" so the second block would therefore never do anything.

There are no wildcards in the SNI example.

Turbine Labs, for instance, propagates this confusion in an article of theirs so it seems not to be something that comes from lack of familiarity with the product.

There is no mention of server_names in that article, and I believe it predates the filter chain matching.

Would you consider us making the second example something like "api.example.com" instead? Then a note could be added about the wildcard matching feature taking care of parent domains.

Done, see #4285.

@jeteon
Copy link
Author

jeteon commented Aug 29, 2018

It's correct, server_names: value is the same as server_names: [value] when dealing with single-element lists in YAML.

Ah... I see. I mistakenly translated server_names: "domain" in YAML to { ..., "server_names" : "domain", ... } in JSON for an XDS server. Thanks for clearing that up. I'm not sure what it means that the resulting JSON was accepted though. I'll have a look through the code to see what happens in that case for my own curiosity some time.

There is no mention of server_names in that article, and I believe it predates the filter chain matching.

The confusion I was referring to was with regard to the wildcard matching that I think always happens according to this:

// If non-empty, a list of server names (e.g. SNI for TLS protocol) to consider when determining
// a filter chain match. Those values will be compared against the server names of a new
// connection, when detected by one of the listener filters.
//
// The server name will be matched against all wildcard domains, i.e. ``www.example.com``
// will be first matched against ``www.example.com``, then ``*.example.com``, then ``*.com``.

The way I read that suggested to me that "example.com" will be a match in the same filter chains as "www.example.com" even without explicitly specifying it. However, I see now that what was meant is that if you had used a wildcard like "*.example.com" then both "www.example.com" and "example.com" would match, which makes more sense.

Thanks for taking the time to address this. I've learned a lot from this even if it wasn't the greatest use of your time.

@jeteon jeteon closed this as completed Aug 29, 2018
ggreenway pushed a commit that referenced this issue Aug 29, 2018
Fixes #4275.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants