Hi @mdconaway this is a great PR, thanks for all the effort you have put on it so far. Having SSL support natively (as an option) is a great addition to RTL. Although, there is an issue with self signed certificates for some browsers e.g. Chrome where the app will not work at all (I am not considering that a big deal though).

Nonetheless, we can merge the change as long as it is an optional configuration, which the user can enable and we keep it OFF by default. As we need to keep in mind that there are vendor integrations for RTL, where https access is enabled via reverse proxy setup and the default should always be ssl as OFF or we will end up breaking such integrations.

As far as the review of the PR is concerned, I would request to limit the changes to SSL only and roll back the rest e.g. the formatted server code and prettier formatter etc. This would enable us to do a thorough review of the relevant changes only.

Looking forward to review and work on this PR with you. Thanks.

Yeah, I can do that.

In this pull request the default value for SSL is definitely false. Regarding Chrome, I am currently able to use this RTL SSL feature branch with self signed certificates in the browser pretty much anywhere on my LAN, a user just has to click "proceed" through the warning messages. Ideally, users would generate their own CA and certs for their LAN/WAN and use the other config options for this enhancement like key, cert, and ca. (These options are absolute file paths to find the key files) Using actual key files also allows x509 client certificates to be requested to have a fully verified SSL tunnel.

This pull request also adds a deprecation warning on server boot if a user has not added a "host" value to their config. You guys can leave the warning as long as you'd like, and someday eventually change the host binding default when you feel sufficient time has passed. This is mainly a security related warning, but does not change any default configs or behavior. I added this deprecation warning because it occurs approximately where the ssl certs are loaded into the server.

While working on this pull I didn't see any linting script/config to check the server/backend code. I could easily include the addition of prettier as just a package.json update, then you guys can run npm run formatserver on your end in order to trust the server linting / formatting. Essentially the server formatter setup is just 2 package.json lines. The prettier changes are fully confined to package.json.

Just force pushed an update that matches the conversation above. Let me know if that is sufficient!

@saubyk The PR is blocked due to your decision on adding this support.

Native SSL/TLS: 👍
Generating key and self-signed cert: 👎

There is an issue on establishing initial trust with self-signed dynamic certs generated by the app like this.

It would still be nice to be able to supply cert+key, making the new config:

 "tls": {
    "key": "<Full path of a server TLS key file to enable TLS, default is null, Optional>",
    "cert": "<Full path of a server TLS pem format certificate bundle to enable TLS, default is null, Optional>"
  }

Note that CA here isn't really needed unless mTLS with client certificates is also implemented, which doesn't seem implemented here?

If that's the case, then:

 "tls": {
    "key": "<Full path of a server TLS key file to enable TLS, default is null, Optional>",
    "cert": "<Full path of a server TLS pem format certificate bundle to enable TLS, default is null, Optional>",
    "ca": <Full path of a TLS certificate authority file to enable TLS, default is null, Optional>
  }

Native SSL/TLS: 👍 Generating key and self-signed cert: 👎

There is an issue on establishing initial trust with self-signed dynamic certs generated by the app like this.

It would still be nice to be able to supply cert+key, making the new config:

 "tls": {
    "key": "<Full path of a server TLS key file to enable TLS, default is null, Optional>",
    "cert": "<Full path of a server TLS pem format certificate bundle to enable TLS, default is null, Optional>"
  }

Note that CA here isn't really needed unless mTLS with client certificates is also implemented, which doesn't seem implemented here?

If that's the case, then:

 "tls": {
    "key": "<Full path of a server TLS key file to enable TLS, default is null, Optional>",
    "cert": "<Full path of a server TLS pem format certificate bundle to enable TLS, default is null, Optional>",
    "ca": <Full path of a TLS certificate authority file to enable TLS, default is null, Optional>
  }

Per this file, inputs of key, cert and ca are all supported:

The self-signed certificate factory is generally not for production use, but rather a way to easily test that SSL functions work without supplying a genuine certificate. In production, you should always generate and supply legitimate certificate files!

This line also only runs the self-signed cert generator if absolutely no certificate files are supplied:

The self-signed certificate factory is generally not for production use, but rather a way to easily test that SSL functions work without supplying a genuine certificate.

This is not obvious to the user and a footgun. It accommodates and incentivizes insecure setups.

Since it is intended for test/dev, I think it can be solved by a separate shell script generating it using openssl in docs. This way, no need to pull in node-forge and add all the extra complexity to the production app.

If doing generation in JS is still desired, I suggest breaking it out to a separate entrypoint so that node-forge can be moved from dependencies to devDependencies.