Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

TLS doesn't check subject.CN for wildcard #4255

Closed
wants to merge 2 commits into from
Closed

TLS doesn't check subject.CN for wildcard #4255

wants to merge 2 commits into from

Conversation

NodePing
Copy link

@NodePing NodePing commented Nov 8, 2012

Allow CN to be checked for wildcard

#4254

Allow CN to be checked for wildcard
@isaacs
Copy link

isaacs commented Nov 9, 2012

@bnoordhuis @indutny Thoughts?

@isaacs
Copy link

isaacs commented Nov 9, 2012

@NodePing It seems fine to me, but please add a test.

@indutny
Copy link
Member

indutny commented Nov 9, 2012

I think it cannot contain wildcard according to specification... Is it so widely used?

@NodePing
Copy link
Author

NodePing commented Nov 9, 2012

We're seeing it on more than a few DigiCert and Thawte certificates but I don't have any stats for how widely it is used.
Two examples from some of our customers are:
graph.facebook.com
shopping.framesdirect.com
Is there a downside to checking for wildcards in the CN? I can't think of one.

@indutny
Copy link
Member

indutny commented Nov 9, 2012

Well, it's not really a good place for them to be, since it's deprecated... ok, I give up. It's not that important after all 🔨

@NodePing
Copy link
Author

NodePing commented Nov 9, 2012

I'm having a hard time trying to come up with a good way to test these goofy certs. Any ideas?

@indutny
Copy link
Member

indutny commented Nov 9, 2012

No, I mean. Lets pull your patch @isaacs

@indutny
Copy link
Member

indutny commented Nov 16, 2012

Ok, @isaacs seems to be pretty busy right now... Probably @bnoordhuis or @piscisaureus can review it? Or @pquerna ?

@bnoordhuis
Copy link
Member

I think it cannot contain wildcard according to specification... Is it so widely used?

I don't know if I'd call it 'widely used' but RFC 2818 certainly allows for it.

@NodePing I'll land your patch but a test case would be nice.

You can generate a self-signed certificate with openssl req (google for 'openssl create self-signed certificate' and you'll find more info.)

Drop the key and the certificate in test/fixtures and the test itself in test/simple.

@NodePing
Copy link
Author

I've updated the existing test for wildcards in CN to assert true, rather than false.

@bnoordhuis
Copy link
Member

@NodePing You're too late, @indutny already fixed it in 4dd70bb and b4b750b. Thanks though.

@bnoordhuis bnoordhuis closed this Jan 19, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants