Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
4 participants

NodePing commented Nov 8, 2012

Allow CN to be checked for wildcard

joyent#4254

@NodePing NodePing Update lib/tls.js
Allow CN to be checked for wildcard
1a8f9fc

isaacs commented Nov 9, 2012

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

Owner

indutny commented Nov 9, 2012

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

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.

Owner

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 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?

Owner

indutny commented Nov 9, 2012

No, I mean. Lets pull your patch @isaacs

Owner

indutny commented Nov 16, 2012

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

Owner

bnoordhuis commented Nov 27, 2012

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.

@CrabBot CrabBot referenced this pull request in node-apn/node-apn Jan 8, 2013

Closed

Hostname/IP doesn't match certificate's altnames #74

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

Owner

bnoordhuis commented Jan 19, 2013

@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 join this conversation on GitHub. Already have an account? Sign in to comment