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.

Sign up for GitHub

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

Jump to bottom

Fixes #6034 - SslContextFactory may select a wildcard certificate dur… #6036

Merged
merged 1 commit into from
Mar 8, 2021
Merged

Fixes #6034 - SslContextFactory may select a wildcard certificate dur… #6036

merged 1 commit into from
Mar 8, 2021

Conversation

sbordet
Copy link
Contributor

@sbordet sbordet commented Mar 4, 2021

…ing SNI selection when a more specific SSL certificate is present.

Now matching certificates are sorted, non-wildcard first, so that a more specific alias is returned.

Signed-off-by: Simone Bordet simone.bordet@gmail.com

@sbordet
Fixes #6034 - SslContextFactory may select a wildcard certificate dur…
7aa69c0 
…ing SNI selection when a more specific SSL certificate is present.

Now matching certificates are sorted, non-wildcard first, so that a more specific alias is returned.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet sbordet requested a review from gregw March 4, 2021 12:09
@sbordet sbordet mentioned this pull request Mar 4, 2021
Closed
@joakime
Copy link
Contributor

joakime commented Mar 4, 2021

Can there be a situation with something like?

*.domain.com
foo.domain.com
*.bar.domain.com
zedoo.domain.com
a.bar.domain.com

If we only sort by size/length of the name, wouldn't this list result in a bad sort?

I only ask as this seems similar (imagine the . is equiv to / and the segments are in reverse and you'll see) to the request matching to url-patterns logic (which is quite complex), and I wonder about the sub-sub-domain behavior.

@sbordet
Copy link
Contributor Author

sbordet commented Mar 4, 2021

@joakime we don't sort by size/length of the name.

Edge cases are still possible, e.g. a certificate with both foo.domain.com and *.domain.com and another certificate with bar.domain.com and *.domain.com -- for x.domain.com we would return (randomly) one of the two, but I think it's acceptable and we can't do better.

At that point, they can write their own SNISelector, I guess.

joakime
joakime approved these changes Mar 5, 2021
View reviewed changes
Copy link
Contributor

@joakime joakime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sbordet sbordet merged commit 8de7b83 into jetty-10.0.x Mar 8, 2021
@sbordet sbordet linked an issue Mar 8, 2021 that may be closed by this pull request
SslContextFactory may select a wildcard certificate during SNI selection when a more specific SSL certificate is present #6034
Closed
@sbordet sbordet deleted the jetty-10.0.x-6034-sni_strict_match branch March 9, 2021 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gregw gregw gregw approved these changes

@joakime joakime joakime approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@sbordet @joakime @gregw