From 0d20d7b68551884e4068a8c1146bc69cf2d59242 Mon Sep 17 00:00:00 2001 From: Steve Wills Date: Mon, 24 Aug 2020 17:50:38 +0000 Subject: [PATCH] net/syncthing: fix SSL errors due to Go 1.15 behaviour change PR: 248867 Submitted by: James French --- net/syncthing/Makefile | 1 + .../files/patch-syncthing_lib_api_api.go | 47 +++++++++++++++++++ .../patch-syncthing_lib_api_api__test.go | 38 +++++++++++++++ ...patch-syncthing_lib_connections_service.go | 15 ++++++ .../patch-syncthing_lib_tlsutil_tlsutil.go | 10 ++++ 5 files changed, 111 insertions(+) create mode 100644 net/syncthing/files/patch-syncthing_lib_api_api.go create mode 100644 net/syncthing/files/patch-syncthing_lib_api_api__test.go create mode 100644 net/syncthing/files/patch-syncthing_lib_connections_service.go create mode 100644 net/syncthing/files/patch-syncthing_lib_tlsutil_tlsutil.go diff --git a/net/syncthing/Makefile b/net/syncthing/Makefile index 1032e16f5e53..a54e13eeb7ca 100644 --- a/net/syncthing/Makefile +++ b/net/syncthing/Makefile @@ -2,6 +2,7 @@ PORTNAME= syncthing PORTVERSION= 1.8.0 +PORTREVISION= 1 DISTVERSIONPREFIX= v CATEGORIES= net MASTER_SITES= https://github.com/syncthing/syncthing/releases/download/v${PORTVERSION}/ diff --git a/net/syncthing/files/patch-syncthing_lib_api_api.go b/net/syncthing/files/patch-syncthing_lib_api_api.go new file mode 100644 index 000000000000..da05a58c45fa --- /dev/null +++ b/net/syncthing/files/patch-syncthing_lib_api_api.go @@ -0,0 +1,47 @@ +--- 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 + } + diff --git a/net/syncthing/files/patch-syncthing_lib_api_api__test.go b/net/syncthing/files/patch-syncthing_lib_api_api__test.go new file mode 100644 index 000000000000..8021148a9379 --- /dev/null +++ b/net/syncthing/files/patch-syncthing_lib_api_api__test.go @@ -0,0 +1,38 @@ +--- 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") + } + } diff --git a/net/syncthing/files/patch-syncthing_lib_connections_service.go b/net/syncthing/files/patch-syncthing_lib_connections_service.go new file mode 100644 index 000000000000..7235d32a89e2 --- /dev/null +++ b/net/syncthing/files/patch-syncthing_lib_connections_service.go @@ -0,0 +1,15 @@ +--- 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. diff --git a/net/syncthing/files/patch-syncthing_lib_tlsutil_tlsutil.go b/net/syncthing/files/patch-syncthing_lib_tlsutil_tlsutil.go new file mode 100644 index 000000000000..8e373fce261e --- /dev/null +++ b/net/syncthing/files/patch-syncthing_lib_tlsutil_tlsutil.go @@ -0,0 +1,10 @@ +--- syncthing/lib/tlsutil/tlsutil.go.orig 2020-08-11 08:56:46 UTC ++++ syncthing/lib/tlsutil/tlsutil.go +@@ -106,6 +106,7 @@ func NewCertificate(certFile, keyFile, commonName stri + Subject: pkix.Name{ + CommonName: commonName, + }, ++ DNSNames: []string{commonName}, + NotBefore: notBefore, + NotAfter: notAfter, + SignatureAlgorithm: x509.ECDSAWithSHA256,