Pull request proposal: automatic certificate renewal

[Paul-Reed]

@BartButenaers

It is a pity that the existing ' https ' property isn't a function. Because now you have to specify both file paths (privkey.pem and cert.pem) twice in the settings

Please disregard if this is an ill-thought-out suggestion, but can't the certs be retrieved from settings.js https: by red.js, without calling it as a separate function.

[BartButenaers]

Paul,
Currently the 'https' has been setup like this:

    https: {
        key: fs.readFileSync('privkey.pem'),
        cert: fs.readFileSync('cert.pem')
    },

As soon as the settings.js file is read by Node-RED, all the code inside will be executed (a.o. the readFileSync functions). From then on the result will be cached (by the 'require' mechanism). This means that the readFileSync will never be executed again afterwards, but you always get the cached old result.

So I 'think' if it originally has been setup as below, that we indeed could have re-read the file (from disc) as often as we wanted:

    https: function() {
        return {
            key: fs.readFileSync('privkey.pem'),
            cert: fs.readFileSync('cert.pem')
        },
    }

So Nick's proposal to use a separate certificate renewal function makes sense ...

[pelis]

In case you have a custom developed node, you have access to the RED object which contains the server (RED.server). So you could implement certificate updates there... The RED object also holds the settings.
You could even create a simple custom node just for that purpose - Such a node wouldn't even have to be part of any flow, you could enable the functionality when the node gets added to the pallet. You would obviously have to be careful that the node would be implemented as a "singleton".

[BartButenaers]

@pelis

The RED object also holds the settings.

Hi @pelis,
Yes that indeed was a problem in my first proposal: All the nodes received (at startup) a RED.settings instance, which contained the original (pre-loaded) content of the certificate.pem and privatekey.pem files. When I loaded a renewed file afterwards, the nodes still only had the old original file contents at their disposal.

