Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Certificate is invalid on Chrome #3535

Closed
ramiel opened this issue Jun 30, 2020 · 24 comments
Closed

Certificate is invalid on Chrome #3535

ramiel opened this issue Jun 30, 2020 · 24 comments
Labels
bug 🐞 Something isn't working upstream ⬆️ Relates to some dependency of this project
Milestone

Comments

@ramiel
Copy link

ramiel commented Jun 30, 2020

If I start a reverse proxy like this

sudo caddy reverse-proxy --to 127.0.0.1:1234

where under :1234 there's a server running, it works perfectly on Firefox when I visit https://localhost. If I visit the same on chromium-based browsers (Chrome, Brave) I get an NET::ERR_CERT_INVALID error.

On the command line I get this error:
http: TLS handshake error from [::1]:43216: remote error: tls: unknown certificate

Caddy version is v2.1.0 h1:MC4d65RCVaEKy1iOFjsD51mybOwS8qdEVBi7ESDhUfE=

@mholt
Copy link
Member

mholt commented Jun 30, 2020

You might need to restart your browser; in any case, just make sure Caddy's root certificate is installed into the trust store used by Chrome / Brave. Caddy's auto-installation is a best-effort but still needs some improvement.

We use functions based on: https://github.com/FiloSottile/mkcert

@mholt mholt closed this as completed Jun 30, 2020
@mikaelhg
Copy link

mikaelhg commented Jul 5, 2020

@mholt, just encountered the same issue with caddy 2.1.1, Chrome 83.0.4103.116. Chrome has the caddy CA on the trusted list, I checked.

To me, it looks like Chrome doesn't like the fact that the certificate from the internal issuer doesn't have the subject commonName set.

@mikaelhg
Copy link

mikaelhg commented Jul 5, 2020

Indeed, when I made a small change to InternalIssuer.Issue

csr.Subject.CommonName = "hardcoded.name"

and

rm -rf ~/.local/share/caddy

suddenly the issue went away, and Chrome accepted the certificate, no problem.

I would love to create a PR of this, but I'm completely ignorant of the Go language, and can't really figure out where I'd get the name for the CSR. I could give it a shot, if you point me in the right direction.

@mholt
Copy link
Member

mholt commented Jul 5, 2020

@mikaelhg Can you post the full output of the certificate viewer in Chrome with the problematic cert?

@mikaelhg
Copy link

mikaelhg commented Jul 5, 2020

Broken certificate:
broken_cert_viewer

Working certiticate:
working_cert_view

diff --git a/modules/caddytls/internalissuer.go b/modules/caddytls/internalissuer.go
index ca43bf8f..1c69bbe2 100644
--- a/modules/caddytls/internalissuer.go
+++ b/modules/caddytls/internalissuer.go
@@ -119,6 +119,8 @@ func (li InternalIssuer) Issue(ctx context.Context, csr *x509.CertificateRequest
                lifetime = issuerCert.NotAfter.Sub(time.Now())
        }
 
+       csr.Subject.CommonName = "baddy.caddy.daddy"
+
        certChain, err := auth.Sign(csr, provisioner.Options{},
                profileDefaultDuration(li.Lifetime),
        )

@mikaelhg
Copy link

mikaelhg commented Jul 5, 2020

Oh, maybe you meant the exports... I had to reproduce the process to get those, so these aren't exactly identical to the first ones, but they behave identically.

Working:

