openwrt / luci

LuCI - OpenWrt Configuration Interface
Apache License 2.0
6.11k stars 2.48k forks source link

luci-app-uhttpd: generated certificate from gui does not contain alt_name resulting in error in browser Edge and Chrome #6701

Open swtrse opened 8 months ago

swtrse commented 8 months ago

Steps to reproduce:

  1. go to: Services → uHTTPd→ uHTTPd Self-signed Certificate Parameters
  2. set parameters to your liking
  3. Save & Apply
  4. click "remove old certificate and key" button

Actual behavior:

  1. The generated certificate will not be accepted by modern browsers

Expected behavior:

A certificate that will be accepted by modern browsers

Additional Information:

The effect you can see on windows and edge for example is. That if you have not trusted the certificate edge will show an NET::ERR_CERT_AUTHORITY_INVALID error. This error can be ignored and the website can be accessed. However, to get rid of the certificate error the normal way is to download the certificate and install it in the "Trusted Root Certificate" store for the local machine. The problem is, if you use the current generated certificate you get an NET:ERR_CERT_INVALID error which you not be ignored and you can not access the website until you removed the certificate from the certificate store again.

As a workaround you can create the certificate manually The commands are

openssl ecparam -name prime256v1 > ec.param
openssl req -x509 -nodes -days 3650 -newkey ec:ec.param -keyout mycert.key -out mycert.crt -config myconfig.conf

The used config looks like this

[req]
distinguished_name  = req_distinguished_name
x509_extensions     = v3_req
prompt              = no
string_mask         = utf8only
utf8            = yes

[req_distinguished_name]
C                   = <Country>
ST                  = <State>
L                   = <local>
O                   = <organisation>
OU                  = <organisation unit>
CN                  = <fqdn of openwrt device (openwrt.lan)>

[v3_req]
keyUsage            = nonRepudiation, digitalSignature, keyEncipherment
extendedKeyUsage    = serverAuth
subjectAltName      = @alt_names

[alt_names]
DNS.1               = <fqdn of openwrt device (openwrt.lan)>
IP.1                = <ipv4 of openwrt device (10.0.0.1)>
IP.2                = <ipv6 of openwrt dervice (fd18:211d:d95::1)>
<add more IPs if needed>

The certificate generated this way gets accepted and the certificate error vanishes as soon as the certificate is installed in the certificate store.

The workaround makes it clear that the certificates generated over the ui lacks the alternative names which seems to be a requirement for modern browsers.

brada4 commented 8 months ago

You establish trusted CA able to sign the whole internet to get rid of a warning.

swtrse commented 8 months ago

You establish trusted CA able to sign the whole internet to get rid of a warning.

If you do not have something constructive to share, please refrain from commenting.

brada4 commented 8 months ago

You imply some nonconformance. Last time I checked cabforum baseline did not mandate 10 years certificate lifetime. Any points more continuing reading further?

swtrse commented 7 months ago

The certificate lifetime is totally irrelevant to this problem. The problem is related to missing alt names, so your input is nonconstructive and leads away from the core of the problem. Therefore, please do not post if you do not have anything to say about the alt names part. But lets just change the command to openssl req -x509 -nodes -days 365 -newkey ec:ec.param -keyout mycert.key -out mycert.crt -config myconfig.conf Same problem, are you happy now?

brada4 commented 7 months ago

You can import any key generated and signed by CA you make trusted on your desktop, but dont use webserver's "nodes" private key for CA that is able to sign anything on the internets after.

stangri commented 7 months ago

@swtrse maybe changing the title of the issue from luci-app-uhttpd Generated certificate from gui does not meet requirements for modern browsers to luci-app-uhttpd: generated certificate from gui does not contain alt_name resulting in error in browser X would describe the problem better.

brada4 commented 7 months ago

@stangri or better "provide option to add alt name to CSR" , the local signing is good as it is, one may change hostname right after first boot made the first key.

stangri commented 7 months ago

@stangri or better "provide option to add alt name to CSR" , the local signing is good as it is, one may change hostname right after first boot made the first key.

