search

kchristensen / udm-le

Let's Encrypt support for Ubiquiti UniFi OS
MIT License
593 stars 79 forks source link

[BUG] #49

Closed evaneaston closed 2 years ago

evaneaston commented 2 years ago

Describe the bug this isn't really a bug in udm-le (it is a feature regression in udm-core). I'm adding the issue here o maybe help other uem-le users who encounter the same problem. When updating to 1.11.0, the unifi-core service failed to start. It couldn't use my letsencrypt crt failing with "Error loading certificate /data/unifi-core/config/unifi-core.crt Cannot read public key. OID is not RSA." My cert was ASN1 OID: prime256v1, NIST CURVE: P-256.

My community.ui.com thread has copies of yhe logged errors. https://community.ui.com/questions/UniFi-OS-is-starting/3a0a50ea-8392-418a-b4f2-64bcede58d48

Version Information (please complete the following information):

Additional context I was able to run unifi-os shell, then remove my custom crt and key from /data/unifi-core/config/. The udm will the gen a self signed cert.

I suspect they reworked the cert management code in this release and it isn't configured for anything but RSA. I hope they fix this soon so we dont have to config lego to generate rsa certa. Maybe well find out more after the weekend.

kchristensen commented 2 years ago

Ugh, at the moment I am not using my udmp, so I haven't been keeping up on firmware versions, thanks for bringing this to my attention.

evaneaston commented 2 years ago

FWIW, I added --key-type rsa4096 to LEGO_ARGS and reran initialize and the udm ui works again. I didn't try to see what happens if you a lego renew with a different key type.

kchristensen commented 2 years ago

If that works, that seems like an easy enough fix. It should renew with the same type of key, so if this proves to fix it for you I'll push up a fix and release a new version for everyone else.

evaneaston commented 2 years ago

That should work. In a quick peruse of the lego source, it looks like the key type is only used for the run command. So only adding those args during initiate makes sense.

It would be really cool on renew to check the OID of the existing cert and if it isn't using an RSA key, convert the renew request automatically to an initiate/run request with rsa4096. But, I understand doing the testing to automate this just takes more precious time, especially if you aren't using the device anymore.

Something like:

IS_KEY_RSA=$(openssl rsa -in ${UDM_LE_PATH}/lego/certificates/${LEGO_CERT_NAME}.key -noout &>  /dev/null ; echo $?)
kchristensen commented 2 years ago

I wouldn't be opposed to adding that, but yeah testing is tough at the moment because my rack is torn apart and I'm using an Edgerouter until they release the UniFi Dream Wall.

Also from what I understand the devices using 2.x firmware like the Pro SE and stuff have ditched docker/podman entirely and are back to the old way of doing things. It's not clear if they'll ever go that route on the udmp but I think this project is not long for this world.

At any rate, if you want to PR up the key fix or a larger PR to do the key conversion (even if it's just, on renew error out if it determines the key isn't RSA) I'd be happy to merge it.

Servery commented 2 years ago

Did I understand correctly that no update with a fix is coming for udm-le? I already have --key-type rsa2048 in the LEGO_ARGS in the file /mnt/data/udm-le/udm-le.sh, do I just need to change that there to --key-type rsa4096 so that I can upgrade to 1.11.0 without problems?

kchristensen commented 2 years ago

I'll push up a fix if Evan doesn't, but I was waiting for verification that everything was working as expected since my ability to test is limited atm.

Give it a shot and see if it works for you and report back?

evaneaston commented 2 years ago

The issue with 1.11.0 is that your cert key must be an RSA key. It can be any valid length. By default lego generates and elliptic curve key. So if you've already customized the script to generate an rsa2048 key, you should be fine.

I proposed rsa4086, not because I like huge keys. Lego produces a 256-bit EC key by default. The NIST-recommended equivalent RSA key size would be 3072 buts. But lego doesn't offer that size. So I opted for the next size up, 4096. I think its generally a good idea for shared code to not change defaults in a way that reduces security.

I've been too busy to do a proper PR, but hope to this week.

kchristensen commented 2 years ago

So I'll admit it's been a while since I looked at this code, but I had a minute so I just went and looked: https://github.com/kchristensen/udm-le/blob/main/udm-le.sh#L10

I've had --key-type rsa2048 in there for at least 17 months. @evaneaston were you running a modified / very old version or something? You should have already had an RSA key unless you a) Hadn't updated in a long time b) Had generated a key more than 17 months ago or whenever I initially added that.

evaneaston commented 2 years ago

Ha funny. I've been renewing a key generated ~spring 2020. And when I quickly looked for the lego run command, I didn't look through the whole script and notice the rsa2048 key type at the top.

That could explain why so few people have had issues with certificates during the latest update.

kchristensen commented 2 years ago

Sounds like the mystery is solved! Going to close this out.