From 4736cccda102f5aca99ee77d0d3b6456ebb5945f Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Wed, 16 Oct 2019 20:31:46 +0200 Subject: [PATCH] all: Update certificate lifetimes (fixes #6036) (#6078) This adds a certificate lifetime parameter to our certificate generation and hard codes it to twenty years in some uninteresting places. In the main binary there are a couple of constants but it results in twenty years for the device certificate and 820 days for the HTTPS one. 820 is less than the 825 maximum Apple allows nowadays. This also means we must be prepared for certificates to expire, so I add some handling for that and generate a new certificate when needed. For self signed certificates we regenerate a month ahead of time. For other certificates we leave well enough alone. --- cmd/stdiscosrv/main.go | 2 +- cmd/strelaypoolsrv/main.go | 2 +- cmd/strelaysrv/main.go | 2 +- cmd/syncthing/main.go | 13 ++++---- lib/api/api.go | 61 ++++++++++++++++++++++++++++++++++--- lib/api/api_test.go | 39 ++++++++++++++++++++++++ lib/discover/global_test.go | 4 +-- lib/syncthing/syncthing.go | 11 ++++--- lib/syncthing/utils.go | 1 + lib/tlsutil/tlsutil.go | 8 +++-- 10 files changed, 119 insertions(+), 24 deletions(-) diff --git a/cmd/stdiscosrv/main.go b/cmd/stdiscosrv/main.go index ab7d1de9..8224b1b9 100644 --- a/cmd/stdiscosrv/main.go +++ b/cmd/stdiscosrv/main.go @@ -100,7 +100,7 @@ func main() { cert, err := tls.LoadX509KeyPair(certFile, keyFile) if err != nil { log.Println("Failed to load keypair. Generating one, this might take a while...") - cert, err = tlsutil.NewCertificate(certFile, keyFile, "stdiscosrv") + cert, err = tlsutil.NewCertificate(certFile, keyFile, "stdiscosrv", 20*365) if err != nil { log.Fatalln("Failed to generate X509 key pair:", err) } diff --git a/cmd/strelaypoolsrv/main.go b/cmd/strelaypoolsrv/main.go index 1f0c9c32..1d3bb37e 100644 --- a/cmd/strelaypoolsrv/main.go +++ b/cmd/strelaypoolsrv/main.go @@ -633,7 +633,7 @@ func createTestCertificate() tls.Certificate { } certFile, keyFile := filepath.Join(tmpDir, "cert.pem"), filepath.Join(tmpDir, "key.pem") - cert, err := tlsutil.NewCertificate(certFile, keyFile, "relaypoolsrv") + cert, err := tlsutil.NewCertificate(certFile, keyFile, "relaypoolsrv", 20*365) if err != nil { log.Fatalln("Failed to create test X509 key pair:", err) } diff --git a/cmd/strelaysrv/main.go b/cmd/strelaysrv/main.go index 09c72875..2300f105 100644 --- a/cmd/strelaysrv/main.go +++ b/cmd/strelaysrv/main.go @@ -155,7 +155,7 @@ func main() { cert, err := tls.LoadX509KeyPair(certFile, keyFile) if err != nil { log.Println("Failed to load keypair. Generating one, this might take a while...") - cert, err = tlsutil.NewCertificate(certFile, keyFile, "strelaysrv") + cert, err = tlsutil.NewCertificate(certFile, keyFile, "strelaysrv", 20*365) if err != nil { log.Fatalln("Failed to generate X509 key pair:", err) } diff --git a/cmd/syncthing/main.go b/cmd/syncthing/main.go index a2e7664d..7082793a 100644 --- a/cmd/syncthing/main.go +++ b/cmd/syncthing/main.go @@ -53,11 +53,12 @@ const ( ) const ( - bepProtocolName = "bep/1.0" - tlsDefaultCommonName = "syncthing" - maxSystemErrors = 5 - initialSystemLog = 10 - maxSystemLog = 250 + bepProtocolName = "bep/1.0" + tlsDefaultCommonName = "syncthing" + maxSystemErrors = 5 + initialSystemLog = 10 + maxSystemLog = 250 + deviceCertLifetimeDays = 20 * 365 ) const ( @@ -424,7 +425,7 @@ func generate(generateDir string) error { if err == nil { l.Warnln("Key exists; will not overwrite.") } else { - cert, err = tlsutil.NewCertificate(certFile, keyFile, tlsDefaultCommonName) + cert, err = tlsutil.NewCertificate(certFile, keyFile, tlsDefaultCommonName, deviceCertLifetimeDays) if err != nil { return errors.Wrap(err, "create certificate") } diff --git a/lib/api/api.go b/lib/api/api.go index 98618c3a..e7a4b83b 100644 --- a/lib/api/api.go +++ b/lib/api/api.go @@ -9,7 +9,9 @@ package api import ( "bytes" "crypto/tls" + "crypto/x509" "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -56,10 +58,11 @@ import ( var bcryptExpr = regexp.MustCompile(`^\$2[aby]\$\d+\$.{50,}`) const ( - DefaultEventMask = events.AllEvents &^ events.LocalChangeDetected &^ events.RemoteChangeDetected - DiskEventMask = events.LocalChangeDetected | events.RemoteChangeDetected - EventSubBufferSize = 1000 - defaultEventTimeout = time.Minute + DefaultEventMask = events.AllEvents &^ events.LocalChangeDetected &^ events.RemoteChangeDetected + DiskEventMask = events.LocalChangeDetected | events.RemoteChangeDetected + EventSubBufferSize = 1000 + defaultEventTimeout = time.Minute + httpsCertLifetimeDays = 820 ) type service struct { @@ -146,6 +149,12 @@ func (s *service) getListener(guiCfg config.GUIConfiguration) (net.Listener, err httpsCertFile := locations.Get(locations.HTTPSCertFile) httpsKeyFile := locations.Get(locations.HTTPSKeyFile) cert, err := tls.LoadX509KeyPair(httpsCertFile, httpsKeyFile) + + // 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) + } if err != nil { l.Infoln("Loading HTTPS certificate:", err) l.Infoln("Creating new HTTPS certificate") @@ -158,7 +167,7 @@ func (s *service) getListener(guiCfg config.GUIConfiguration) (net.Listener, err name = s.tlsDefaultCommonName } - cert, err = tlsutil.NewCertificate(httpsCertFile, httpsKeyFile, name) + cert, err = tlsutil.NewCertificate(httpsCertFile, httpsKeyFile, name, httpsCertLifetimeDays) } if err != nil { return nil, err @@ -1672,3 +1681,45 @@ func addressIsLocalhost(addr string) bool { return ip.IsLoopback() } } + +func checkExpiry(cert tls.Certificate) error { + leaf := cert.Leaf + if leaf == nil { + // Leaf can be nil or not, depending on how parsed the certificate + // was when we got it. + if len(cert.Certificate) < 1 { + // can't happen + return errors.New("no certificate in certificate") + } + var err error + leaf, err = x509.ParseCertificate(cert.Certificate[0]) + if err != nil { + return err + } + } + + 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 + // add, so we leave it alone. + return nil + } + + if leaf.NotAfter.Before(time.Now()) { + return errors.New("certificate has expired") + } + if leaf.NotAfter.Before(time.Now().Add(30 * 24 * time.Hour)) { + return errors.New("certificate will soon expire") + } + + // On macOS, check for certificates issued on or after July 1st, 2019, + // with a longer validity time than 825 days. + cutoff := time.Date(2019, 7, 1, 0, 0, 0, 0, time.UTC) + if runtime.GOOS == "darwin" && + leaf.NotBefore.After(cutoff) && + leaf.NotAfter.Sub(leaf.NotBefore) > 825*24*time.Hour { + return errors.New("certificate incompatible with macOS 10.15 (Catalina)") + } + + return nil +} diff --git a/lib/api/api_test.go b/lib/api/api_test.go index 13d1b738..bea0bced 100644 --- a/lib/api/api_test.go +++ b/lib/api/api_test.go @@ -32,6 +32,7 @@ import ( "github.com/syncthing/syncthing/lib/model" "github.com/syncthing/syncthing/lib/protocol" "github.com/syncthing/syncthing/lib/sync" + "github.com/syncthing/syncthing/lib/tlsutil" "github.com/syncthing/syncthing/lib/ur" "github.com/thejerf/suture" ) @@ -1119,6 +1120,44 @@ func TestPrefixMatch(t *testing.T) { } } +func TestCheckExpiry(t *testing.T) { + dir, err := ioutil.TempDir("", "syncthing-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + // Self signed certificates expiring in less than a month are errored so we + // can regenerate in time. + crt, err := tlsutil.NewCertificate(filepath.Join(dir, "crt"), filepath.Join(dir, "key"), "foo.example.com", 29) + if err != nil { + t.Fatal(err) + } + if err := checkExpiry(crt); err == nil { + t.Error("expected expiry error") + } + + // Certificates with at least 31 days of life left are fine. + crt, err = tlsutil.NewCertificate(filepath.Join(dir, "crt"), filepath.Join(dir, "key"), "foo.example.com", 31) + if err != nil { + t.Fatal(err) + } + if err := checkExpiry(crt); err != nil { + t.Error("expected no error:", err) + } + + if runtime.GOOS == "darwin" { + // Certificates with too long an expiry time are not allowed on macOS + crt, err = tlsutil.NewCertificate(filepath.Join(dir, "crt"), filepath.Join(dir, "key"), "foo.example.com", 1000) + if err != nil { + t.Fatal(err) + } + if err := checkExpiry(crt); err == nil { + t.Error("expected expiry error") + } + } +} + func equalStrings(a, b []string) bool { if len(a) != len(b) { return false diff --git a/lib/discover/global_test.go b/lib/discover/global_test.go index 45621749..23b328eb 100644 --- a/lib/discover/global_test.go +++ b/lib/discover/global_test.go @@ -112,7 +112,7 @@ func TestGlobalOverHTTPS(t *testing.T) { } // Generate a server certificate. - cert, err := tlsutil.NewCertificate(dir+"/cert.pem", dir+"/key.pem", "syncthing") + cert, err := tlsutil.NewCertificate(dir+"/cert.pem", dir+"/key.pem", "syncthing", 30) if err != nil { t.Fatal(err) } @@ -177,7 +177,7 @@ func TestGlobalAnnounce(t *testing.T) { } // Generate a server certificate. - cert, err := tlsutil.NewCertificate(dir+"/cert.pem", dir+"/key.pem", "syncthing") + cert, err := tlsutil.NewCertificate(dir+"/cert.pem", dir+"/key.pem", "syncthing", 30) if err != nil { t.Fatal(err) } diff --git a/lib/syncthing/syncthing.go b/lib/syncthing/syncthing.go index a0c07b0e..0b11d1d1 100644 --- a/lib/syncthing/syncthing.go +++ b/lib/syncthing/syncthing.go @@ -37,11 +37,12 @@ import ( ) const ( - bepProtocolName = "bep/1.0" - tlsDefaultCommonName = "syncthing" - maxSystemErrors = 5 - initialSystemLog = 10 - maxSystemLog = 250 + bepProtocolName = "bep/1.0" + tlsDefaultCommonName = "syncthing" + maxSystemErrors = 5 + initialSystemLog = 10 + maxSystemLog = 250 + deviceCertLifetimeDays = 20 * 365 ) type ExitStatus int diff --git a/lib/syncthing/utils.go b/lib/syncthing/utils.go index 4ad6657e..2070e786 100644 --- a/lib/syncthing/utils.go +++ b/lib/syncthing/utils.go @@ -35,6 +35,7 @@ func LoadOrGenerateCertificate(certFile, keyFile string) (tls.Certificate, error locations.Get(locations.CertFile), locations.Get(locations.KeyFile), tlsDefaultCommonName, + deviceCertLifetimeDays, ) } return cert, nil diff --git a/lib/tlsutil/tlsutil.go b/lib/tlsutil/tlsutil.go index 6bb3a42f..0bce3566 100644 --- a/lib/tlsutil/tlsutil.go +++ b/lib/tlsutil/tlsutil.go @@ -95,15 +95,17 @@ func SecureDefault() *tls.Config { } // NewCertificate generates and returns a new TLS certificate. -func NewCertificate(certFile, keyFile, commonName string) (tls.Certificate, error) { +func NewCertificate(certFile, keyFile, commonName string, lifetimeDays int) (tls.Certificate, error) { priv, err := ecdsa.GenerateKey(elliptic.P384(), rand.Reader) if err != nil { return tls.Certificate{}, fmt.Errorf("generate key: %s", err) } - notBefore := time.Now() - notAfter := time.Date(2049, 12, 31, 23, 59, 59, 0, time.UTC) + notBefore := time.Now().Truncate(24 * time.Hour) + notAfter := notBefore.Add(time.Duration(lifetimeDays*24) * time.Hour) + // NOTE: update checkExpiry() appropriately if you add or change attributes + // in here, especially DNSNames or IPAddresses. template := x509.Certificate{ SerialNumber: new(big.Int).SetInt64(rand.Int63()), Subject: pkix.Name{