Yeah, I've been thinking about that -- when fix is implemented, ideally the cert generation will need to be triggered on hostname or local domain change. Maybe that's why it's what it is.

brada4 commented 7 months ago

Maybe it needs more expedite way to support "enterprise ca" signing like export csr (adding altnames), then import crt while crypto key never leaves the device.

systemcrash commented 7 months ago

@stangri or better "provide option to add alt name to CSR" , the local signing is good as it is, one may change hostname right after first boot made the first key.

Yeah, I've been thinking about that -- when fix is implemented, ideally the cert generation will need to be triggered on hostname or local domain change. Maybe that's why it's what it is.

I was just looking at this yesterday, whether one could also add the sAN portion. Needs investigation to see whether this non-standard extension is possible to add via all of the tools that the init script looks through. Grabbing the current hostname, whatever.lan and main configured IPs uhttpd listens on.

systemcrash commented 7 months ago

Maybe it needs more expedite way to support "enterprise ca" signing like export csr (adding altnames), then import crt while crypto key never leaves the device.

I wonder whether there's an entire infrastructure for something like this... kind of like ACME.

hnyman commented 7 months ago

was just looking at this yesterday, whether one could also add the sAN portion.

Have you noticed the commit in @jow- staging repo?

https://git.openwrt.org/?p=openwrt/staging/jow.git;a=commitdiff;h=885b6423a2f286e32e0ade933231f1f55221723d

systemcrash commented 7 months ago

Author: Pat. Year: 2019. 👀

Multiple sAN are possible so adding the various actual URL to reach openwrt would be helpful.

hnyman commented 7 months ago

Author: Pat. Year: 2019

Yeah, jow has had it for a long time in the repo.

And I have noticed it there for some time. I authored the uhttpd cert generation changes in 2016 to create the possibility to use openSSL. But my Firefox has been happy with the exception that I defined, so I have not have a need to really look into the proposed commit about alternative name support.

Not quite sure why @jow- has not commited that into the main repo if it works.

systemcrash commented 7 months ago

I suppose more detail has the potential for more problems. And now we have a valid use case. I've not yet upgraded macOS. And I use Firefox usually anyway. But I imagine it's the same there since cert repo is os based.

jow- commented 7 months ago

I never pushed it since I wasn't really affected and had no access to a macOS installation to test it. With Firefox and Chrome as well as my various iOS devices I was always able to just dismiss the SSL nag screens.

brada4 commented 7 months ago

for safari you need to reduce validity period from 730 to 497 days in uhttpd init file. It is in cabforum spec, just that only apple rejects excessive validity interval besides current and remaining validity.

systemcrash commented 7 months ago

Does it outright reject the certificate or does it present a separate warning for this?

brada4 commented 7 months ago

You have to click modal popup every time like on expired cert.

swtrse commented 7 months ago

Just to be clear. I ran into this problem pressing this button. Not during some automatic certificate creation.... image

brada4 commented 7 months ago

you changed self-signed certificate including it being its own CA, so you had to trust new self-signed certificate. Nothing that certificate attributes will ever cure.

swtrse commented 7 months ago

Ok, we understand that reading for meaning is not one of your strengths. But I think I've finally recognized your error in thinking.

Just for convenience I use a self-signed certificate in the example. If you able to use an registered CA you just skip the first steps and start with step 7 but the outcome is still the same.