However in the last version of my code (based on Nick's proposal), all the nodes now have the two new properties available:

And since the settings contain a renewal "function":

This means that now each node can execute that function, to read the (perhaps renewed) files whenever they want. They will receive two buffers that represent the content of both files:

[Paul-Reed]

I assume that if this PR was added to node-RED, it would then open the door for the development that you hinted at in this post;

Node-RED SSL using Letsencrypt & Certbot

Hi Ioan, I have no knowledge about certbot, but I'm about to announce a new LetsEncrypt node for Node-RED. So I have been playing with the acme protocol already, which hopefully can get you a bit started ... When you request a new certificate for a domain (mydomain.com), the guys from Letsencrypt have to be sure that you are the owner of domain.com. Otherwise everybody could request a certificate, which means it wouldn't be secure anymore ... To make sure you are the owner, they will send…

If so, that would make adding SSL really easy, and be a very welcome addition.

Is there any news on this PR? The discussion around it seems to have gone quiet...

[dceejay]

It’s half term down here, so Nick is on vacation for a while

[knolleary]

At this point, there has been no pr raised for us to review or discuss.

The proposal still doesn't quite fit the "clean API" test for me. I don't see why a user should provide the certs in two different ways - both under the HTTPS property and the proposed new refresh function. I think it would be better if it were all under the https setting and the cert properties can either be the cert content itself (as is today), or a function. If it's a function and a refresh interval is also specified under the HTTPS property, then we will call it as needed.

(Yes, on holiday, so response times will be depending on kids and other interesting activities)

[BartButenaers]

@knolleary

both under the HTTPS property and the proposed new refresh function

Hey Nick,
Seems that I have not interpreted your "I'd suggest a design that allows the user to provide a function that Node-RED will call at the desired" correctly. Thought you meant a separate function.
But an (optional) function for the https setting itself indeed would be better. Have been thinking about that also at the time being, but thought it could break existing nodes that use the current https setting (and which don't test whether it is a function or not).
Will go back to the drawing table, as soon as I have time ...
Enjoy your holidays!!!

@Paul-Reed

I assume that if this PR was added to node-RED, it would then open the door for the development that you hinted at in this post

That Letsencrypt node was indeed the trigger for this pull request proposal. But I have no time anyway to do it all at once. Unfortunately ...

[BartButenaers]

@knolleary

I think it would be better if it were all under the https setting and the cert properties can either be the cert content itself (as is today), or a function. If it's a function and a refresh interval is also specified under the HTTPS property, then we will call it as needed.

Hey Nick,
Some delay due to a surgery...
Have the third implementation already running a couple of weeks on my RPI, and had no issues yet...

The settings.js file now looks like this:

    // The following property can be used to enable HTTPS
    // See http://nodejs.org/api/https.html#https_https_createserver_options_requestlistener
    // for details on its contents.
    // See the comment at the top of this file on how to load the `fs` module used by this setting.
    //https: {
    //  key: fs.readFileSync('privkey.pem'),
    //  cert: fs.readFileSync('cert.pem')
    //},
    // This 'https' property can also be a function.  For example for automatic certificate renewal 
    // (see the 'credentialRenewalTime' property below), this property needs to be a function:
    //https: function() {
    //     return {
    //         key: fs.readFileSync('privkey.pem'),
    //         cert: fs.readFileSync('cert.pem')
    //     }
    //},

    // The following property can be used to load renewed credential files at regular time intervals (seconds).
    // Prerequisite: the 'https' session should be enabled (based on a function)!
    // Caution: NodeJs version 11 or above is required to use this option!
    //credentialRenewalTime: 3600,

When the 'credentialRenewalTime' is activated, the 'https' property MUST be a function. Otherwise the OLD file contents (cached by 'require') will be reused over and over again. I think that some users might forget about this, and will not understand why the certificate isn't renewed in their browser. Therefore I have decided to let the renewal fail (with console log) in that case, to minimize the risc for such mistakes:

And the red.js file now looks like this:

if (settings.https) {
    var startupHttps = settings.https;
    
    if (typeof startupHttps === "function") {
        // Get the result of the function, because createServer doesn't accept functions as input
        startupHttps = startupHttps();
    }  

    server = https.createServer(startupHttps,function(req,res) {app(req,res);});
    
    // Setup automatic certificate renewal for NodeJs version 11 and above
    if (settings.credentialRenewalTime) {
        if (server.setSecureContext) {
            console.log("Checking renewed credentials every " + parseInt(settings.credentialRenewalTime) + " seconds.");
            setInterval(function () {
                try {
                    //console.log("Checking for renewed credentials.");
                    var renewedHttps = settings.https;
                    
                    if (typeof renewedHttps !== "function") {
                        console.log("Cannot renew credentials when the https property isn't a function.");
                        return;
                    }
                    
                    // Get the result of the function, because createServer doesn't accept functions as input
                    renewedHttps = renewedHttps();
                    
                    if (!renewedHttps.key || !renewedHttps.cert) {
                        console.log("Cannot renew credentials when the https property function doesn't return a 'key' and 'cert'.");
                        return;
                    }
                        
                    // Only update the credentials in the server when key or cert has changed
                    if(!server.key || !server.cert || !server.key.equals(renewedHttps.key) || !server.cert.equals(renewedHttps.cert)) {
                        server.setSecureContext(renewedHttps);
                        console.log("The credentials have been renewed.");
                    }
                } catch(err) {
                    console.log("Cannot renew the credentials: " + err);
                }
            }, parseInt(settings.credentialRenewalTime) * 1000);
        } else {
            console.log("Cannot renew credentials automatically.  NodeJs version 11 or above is required.");
        }
    }
} else {
    server = http.createServer(function(req,res) {app(req,res);});
}

Don't think there is any impact on the mechanism as is today: as long as the 'https' function isn't a function and the 'credentialRenewalTime' isn't activated, it should run like it used to do in the past...

I have added a check to make sure the file contents have changed, before loading them into the webserver. Just to be sure that we don't have any impact on the running server when the file contents haven't been renewed. That way it is even safe to set the 'credentialRenewalTime' property to run much more often, if that should be required for some use case ...

Hopefully this implementation finally fits the "clean API" test?

Thanks for reviewing!
Bart

[Paul-Reed]

Do we really need the two alternative https settings including in settings.js? Why not just the 'function' version.

Case 1 - New installation
Any new installation would use the 'new' settings.js file which only contained the 'function https'.

Case 2 - node-RED updates
Because settings.js is not overwritten when updating or re-running the script, existing users would continue to use the 'none-function' version that remains in their existing settings file.

Users using the 'old' settings file would be able to easily convert to the function version without replacing settings.js, by a simple edit, that could be described in the user guide/blog.

(I just know that the next post will contain Case 3, Case 4, etc )

[BartButenaers]

Hey Paul,
It makes sense to me what you are saying.
For me it is all fine to be honest. Just want to create a pull request for this tiny weeny code snippet, to get rid of it in my todo list. And preferable before the lock-down in Belgium is finished, and we are allowed to leave our homes again

[BartButenaers]

Hey Nick (@knolleary),
If you have some time, it would be nice if you could have a look at my last proposal (and Paul's remark about it). When it is ok for you, I will create a pull request because I need certificate renewal to better support a series of new nodes (e.g. my latest node-red-contrib-ui-web-push) which need trusted certificates like e.g. Letsencrypt.
Thanks!!

[dceejay]

Nick has been sick this last week and recovering. He’s managed to get 1.0.5 out and has kids at home to manage full time over Easter. So while I’m sure he will get to it, it may not be top of his priority list for a while.

[Paul-Reed]

Any decision yet @BartButenaers?

[knolleary]

@Paul-Reed if you read the last couple of posts you'll see this hasn't been resolved yet.

Here's my proposal for how to support the cert renewal. Note this is primarily about the externals of the design - how it would get used. I'm less concern at this stage about individual lines of code in the internal implementation - we can save that for discussion in the PR.

1. https can be the object it is today and everything works as it does today.
2. Alternatively, https can be a Function that when called, returns the object (or a Promise.. see below) with the appropriate values.
3. httpsRefreshInterval is a new property to determine how often it will refresh the https configuration.

Furthermore...

1. if https is a static object and httpsRefreshInterval is set, we should log a warning that is an invalid configuration, but it should not be fatal.
2. if httpsRefreshInterval is set and we are running in a version of Node that doesn't support changing the secure context, we should log a warning.
3. if https is a function, it should be able to return either the Object itself, or a Promise that resolves to the object. This will allow the function to perform asynchronous actions, such as re-reading files etc.

Any changes need to go into the dev branch for the 1.1.0 release.

[Paul-Reed]

@knolleary

if you read the last couple of posts you'll see this hasn't been resolved yet.

It was a subtle rhetorical question.
My apologies if it's not appreciated.

[BartButenaers]

I finally managed to find some time to create a pull request. Fingers crossed

[BartButenaers]

Morning Paul,
not sure if you have seen it, but the pull-request for automatic certificate renewal has finally been merged for the 1.1
Thanks to Nick and Dave for reviewing this PR and assisting me through the hell of promises!
Have learned a lot from this PR, but I have aged a couple of years due to it
Next PR will be a slightly more digestible topic ...