Attachment #217476 for bug #248867

View | Details | Raw Unified | Return to bug 248867
Collapse All | Expand All

Lines 2-7 Link Here

(-)net/syncthing/Makefile (+1 lines)
2		2
3	PORTNAME= syncthing	3	PORTNAME= syncthing
4	PORTVERSION= 1.8.0	4	PORTVERSION= 1.8.0
		5	PORTREVISION= 1
5	DISTVERSIONPREFIX= v	6	DISTVERSIONPREFIX= v
6	CATEGORIES= net	7	CATEGORIES= net
7	MASTER_SITES= https://github.com/syncthing/syncthing/releases/download/v${PORTVERSION}/	8	MASTER_SITES= https://github.com/syncthing/syncthing/releases/download/v${PORTVERSION}/




--- syncthing/lib/api/api.go.orig	2020-08-11 08:56:46 UTC
+++ syncthing/lib/api/api.go
@@ -149,7 +149,7 @@ func (s *service) getListener(guiCfg config.GUIConfigu
 	// 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,10 +1756,19 @@ 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
 	}
 




--- syncthing/lib/api/api_test.go.orig	2020-08-11 08:56:46 UTC
+++ syncthing/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")
 		}
 	}




--- syncthing/lib/connections/service.go.orig	2020-08-11 08:56:46 UTC
+++ syncthing/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.

Line 0 Link Here

(-)net/syncthing/files/patch-syncthing_lib_tlsutil_tlsutil.go (+10 lines)
	1	--- syncthing/lib/tlsutil/tlsutil.go.orig 2020-08-11 08:56:46 UTC
	2	+++ syncthing/lib/tlsutil/tlsutil.go
	3	@@ -106,6 +106,7 @@ func NewCertificate(certFile, keyFile, commonName stri
	4	Subject: pkix.Name{
	5	CommonName: commonName,
	6	},
	7	+ DNSNames: []string{commonName},
	8	NotBefore: notBefore,
	9	NotAfter: notAfter,
	10	SignatureAlgorithm: x509.ECDSAWithSHA256,

Return to bug 248867

Line 0 Link Here

(-)net/syncthing/files/patch-syncthing_lib_api_api.go (+47 lines)
	1	--- syncthing/lib/api/api.go.orig 2020-08-11 08:56:46 UTC
	2	+++ syncthing/lib/api/api.go
	3	@@ -149,7 +149,7 @@ func (s *service) getListener(guiCfg config.GUIConfigu
	4	// If the certificate has expired or will expire in the next month, fail
	5	// it and generate a new one.
	6	if err == nil {
	7	- err = checkExpiry(cert)
	8	+ err = shouldRegenerateCertificate(cert)
	9	}
	10	if err != nil {
	11	l.Infoln("Loading HTTPS certificate:", err)
	12	@@ -1736,7 +1736,11 @@ func addressIsLocalhost(addr string) bool {
	13	}
	14	}
	15
	16	-func checkExpiry(cert tls.Certificate) error {
	17	+// shouldRegenerateCertificate checks for certificate expiry or other known
	18	+// issues with our API/GUI certificate and returns either nil (leave the
	19	+// certificate alone) or an error describing the reason the certificate
	20	+// should be regenerated.
	21	+func shouldRegenerateCertificate(cert tls.Certificate) error {
	22	leaf := cert.Leaf
	23	if leaf == nil {
	24	// Leaf can be nil or not, depending on how parsed the certificate
	25	@@ -1752,10 +1756,19 @@ func checkExpiry(cert tls.Certificate) error {
	26	}
	27	}
	28
	29	- if leaf.Subject.String() != leaf.Issuer.String() \|\|
	30	- len(leaf.DNSNames) != 0 \|\| len(leaf.IPAddresses) != 0 {
	31	- // The certificate is not self signed, or has DNS/IP attributes we don't
	32	+ if leaf.Subject.String() != leaf.Issuer.String() \|\| len(leaf.IPAddresses) != 0 {
	33	+ // The certificate is not self signed, or has IP attributes we don't
	34	// add, so we leave it alone.
	35	+ return nil
	36	+ }
	37	+ if len(leaf.DNSNames) > 1 {
	38	+ // The certificate has more DNS SANs attributes than we ever add, so
	39	+ // we leave it alone.
	40	+ return nil
	41	+ }
	42	+ if len(leaf.DNSNames) == 1 && leaf.DNSNames[0] != leaf.Issuer.CommonName {
	43	+ // The one SAN is different from the issuer, so it's not one of our
	44	+ // newer self signed certificates.
	45	return nil
	46	}
	47

Line 0 Link Here

(-)net/syncthing/files/patch-syncthing_lib_api_api__test.go (+38 lines)
	1	--- syncthing/lib/api/api_test.go.orig 2020-08-11 08:56:46 UTC
	2	+++ syncthing/lib/api/api_test.go
	3	@@ -1136,7 +1136,7 @@ func TestPrefixMatch(t *testing.T) {
	4	}
	5	}
	6
	7	-func TestCheckExpiry(t *testing.T) {
	8	+func TestShouldRegenerateCertificate(t *testing.T) {
	9	dir, err := ioutil.TempDir("", "syncthing-test")
	10	if err != nil {
	11	t.Fatal(err)
	12	@@ -1149,7 +1149,7 @@ func TestCheckExpiry(t *testing.T) {
	13	if err != nil {
	14	t.Fatal(err)
	15	}
	16	- if err := checkExpiry(crt); err == nil {
	17	+ if err := shouldRegenerateCertificate(crt); err == nil {
	18	t.Error("expected expiry error")
	19	}
	20
	21	@@ -1158,7 +1158,7 @@ func TestCheckExpiry(t *testing.T) {
	22	if err != nil {
	23	t.Fatal(err)
	24	}
	25	- if err := checkExpiry(crt); err != nil {
	26	+ if err := shouldRegenerateCertificate(crt); err != nil {
	27	t.Error("expected no error:", err)
	28	}
	29
	30	@@ -1168,7 +1168,7 @@ func TestCheckExpiry(t *testing.T) {
	31	if err != nil {
	32	t.Fatal(err)
	33	}
	34	- if err := checkExpiry(crt); err == nil {
	35	+ if err := shouldRegenerateCertificate(crt); err == nil {
	36	t.Error("expected expiry error")
	37	}
	38	}

Line 0 Link Here

(-)net/syncthing/files/patch-syncthing_lib_connections_service.go (+15 lines)
	1	--- syncthing/lib/connections/service.go.orig 2020-08-11 08:56:46 UTC
	2	+++ syncthing/lib/connections/service.go
	3	@@ -305,7 +305,11 @@ func (s *service) handle(ctx context.Context) {
	4	if certName == "" {
	5	certName = s.tlsDefaultCommonName
	6	}
	7	- if err := remoteCert.VerifyHostname(certName); err != nil {
	8	+ if remoteCert.Subject.CommonName == certName {
	9	+ // All good. We do this check because our old style certificates
	10	+ // have "syncthing" in the CommonName field and no SANs, which
	11	+ // is not accepted by VerifyHostname() any more as of Go 1.15.
	12	+ } else if err := remoteCert.VerifyHostname(certName); err != nil {
	13	// Incorrect certificate name is something the user most
	14	// likely wants to know about, since it's an advanced
	15	// config. Warn instead of Info.