Fix self-signed cert validity on macOS systems#32698
Conversation
|
Looks like teleterm is not happy with this change? @ravicious any ideas? |
reedloden
left a comment
reedloden
left a comment
There was a problem hiding this comment.
Are there any other places where this needs to be updated to better future-proof things? I see that we do self-signed certs in other places as well, but not sure if they would be TLS server certs.
|
Safari is not happy with this either. Refuses to connect and the logs show: 301 is TLS 1.1, which seems odd. |
outputs this:
|
|
The teleterm test fails because in that test we call This ugly patch verifies that passing Patchdiff --git a/lib/teleterm/grpccredentials.go b/lib/teleterm/grpccredentials.go
index 634b219915..93add3d2de 100644
--- a/lib/teleterm/grpccredentials.go
+++ b/lib/teleterm/grpccredentials.go
@@ -136,3 +136,41 @@ func generateAndSaveCert(targetPath string) (tls.Certificate, error) {
return certificate, nil
}
+
+func generateAndSaveClientCert(targetPath string) (tls.Certificate, error) {
+ // The cert is first saved under a temp path and then renamed to targetPath. This prevents other
+ // processes from reading a half-written file.
+ tempFile, err := os.CreateTemp(filepath.Dir(targetPath), filepath.Base(targetPath))
+ if err != nil {
+ return tls.Certificate{}, trace.Wrap(err)
+ }
+ defer os.Remove(tempFile.Name())
+
+ cert, err := cert.GenerateSelfSignedClientCert([]string{"localhost"}, nil)
+ if err != nil {
+ return tls.Certificate{}, trace.Wrap(err, "failed to generate the certificate")
+ }
+
+ if err = tempFile.Chmod(0600); err != nil {
+ return tls.Certificate{}, trace.Wrap(err)
+ }
+
+ if _, err = tempFile.Write(cert.Cert); err != nil {
+ return tls.Certificate{}, trace.Wrap(err)
+ }
+
+ if err = tempFile.Close(); err != nil {
+ return tls.Certificate{}, trace.Wrap(err)
+ }
+
+ if err = os.Rename(tempFile.Name(), targetPath); err != nil {
+ return tls.Certificate{}, trace.Wrap(err)
+ }
+
+ certificate, err := keys.X509KeyPair(cert.Cert, cert.PrivateKey)
+ if err != nil {
+ return tls.Certificate{}, trace.Wrap(err)
+ }
+
+ return certificate, nil
+}
diff --git a/lib/teleterm/teleterm_test.go b/lib/teleterm/teleterm_test.go
index aa703ed5ea..879f6613fe 100644
--- a/lib/teleterm/teleterm_test.go
+++ b/lib/teleterm/teleterm_test.go
@@ -209,7 +209,7 @@ func createValidClientTLSConfig(t *testing.T, certsDir string) *tls.Config {
// reach the tsh gRPC server, so we need to use the renderer cert as the client cert.
clientCertPath := filepath.Join(certsDir, rendererCertFileName)
serverCertPath := filepath.Join(certsDir, tshdCertFileName)
- clientCert, err := generateAndSaveCert(clientCertPath)
+ clientCert, err := generateAndSaveClientCert(clientCertPath)
require.NoError(t, err)
tlsConfig, err := createClientTLSConfig(clientCert, serverCertPath)
diff --git a/lib/utils/cert/selfsigned.go b/lib/utils/cert/selfsigned.go
index 353d068971..696826387e 100644
--- a/lib/utils/cert/selfsigned.go
+++ b/lib/utils/cert/selfsigned.go
@@ -113,3 +113,67 @@ func GenerateSelfSignedCert(hostNames []string, ipAddresses []string) (*Credenti
Cert: pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: derBytes}),
}, nil
}
+
+func GenerateSelfSignedClientCert(hostNames []string, ipAddresses []string) (*Credentials, error) {
+ priv, err := native.GenerateRSAPrivateKey()
+ if err != nil {
+ return nil, trace.Wrap(err)
+ }
+ notBefore := time.Now()
+ notAfter := notBefore.Add(macMaxTLSCertValidityPeriod)
+
+ serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128)
+ serialNumber, err := rand.Int(rand.Reader, serialNumberLimit)
+ if err != nil {
+ return nil, trace.Wrap(err)
+ }
+
+ entity := pkix.Name{
+ CommonName: "localhost",
+ Country: []string{"US"},
+ Organization: []string{"localhost"},
+ }
+
+ template := x509.Certificate{
+ SerialNumber: serialNumber,
+ Issuer: entity,
+ Subject: entity,
+ NotBefore: notBefore,
+ NotAfter: notAfter,
+ KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
+ ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}, // https://support.apple.com/en-in/HT210176
+ BasicConstraintsValid: true,
+ IsCA: true,
+ }
+
+ // collect IP addresses localhost resolves to and add them to the cert. template:
+ template.DNSNames = append(hostNames, "localhost.local")
+ ips, _ := net.LookupIP("localhost")
+ if ips != nil {
+ template.IPAddresses = append(ips, net.ParseIP("::1"))
+ }
+ for _, ip := range ipAddresses {
+ ipParsed := net.ParseIP(ip)
+ if ipParsed == nil {
+ return nil, trace.Errorf("Unable to parse IP address for self-signed certificate (%v)", ip)
+ }
+ template.IPAddresses = append(template.IPAddresses, ipParsed)
+ }
+
+ derBytes, err := x509.CreateCertificate(rand.Reader, &template, &template, &priv.PublicKey, priv)
+ if err != nil {
+ return nil, trace.Wrap(err)
+ }
+
+ publicKeyBytes, err := x509.MarshalPKIXPublicKey(priv.Public())
+ if err != nil {
+ logrus.Error(err)
+ return nil, trace.Wrap(err)
+ }
+
+ return &Credentials{
+ PublicKey: pem.EncodeToMemory(&pem.Block{Type: "RSA PUBLIC KEY", Bytes: publicKeyBytes}),
+ PrivateKey: pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(priv)}),
+ Cert: pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: derBytes}),
+ }, nil
+}
|
|
Oh nice, thanks! I did not know about that GRPC log verbosity env var. |
zmb3
force-pushed
the
zmb3/fix-macos-selfsigned-web-cert
branch
from
de857d8 to
711af0f
Compare
|
Still a little stuck on Safari. Here's what I've found. I added some debug logging to the ALPN proxy's When using Teleport's self-signed cert for the web proxy and attempting to load the page in Safari we see 3 client hellos: The first two seem to make sense. The 3rd connection only advertises support for TLS 1.1 (769 or 0x301), which probably explains the If I use the same certs but attempt to load the page from Chromium, everything works fine. In the logs, I only see the client advertising support for TLS 1.3 and 1.2: Then I added Teleport's self signed proxy to the macOS trust root: Now the UI loads properly and I no longer see connections with support for TLS 1.1 only: |
|
Okay, the Safari issue was just cached HSTS data (it doesn't like that the cert changed). If I open the web UI in a private tab or reset the browser data for this site everything loads fine. @tcsc that means this is ready for review. Thanks :-) |
|
I took another look at this and it made me remember that on Windows we generate a single keypair and then use it as the server cert and the client cert. This was not caught by the test. The test makes a test connection to the gRPC server ran by tsh daemon. But it does not test the other connection where tsh daemon acts as a client and talks with the gRPC server ran from the Electron app. If we were to fix it, I suppose it'd require generating two separate keypairs, saving the public keys under two different paths and then replicating this change in the JS equivalent. It shouldn't be a lot of work, the code is already organized in a way that supports arbitrary number of certs. OTOH I tested this branch on Windows and it all works as expected with no warnings of any kind. I'm partial to just keeping it as it is. Technically, we should then add those EKUs to certs generated from JS as well and this adds some additional work. Alternatively, we could make sure to add EKUs only on macOS. |
|
Thanks @ravicious - those sound like improvements worth making to Teleport Connect but let's keep them out of scope for this change. |
public-teleport-github-review-bot
Bot
removed the request for review
from tcsc
zmb3
force-pushed
the
zmb3/fix-macos-selfsigned-web-cert
branch
from
711af0f to
fc65d1c
Compare
As per https://support.apple.com/en-in/HT210176: > TLS server certificates must contain an ExtendedKeyUsage (EKU) extension containing the id-kp-serverAuth OID. We were not specifying this EKU. Validated by checking with the old self-signed certs: $ security verify-cert -c webproxy_cert.pem -p ssl -r webproxy_cert.pem Cert Verify Result: Invalid Extended Key Usage for policy And then repeating the process after this change: $ security verify-cert -c webproxy_cert.pem -p ssl -r webproxy_cert.pem ...certificate verification successful. Closes #32531
zmb3
force-pushed
the
zmb3/fix-macos-selfsigned-web-cert
branch
from
fc65d1c to
a6960ea
Compare
5 participants
As per https://support.apple.com/en-in/HT210176:
We were not specifying this EKU.
Validated by checking with the old self-signed certs:
And then repeating the process after this change:
Closes #32531