-----BEGIN CERTIFICATE-----
MIIB3zCCAYSgAwIBAgIQPZGE/xNM/M83yKL4TJdzBTAKBggqhkjOPQQDAjAzMTEw
LwYDVQQDEyhDYWRkeSBMb2NhbCBBdXRob3JpdHkgLSBFQ0MgSW50ZXJtZWRpYXRl
MB4XDTIwMDcwNTIwMDYwMloXDTIwMDcwNjA4MDcwMlowHDEaMBgGA1UEAxMRYmFk
ZHkuY2FkZHkuZGFkZHkwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAQMjhI3Ixo1
mdSo1/POWHK/mG5Olm3HScY2i+a9GcOg9FGZ4gZO7Egiv/zmD3QfEk2rdkclOWNA
b1iviT2n51DMo4GQMIGNMA4GA1UdDwEB/wQEAwIFoDAdBgNVHSUEFjAUBggrBgEF
BQcDAQYIKwYBBQUHAwIwHQYDVR0OBBYEFK2ZwGgiGFL9adeJJoJid0Dtnl1UMB8G
A1UdIwQYMBaAFDaB+1q7Pttk4Ej1wkQYF+fjWFfMMBwGA1UdEQQVMBOCEWJhZGR5
LmNhZGR5LmRhZGR5MAoGCCqGSM49BAMCA0kAMEYCIQDpKOyfs6iFW/AXC1VK7E5X
jj7q7P46AbCLaeNcOYuUugIhAPixHSmHCvPnmTE2WH2/eyEmXUQ3nvWetNG9ZGrR
MJeZ
-----END CERTIFICATE-----
-----BEGIN CERTIFICATE-----
MIIByDCCAW6gAwIBAgIRAPnEzy/G99tpnUwfhDERGFYwCgYIKoZIzj0EAwIwMDEu
MCwGA1UEAxMlQ2FkZHkgTG9jYWwgQXV0aG9yaXR5IC0gMjAyMCBFQ0MgUm9vdDAe
Fw0yMDA3MDUyMDA3MDJaFw0yMDA3MTIyMDA3MDJaMDMxMTAvBgNVBAMTKENhZGR5
IExvY2FsIEF1dGhvcml0eSAtIEVDQyBJbnRlcm1lZGlhdGUwWTATBgcqhkjOPQIB
BggqhkjOPQMBBwNCAASbNayUyPTXIUKRqjTCV9isoKO4EgZcX3qHx+BqpQS3tcrq
qITW+x+GpHciXjV3eaWhi9Zw9GvbOPMFd5+Bp+/Eo2YwZDAOBgNVHQ8BAf8EBAMC
AQYwEgYDVR0TAQH/BAgwBgEB/wIBADAdBgNVHQ4EFgQUNoH7Wrs+22TgSPXCRBgX
5+NYV8wwHwYDVR0jBBgwFoAUMgmOzIXRgzYu9DufUIiFkq/6tHwwCgYIKoZIzj0E
AwIDSAAwRQIgT9Op0VJSVKV+R8CdYy8N3kT9fzb+SHxbO7vQR7PnKaICIQD4QVlI
w9DaNLD2PNeGahcHTywUyJigbJkZiKsGIQMqzw==
-----END CERTIFICATE-----
-----BEGIN CERTIFICATE-----
MIIBojCCAUmgAwIBAgIQeWIqS1YzL7OMucG4+1619TAKBggqhkjOPQQDAjAwMS4w
LAYDVQQDEyVDYWRkeSBMb2NhbCBBdXRob3JpdHkgLSAyMDIwIEVDQyBSb290MB4X
DTIwMDcwNTIwMDcwMloXDTMwMDUxNDIwMDcwMlowMDEuMCwGA1UEAxMlQ2FkZHkg
TG9jYWwgQXV0aG9yaXR5IC0gMjAyMCBFQ0MgUm9vdDBZMBMGByqGSM49AgEGCCqG
SM49AwEHA0IABJ6MZL7Kcyl62E33jJqqDeQZpRl4M5Jj8B1ohjClRGTveQ9Jxnh5
7jD9uq72JPevmtHNjj+bm0d/MzR53Pgp1M2jRTBDMA4GA1UdDwEB/wQEAwIBBjAS
BgNVHRMBAf8ECDAGAQH/AgEBMB0GA1UdDgQWBBQyCY7MhdGDNi70O59QiIWSr/q0
fDAKBggqhkjOPQQDAgNHADBEAiBUspmXwYIf6FhYf4HGCFrDYVhvZz4fxMMM1T/8
UK244gIgEIz9br+OhB5nr/0kGePbmZYqTQQR0WRBtMkcTlJFugU=
-----END CERTIFICATE-----

Broken:

-----BEGIN CERTIFICATE-----
MIIBwTCCAWigAwIBAgIQFrtoMOIdfAAT1UNGZ5YAADAKBggqhkjOPQQDAjAzMTEw
LwYDVQQDEyhDYWRkeSBMb2NhbCBBdXRob3JpdHkgLSBFQ0MgSW50ZXJtZWRpYXRl
MB4XDTIwMDcwNTIwMTAzN1oXDTIwMDcwNjA4MTEzN1owADBZMBMGByqGSM49AgEG
CCqGSM49AwEHA0IABHakCMgPpJeg2UHHmOIFMUCj11WsOWPhKjfgaQUZAYh4LUOO
qPuYnlTCRWE9cTt0xbsHGWMLbVFDmdmvVbkoIuujgZAwgY0wDgYDVR0PAQH/BAQD
AgWgMB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjAdBgNVHQ4EFgQU7uuD
9zvb5Qbw5XiC0YAPnZAuiiswHwYDVR0jBBgwFoAURtBHg+nishLcDpfYVskOIAXx
t+0wHAYDVR0RBBUwE4IRYmFkZHkuY2FkZHkuZGFkZHkwCgYIKoZIzj0EAwIDRwAw
RAIgTqv7CgI8sYhI3w7R1LKIwJY3Apvc4yg7C96aLEV5g34CIAZYCQ7xhSgP4paX
TDOTD7q+UStFmyA7sRButOoUt7cD
-----END CERTIFICATE-----
-----BEGIN CERTIFICATE-----
MIIBxzCCAW6gAwIBAgIRAKnNEhIMpYaRUJa8GsgZSw8wCgYIKoZIzj0EAwIwMDEu
MCwGA1UEAxMlQ2FkZHkgTG9jYWwgQXV0aG9yaXR5IC0gMjAyMCBFQ0MgUm9vdDAe
Fw0yMDA3MDUyMDExMzZaFw0yMDA3MTIyMDExMzZaMDMxMTAvBgNVBAMTKENhZGR5
IExvY2FsIEF1dGhvcml0eSAtIEVDQyBJbnRlcm1lZGlhdGUwWTATBgcqhkjOPQIB
BggqhkjOPQMBBwNCAATH3M6QzeSNevhvnI4c1VT2OZhBfyV6a77jhiS0rFGVpuTp
G8r7nt5IoUfMEiA9eQwsxawCBnUseIhfqRKpvikPo2YwZDAOBgNVHQ8BAf8EBAMC
AQYwEgYDVR0TAQH/BAgwBgEB/wIBADAdBgNVHQ4EFgQURtBHg+nishLcDpfYVskO
IAXxt+0wHwYDVR0jBBgwFoAUKiCIhhWFXS3kCyQMQrO0m/yED7AwCgYIKoZIzj0E
AwIDRwAwRAIgb8wIV/TP/o3kAxIse+WXJJ0wXzqle800VqvqdSJ5XcsCIGQ4pdPJ
2+KwGPi90GKALC36eHoLjAKAmDKlv4jBDHkm
-----END CERTIFICATE-----

@mikaelhg
Copy link

mikaelhg commented Jul 5, 2020

Should I open a new issue, or will you reopen this one?

@mholt
Copy link
Member

mholt commented Jul 5, 2020

Thanks, the full PEM dump was helpful. It appears to be because the SAN extension is not marked as critical, which Chrome requires when there is not a CommonName (subject).

@mholt mholt reopened this Jul 5, 2020
@mholt mholt closed this as completed in 2a5599e Jul 6, 2020
@mholt
Copy link
Member

mholt commented Jul 6, 2020

This was a combination of behaviors in both smallstep and Chrome.

Upgrading to the latest smallstep release (v0.14.6) seems to fix the problem for me. (/cc @maraino)

With the latest update, Key Usage Key Encipherment is gone is gone, and SAN extension has "Critical: Yes" which I believe is the correct behavior.

@mholt mholt added bug 🐞 Something isn't working upstream ⬆️ Relates to some dependency of this project labels Jul 6, 2020
@mholt mholt modified the milestones: 2.x, v2.2.0 Jul 6, 2020
@mikaelhg
Copy link

mikaelhg commented Jul 6, 2020

I can confirm that the current master fixed the issue for me. Thank you.

@aduzsardi
Copy link

aduzsardi commented Jul 9, 2020

Building from master seems to fix the issue with chrome and chromium but the subject is still missing in the issued certificate, is this case for you too @mikaelhg ?

@mikaelhg
Copy link

mikaelhg commented Jul 9, 2020