Steps to reproduce (tested on Windows 11) 1) Log in to openwrt. -> System->Administration->HTTP(S) Access->check Redirect to HTTPS->Save&Apply 2) Close Browser and connect to openwrt again. 3) You will be welcomed with an NET::ERR_CERT_AUTHORITY_INVALID error page on Edge, Chrome and every Chromium based Browser. Most likely on every current browser. 4) With click on advanced you get the option to ignore the warning and continue to the openwrt login page. 5) The important part here is in step 3 you can ignore and continue so this is only a nuisance. Adding alt names will of course not solve that problem because the CA is not trusted. However there are ways to solve that. a) Using a trusted CA b) add the certificate in the certificate store and register it as trusted CA. 6) Based on 4 wie download the certificate (since wie did not use a trusted CA) and install it in the certificate store (Under Windows 11 this would be Local Machine->Trusted Root Certificate). Which is the normal way of getting rid of warnings for self signed certificates or for using your own CA. I am sure macOS has some similar functions. 7) Close Browser and connect to openwrt again. 8) You will be welcomed with an NET:ERR_CERT_INVALID error page on Edge, Chrome and every Chromium based Browser. Most likely on other current browsers too. 9) The important part here is that in step 8 you can not choose to ignore this warning and therefore unable to connect to the openwrt login page. More important if you have used a globally trusted CA you only option to gain access to the openwrt pages again is to untrust that CA. 10) The Waring from step 8 only goes away if the alt names DNS.1 and IP.1 are present. Other alt names are optional.

So as long as the Certificate does not hold the alt names it does not matter what CA you are using you will be locked out from the openwrt pages. Ironically this is only the case when the CA is trusted. Because for untrusted CAs you will run into the waring mentioned in step 3 and that can be ignored.

Did you get it now? Step 8+9 is the problem. Not anything else.

The missing function to choose an CA to create a certificate with a trusted certificate chain can be addressed in the future if there is demand but it does not affect the problem here.

brada4 commented 7 months ago

Yes, very simple, no matter your suggested modification you will need to accept self-signed certificate once.

swtrse commented 7 months ago

Yes, very simple, no matter your suggested modification you will need to accept self-signed certificate once.

Yes, now we are on the same page. Yes, I will need to accept the sef-signed certificate once or at least the used CA once if not already trusted. That is correct and never was out of the question.

The whole case would not be as urgent if edge or chrome would give the user the ability to ignore the warning. In my research i found some registry keys that can be set but all they achieve is to disable the option to ignore certificates waring completely so if they are set you cannot even go past the waring in step 3. Thankfully they are not activated by default.

hnyman commented 7 months ago

@jow-

I think that the commit in your staging repo would help for the "step 8+9" problem:

as long as the Certificate does not hold the alt names it does not matter what CA you are using you will be locked out from the openwrt pages. Ironically this is only the case when the CA is trusted

Most users never bother to add the self-signing CA into the trusted CA database, so they never run into this as we just once accept the NET::ERR_CERT_AUTHORITY_INVALID warning.

But apparently some users do tinker with CA database and will then later run into NET:ERR_CERT_INVALID. Those two new fields might help these users.

I applied the patch to my live OpenWrt system (with OpenSSL) and both Firefox, Edge and Chrome (in Windows) showed the new fields in the certificate data for the new self-generated certificates. Normal NET::ERR_CERT_AUTHORITY_INVALID warning, but nothing else.

So, the patch in your repo works at least with OpenSSL based OpenWrt. However, I doubt that those parameters will not work with px5g tools with wolfssl or mbedtls.

Looking at px5g sources, e.g. https://github.com/openwrt/openwrt/blob/main/package/utils/px5g-mbedtls/px5g-mbedtls.c , we might need to add support for extendedKeyUsage and subjectAltName into px5g.

If the new generation attributes only work with OpenSLL and cause errors with mbedtls (?), the change might now be added only to the OpenSSL based variant of the command. I tested also that modification:

root@router5:~# diff -u /rom/etc/init.d/uhttpd /etc/init.d/uhttpd
--- /rom/etc/init.d/uhttpd      2023-11-24 17:25:42.000000000 +0200
+++ /etc/init.d/uhttpd  2023-11-25 14:33:12.000000000 +0200
@@ -52,7 +52,7 @@
        local KEY_OPTS="rsa:${bits:-2048}"
        local UNIQUEID=$(dd if=/dev/urandom bs=1 count=4 | hexdump -e '1/1 "%02x"')
        [ "$key_type" = "ec" ] && KEY_OPTS="ec -pkeyopt ec_paramgen_curve:${ec_curve:-P-256}"
