https.createServer accepts any non-falsey values for key and cert

[ramblinjan]

• Version: v7.4.0
• Platform: Darwin Kernel Version 16.5.0: Fri Mar 3 16:52:33 PST 2017; root:xnu-3789.51.2~3/RELEASE_X86_64 x86_64
• Subsystem: https

When using https.createServer, passing a boolean to cert and/or key will not cause an error, but will result in an invalid certificate at runtime. For instance:

require('https').createServer({
  key: true,
  cert: true
// ...

I discovered this with a pretty silly code mistake like:

const key = fs.existsSync('path/to/production/tls.key') || fs.readFileSync('path/to/fallback/tls.key);
const cert = fs.existsSync('path/to/production/tls.cert') || fs.readFileSync('path/to/fallback/tls.cert);

It was certainly user error. However, in the types for the options object, the valid types are listed as:

• key | <string[]> | | <Buffer[]> | <Object[]>
• cert | <string[]> | | <Buffer[]>

Then in https.js, there is no typechecking for creating the server, and for outbound requests, non-falsey value for gets concatenated (and therefore cast to a string):

  name += ':';
  if (options.cert)
    name += options.cert;

// ...

  name += ':';
  if (options.key)
    name += options.key;

For invalid types, there could probably be an error thrown rather than issuing a completely invalid certificate. Though it would be overkill to validate certs at runtime rather than time of connection, checking for valid types might be a good idea since there's currently (as far as I can tell) no difference between this mistake and a totally invalid certificate, making it hard to debug. Instead a non-string, non-object (for key), non-buffer should probably be functionally identical to having no certificate.

---

For SEO purposes, this mistake will likely result in the following error with an in-browser response:
ERR_SSL_VERSION_OR_CIPHER_MISMATCH

[evanlucas]

Possible duplicate of #11978?

[ramblinjan]

@evanlucas I'd have to repro it with a different setup to be sure, but I think for null values the browser will report that the certificate is simply missing and declare the site insecure. In this case, the site won't even load because the handshake completely fails and has a ERR_SSL_VERSION_OR_CIPHER_MISMATCH error, which isn't even all that Google-able because that's usually related to using outdated SSL or a client issue. In this specific case, the cert gets treated as "valid," but wrong rather than missing. Point of failure, result, and debug path is totally different from similar issues.

[Trott]

@nodejs/http

[bnoordhuis]

Rejecting definitely wrong values is a good idea. I've added the good-first-contribution label.