@aduzsardi , it is, and there might be further complications with non 200 OK results. I'll have to look into it when I have time.

Edit: Nevermind, it's just Chrome marking the default 404 as untrusted because the content comes from an internal source. With an actual 404 document Chrome still shows the page as trusted.

@mholt
Copy link
Member

mholt commented Jul 9, 2020

@aduzsardi As I said in the other issue, CommonName has been deprecated for 20 years. It is intentional that CommonName is not set in Caddy certificates. Even Chrome has not accepted it for years now.

@aduzsardi
Copy link

i think it should still have some kind of identifier in the subject field, if not why do all the CA's add it anyway ? :)

cert-view-chrome_1

cert-view-chrome_2

@mholt
Copy link
Member

mholt commented Jul 9, 2020

The subject is not required when (as of somewhat recently) the SAN extension is critical, which is what was fixed upstream. You can think that field should be used out all you want, but there's good reasons it was deprecated 20 years ago. I imagine CAs today only fill it out for backwards compatibility with really old software (that, and a bug in macOS until 10.13) which isn't something Caddy cares about (in fact it's contrary to the goals of this project).

@aduzsardi
Copy link

aduzsardi commented Jul 9, 2020

I'm ok with this too even if it will be tagged as "Won't fix/not a bug" , but when looking at the Certificate hierarchy tree view for the certificate it just looks very silly

Root CA -> Intermediate CA -> Blank

@mholt
Copy link
Member

mholt commented Jul 9, 2020

@aduzsardi The issue was fixed, see above. Just not in the way that you want (for no reason other than "looks silly" -- if that's your gripe, please file an issue with the certificate viewing software instead).

@polarathene
Copy link

@mholt I just updated to latest Docker tag from 6 hours ago. This is still 2.1.1, so perhaps not?

caddy version
v2.1.1 h1:X9k1+ehZPYYrSqBvf/ocUgdLSRIuiNiMo7CvyGUQKeA=

I have imported the root.crt and intermediate.crt into my local OS trust store (where they've been converted to caddy PEM files), as well as used Chrome settings UI for security to import the authority (not sure if intermediate import was required but did that as well).

Still fails to be recognized as secure. Firefox accepts it. Providing leaf cert I generate from mkcert for Caddy to use is fine in Chrome.


While I don't know if it's any different with the fix, I also agree it's not great for Firefox or Chrome UI detail cert view, as the leaf cert is literally an invisible/blank field(in Firefox it's indicated via tabs, in Chrome it's a tree view of the cert chain). CommonName might not be required but at least provides a better UX for humans inspecting the certificate chain via browsers UI.

@francislavoie
Copy link
Member

We don't publish beta and RC versions to Docker.

You can build the latest from source though, with xcaddy and the golang image as a base.

@polarathene
Copy link

I am fine using mkcert for now, I'll wait for 2.2 whenever that is. I just wanted confirmation if it was going to be a bug fix pushed to 2.1, all good 👍

@thomaskonrad
Copy link

Is this supposed to be fixed in v2.2.0-rc.3? I just tried this version, and I still experience the same issue in Chromium (NET::ERR_CERT_INVALID).

@mholt
Copy link
Member

mholt commented Sep 24, 2020

If you do caddy untrust and then caddy trust, do you get any errors or warnings? (I recently encountered something similar but I think I know what the problem is.)

@aduzsardi
Copy link

While I don't know if it's any different with the fix, I also agree it's not great for Firefox or Chrome UI detail cert view, as the leaf cert is literally an invisible/blank field(in Firefox it's indicated via tabs, in Chrome it's a tree view of the cert chain). CommonName might not be required but at least provides a better UX for humans inspecting the certificate chain via browsers UI.

Thank you for putting this in a better argument that i could , just because something is not required per say , doesn't mean it has no value. :)

@mholt
Copy link
Member

mholt commented Sep 25, 2020

@aduzsardi Using the CommonName field after it has been deprecated is incorrect software; if it looks weird in the cert viewer, the cert viewer will need to fix that. Please focus these complaints in the proper projects where it can be fixed.

@caddyserver caddyserver locked as resolved and limited conversation to collaborators Sep 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug 🐞 Something isn't working upstream ⬆️ Relates to some dependency of this project
Projects
None yet
Development

No branches or pull requests

7 participants