Fails to notify that Node v10.12.0 or better is needed #8

New Issue

Ghost · 2019-11-19T05:49:28Z

Ghost commented

2019-11-19 05:49:28 +00:00

It seems GreenLock v3 uses "require('crypto').generateKeyPair()" which only exists starting with NodeJS v10.12.0 as documented here: https://nodejs.org/api/crypto.html#crypto_crypto_generatekeypair_type_options_callback

The code just errors when I called greenlock.renew() with no indication of the problem. I had to trace the GreenLock code to find this out. Would it be possible to add a detection and notification of this so that others don't run into the same problem? - Thanks.

It seems GreenLock v3 uses "require('crypto').generateKeyPair()" which only exists starting with NodeJS v10.12.0 as documented here: https://nodejs.org/api/crypto.html#crypto_crypto_generatekeypair_type_options_callback The code just errors when I called greenlock.renew() with no indication of the problem. I had to trace the GreenLock code to find this out. Would it be possible to add a detection and notification of this so that others don't run into the same problem? - Thanks.

coolaj86 commented

2019-11-19 05:53:32 +00:00

A few things:

What error did you get?
Yes, it's already supposed to throw an error that says something very similar to "upgrade to node v10.12 or install ursa or node-forge for older node versions"
Did you perhaps supply a notify function and ignore the error events?

A few things: 1. What error did you get? 2. Yes, it's already supposed to throw an error that says something very similar to "upgrade to node v10.12 or install ursa or node-forge for older node versions" 3. Did you perhaps supply a `notify` function and ignore the `error` events?

Ghost commented

2019-11-19 06:05:10 +00:00

Wow, your quick on the response.

The .then() on the renew() got called with no indication of the exception. I think it returned an empty array [] if I remember correctly.
Your probably right. It did not seem to have propagated up.
Yes, I did get "error", arguments = {"length":0}. Nothing else.

I will put guards in my code to check for this, but figured I put this here to help others. Thank you for GreenLock by the way, it's almost core infrastructure. I try to support Node v6 and higher for MeshCentral, but for Let's Encrypt, I will have to bump up the requirements.

Wow, your quick on the response. 1. The .then() on the renew() got called with no indication of the exception. I think it returned an empty array [] if I remember correctly. 2. Your probably right. It did not seem to have propagated up. 3. Yes, I did get "error", arguments = {"length":0}. Nothing else. I will put guards in my code to check for this, but figured I put this here to help others. Thank you for GreenLock by the way, it's almost core infrastructure. I try to support Node v6 and higher for MeshCentral, but for Let's Encrypt, I will have to bump up the requirements.

coolaj86 commented

2019-11-19 06:24:31 +00:00

You happened to catch me at a good time.

Hmm.... I'll have to look into that.

There was some back and forth about node v6 support and ultimately I decided that continuing to support it wasn't worth it. Everything should be supported in node v8, but I've only been testing v10, v11, and v12 while I iron out a few last issues with the normal use of Greenlock v3.

I know that in v8 you have to install ursa or node-forge and you have to change accountKeyType to RSA-2048 due to the lack of P-256 support.

And, due to community feedback, I'm actually going to publish v4 soon. It's not actually a major change, but I'm going back to configDir instead of configFile, which will require a manual step mv greenlock.json ./path/to/confdir/config.json that I can't reasonably automate.

I plan to release Greenlock Pro soonish, perhaps in January, as well.

You happened to catch me at a good time. Hmm.... I'll have to look into that. There was some back and forth about node v6 support and ultimately I decided that continuing to support it wasn't worth it. Everything _should_ be supported in node v8, but I've only been testing v10, v11, and v12 while I iron out a few last issues with the normal use of Greenlock v3. I know that in v8 you have to install ursa or node-forge and you have to change `accountKeyType` to `RSA-2048` due to the lack of `P-256` support. And, due to community feedback, I'm actually going to publish v4 soon. It's not actually a major change, but I'm going back to `configDir` instead of `configFile`, which will require a manual step `mv greenlock.json ./path/to/confdir/config.json` that I can't reasonably automate. I plan to release Greenlock Pro soonish, perhaps in January, as well.

Ghost commented

2019-11-19 19:46:04 +00:00

Nice. I have no problem requiring a better version of NodeJS if Let's Encrypt is used. Plenty of people use MeshCentral without GreenLock on a Raspberry Pi and so, it will still work on v6 in that case. I dynamically install GreenLock when needed, so it's not part of the MeshCentral dependencies by default.

FYI. If you want to see what the last few weeks of MeshCentral/GreenLock looked like:
https://github.com/Ylianst/MeshCentral/issues/658
https://github.com/Ylianst/MeshCentral/issues/629
https://github.com/Ylianst/MeshCentral/issues/662

I am not sure what you would think of my implementation of GreenLock in MeshCentral. I want to avoid placing any configuration file for GreenLock and instead feeding that config at runtime. At some point, I would like to optionally store the LE certs in HashiCorp Vault.

Looking forward to your new releases.

Nice. I have no problem requiring a better version of NodeJS if Let's Encrypt is used. Plenty of people use MeshCentral without GreenLock on a Raspberry Pi and so, it will still work on v6 in that case. I dynamically install GreenLock when needed, so it's not part of the MeshCentral dependencies by default. FYI. If you want to see what the last few weeks of MeshCentral/GreenLock looked like: https://github.com/Ylianst/MeshCentral/issues/658 https://github.com/Ylianst/MeshCentral/issues/629 https://github.com/Ylianst/MeshCentral/issues/662 I am not sure what you would think of my implementation of GreenLock in MeshCentral. I want to avoid placing any configuration file for GreenLock and instead feeding that config at runtime. At some point, I would like to optionally store the LE certs in HashiCorp Vault. Looking forward to your new releases.

Ghost closed this issue

2020-03-02 20:01:45 +00:00

Sign in to join this conversation.

No Label

No Milestone

No Assignees

2 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: root/greenlock.js#8