-       [ -x "$OPENSSL_BIN" ] && GENKEY_CMD="$OPENSSL_BIN req -x509 -sha256 -outform der -nodes"
+       [ -x "$OPENSSL_BIN" ] && GENKEY_CMD="$OPENSSL_BIN req -x509 -sha256 -outform der -nodes -addext extendedKeyUsage=serverAuth -addext subjectAltName=DNS:\"${commonname:-OpenWrt}\""
        [ -x "$PX5G_BIN" ] && GENKEY_CMD="$PX5G_BIN selfsigned -der"
        [ -n "$GENKEY_CMD" ] && {
                $GENKEY_CMD \
swtrse commented 7 months ago

@hnyman I tested the changes on my system and even added the two IP names to the certificate and it worked fine. I will go on and try to make this work with my own created CA. That would be perfekt.

For the time being I use a script that I run periodically that is working too.

A plugin gui-based to let openwrt act as a root authority would be a dream....

brada4 commented 7 months ago

@swtrse yes, cool make your CA, that is the safest way. My complaint was that you cannot export CSR and import CRT to support your own (or "enterprise") CA

hnyman commented 7 months ago

Regarding px5g: the new fields have actually been added to px5g-wolfssl by commit https://github.com/openwrt/openwrt/commit/6bfc8bb4a37903bd1d3bb7e7824d89f3a2cca6a1 in Dec 2021. If cert has been created with px5g-wolfssl, there is already now altName in the cert.

Only px5g-mbedtls is apparently missing support for altNames, keyusage etc. But it is our current default...

So, in the current state:

brada4 commented 7 months ago

@hnyman looks like the frameworks need to be "aligned" to give consistent results....

systemcrash commented 7 months ago

for safari you need to reduce validity period from 730 to 497 days in uhttpd init file. It is in cabforum spec, just that only apple rejects excessive validity interval besides current and remaining validity.

Could not find any reference to 497. I did find 397, however.

https://github.com/cabforum/servercert/blob/90a98dc7c1131eaab01af411968aa7330d315b9b/docs/BR.md?plain=1#L175

Certificates issued SHOULD NOT have a Validity Period greater than 397 days and MUST NOT have a Validity Period greater than 398 days.

This seemingly regards server certificates signed by a CA. So whatever downward limits a browser applies to certificates may stem from this value.

systemcrash commented 7 months ago

Regarding px5g: the new fields have actually been added to px5g-wolfssl by commit openwrt/openwrt@6bfc8bb in Dec 2021. If cert has been created with px5g-wolfssl, there is already now altName in the cert.

* px5g-mbedtls, our current default, is still missing support for the fields. (but the code in px5g-wolfssl commit might give the correct format to add it)

Are there plans or ideas to migrate to mbedtls v3.x? It has clean helper methods and structure to handle exactly this. I wrote something resembling a patch using them.

A migration to v3 is no basic task by the looks of things.

mbedtls v2.x is ... a mess. To get correct sAN values and/or extended usages into a cert is messy and it looks like you must roll your own ASN1/OCTET strings. Not impossible.

systemcrash commented 3 months ago

The linked PR should make fixing this possible https://github.com/openwrt/openwrt/pull/15092

hnyman commented 2 months ago

@systemcrash Did you ever manage to actually test your code of "something resembling a patch"? https://github.com/systemcrash/openwrt/commit/696eeae40ce2dddc11fb5d79f672662506390131 I tried using it and seemed to stumble into some logical errors ( as mbedtls disliked the empty last list item etc.).

However, thanks to your initial work, I managed to create a working patch by modifying your code.

I also modified uhttpd to have the creation argument included, taking the old patch from @jow- staging repo. (I also decreased the default validity time of certificate, as it seems to have been decreased in 2020)

I have tested this with openssl, mbedtls and wolfssl. Seems to work. https://github.com/hnyman/openwrt/commits/uhttpdcert/

Before doing a PR, I will likely squash your commit and mine, so that it is more clean.

Open for any comments...

systemcrash commented 2 months ago

No. I just left the code around in the hope that it would be a useful starting point when v3.x arrived. I remember fighting with compile errors before I could get to polishing code logic. Squash away. 😅

hnyman commented 2 months ago

PR: https://github.com/openwrt/openwrt/pull/15366