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

Adding an extra check for cert.subject before assuming it is there. #2556

Closed
wants to merge 1 commit into from

Conversation

mansona
Copy link

@mansona mansona commented Aug 26, 2015

One of my long running node services started giving out a TLS error without very much detail overnight:

 TypeError: Cannot read property 'CN' of undefined
     at Object.checkServerIdentity (tls.js:182:37)
     at TLSSocket.<anonymous> (_tls_wrap.js:1016:29)
     at emitNone (events.js:67:13)
     at TLSSocket.emit (events.js:166:7)
     at TLSSocket._finishInit (_tls_wrap.js:566:8)

I tracked it down to the line:

        reason = util.format('Host: %s is not cert\'s CN: %s',
                             host,
                             cert.subject.CN);

which assumes cert.subject is available.

I don't really know what is causing this and I can't tell which domain is causing this so I probably won't be able to recreate this locally to verify it's still working but I think it is at least sensible to use defensive programming in this instance.

I would ideally like to back port this to the 2.5.x branch but I couldn't find the head of that branch, can someone tell me which branch is currently the 2.5.x head?

@mscdex mscdex added the tls Issues and PRs related to the tls subsystem. label Aug 26, 2015
@mscdex
Copy link
Contributor

mscdex commented Aug 26, 2015

Duplicate of #2304 which was fixed by #2312.

@mscdex mscdex closed this Aug 26, 2015
@targos
Copy link
Member

targos commented Aug 26, 2015

@mansona the fix @mscdex is talking about is included since v3.1.0

@mansona
Copy link
Author

mansona commented Aug 27, 2015

Thanks for the update. @targos thanks for the update, that will be why I haven't been able to see it. I am currently stuck on v2.5.0 because of a few binary packages updating so I will just have to wait.

Thanks again 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants