lib/connections, lib/tlsutil: Handle certName in Go 1.15 (fixes #6867) (#6868)

Our authentication is based on device ID (certificate fingerprint) but
we also check the certificate name for ... historical extra security
reasons. (I don't think this adds anything but it is what it is.) Since
that check breaks in Go 1.15 this change does two things:

- Adds a manual check for the peer certificate CommonName, and if they
  are equal we are happy and don't call the more advanced
  VerifyHostname() function. This allows our old style certificates to
  still pass the check.

- Adds the cert name "syncthing" as a DNS SAN when generating the
  certificate. This is the correct way nowadays and makes VerifyHostname()
  happy in Go 1.15 as well, even without the above patch.

lib/api/api.go

@@ -149,7 +149,7 @@ func (s *service) getListener(guiCfg config.GUIConfiguration) (net.Listener, err
 	// If the certificate has expired or will expire in the next month, fail
 	// it and generate a new one.
 	if err == nil {
-		err = checkExpiry(cert)
+		err = shouldRegenerateCertificate(cert)
 	}
 	if err != nil {
 		l.Infoln("Loading HTTPS certificate:", err)
@@ -1736,7 +1736,11 @@ func addressIsLocalhost(addr string) bool {
 	}
 }
 
-func checkExpiry(cert tls.Certificate) error {
+// shouldRegenerateCertificate checks for certificate expiry or other known
+// issues with our API/GUI certificate and returns either nil (leave the
+// certificate alone) or an error describing the reason the certificate
+// should be regenerated.
+func shouldRegenerateCertificate(cert tls.Certificate) error {
 	leaf := cert.Leaf
 	if leaf == nil {
 		// Leaf can be nil or not, depending on how parsed the certificate
@@ -1752,12 +1756,21 @@ func checkExpiry(cert tls.Certificate) error {
 		}
 	}
 
-	if leaf.Subject.String() != leaf.Issuer.String() ||
-		len(leaf.DNSNames) != 0 || len(leaf.IPAddresses) != 0 {
-		// The certificate is not self signed, or has DNS/IP attributes we don't
+	if leaf.Subject.String() != leaf.Issuer.String() || len(leaf.IPAddresses) != 0 {
+		// The certificate is not self signed, or has IP attributes we don't
 		// add, so we leave it alone.
 		return nil
 	}
+	if len(leaf.DNSNames) > 1 {
+		// The certificate has more DNS SANs attributes than we ever add, so
+		// we leave it alone.
+		return nil
+	}
+	if len(leaf.DNSNames) == 1 && leaf.DNSNames[0] != leaf.Issuer.CommonName {
+		// The one SAN is different from the issuer, so it's not one of our
+		// newer self signed certificates.
+		return nil
+	}
 
 	if leaf.NotAfter.Before(time.Now()) {
 		return errors.New("certificate has expired")

lib/api/api_test.go

@@ -1136,7 +1136,7 @@ func TestPrefixMatch(t *testing.T) {
 	}
 }
 
-func TestCheckExpiry(t *testing.T) {
+func TestShouldRegenerateCertificate(t *testing.T) {
 	dir, err := ioutil.TempDir("", "syncthing-test")
 	if err != nil {
 		t.Fatal(err)
@@ -1149,7 +1149,7 @@ func TestCheckExpiry(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := checkExpiry(crt); err == nil {
+	if err := shouldRegenerateCertificate(crt); err == nil {
 		t.Error("expected expiry error")
 	}
 
@@ -1158,7 +1158,7 @@ func TestCheckExpiry(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := checkExpiry(crt); err != nil {
+	if err := shouldRegenerateCertificate(crt); err != nil {
 		t.Error("expected no error:", err)
 	}
 
@@ -1168,7 +1168,7 @@ func TestCheckExpiry(t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
-		if err := checkExpiry(crt); err == nil {
+		if err := shouldRegenerateCertificate(crt); err == nil {
 			t.Error("expected expiry error")
 		}
 	}

lib/connections/service.go

@@ -305,7 +305,11 @@ func (s *service) handle(ctx context.Context) {
 		if certName == "" {
 			certName = s.tlsDefaultCommonName
 		}
-		if err := remoteCert.VerifyHostname(certName); err != nil {
+		if remoteCert.Subject.CommonName == certName {
+			// All good. We do this check because our old style certificates
+			// have "syncthing" in the CommonName field and no SANs, which
+			// is not accepted by VerifyHostname() any more as of Go 1.15.
+		} else if err := remoteCert.VerifyHostname(certName); err != nil {
 			// Incorrect certificate name is something the user most
 			// likely wants to know about, since it's an advanced
 			// config. Warn instead of Info.

lib/tlsutil/tlsutil.go

@@ -106,6 +106,7 @@ func NewCertificate(certFile, keyFile, commonName string, lifetimeDays int) (tls
 		Subject: pkix.Name{
 			CommonName: commonName,
 		},
+		DNSNames:              []string{commonName},
 		NotBefore:             notBefore,
 		NotAfter:              notAfter,
 		SignatureAlgorithm:    x509.ECDSAWithSHA256,