From c979cde002597d287f3dc560b9dbefc6c9d26f08 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Thu, 17 Aug 2023 12:38:48 +0100 Subject: [PATCH] ftp: fix 425 "TLS session of data connection not resumed" errors As an extra security feature some FTP servers (eg FileZilla) require that the data connection re-use the same TLS connection as the control connection. This is a good thing for security. The message "TLS session of data connection not resumed" means that it was not done. The problem turned out to be that rclone was re-using the TLS session cache between concurrent connections so the resumed TLS data connection could from any of the control connections. This patch makes each TLS connection have its own session cache which should fix the problem. This also reverts the ftp library to the upstream version which now contains all of our patches. Fixes #7234 --- backend/ftp/ftp.go | 51 ++++++++++++++++++++++++++++------------------ go.mod | 3 +-- go.sum | 6 ++---- 3 files changed, 34 insertions(+), 26 deletions(-) diff --git a/backend/ftp/ftp.go b/backend/ftp/ftp.go index 1aa646bef..aab419779 100644 --- a/backend/ftp/ftp.go +++ b/backend/ftp/ftp.go @@ -15,7 +15,7 @@ import ( "sync" "time" - "github.com/rclone/ftp" + "github.com/jlaffaye/ftp" "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/accounting" "github.com/rclone/rclone/fs/config" @@ -249,7 +249,6 @@ type Fs struct { pool []*ftp.ServerConn drain *time.Timer // used to drain the pool when we stop using the connections tokens *pacer.TokenDispenser - tlsConf *tls.Config pacer *fs.Pacer // pacer for FTP connections fGetTime bool // true if the ftp library accepts GetTime fSetTime bool // true if the ftp library accepts SetTime @@ -362,10 +361,36 @@ func shouldRetry(ctx context.Context, err error) (bool, error) { return fserrors.ShouldRetry(err), err } +// Get a TLS config with a unique session cache. +// +// We can't share session caches between connections. +// +// See: https://github.com/rclone/rclone/issues/7234 +func (f *Fs) tlsConfig() *tls.Config { + var tlsConfig *tls.Config + if f.opt.TLS || f.opt.ExplicitTLS { + tlsConfig = &tls.Config{ + ServerName: f.opt.Host, + InsecureSkipVerify: f.opt.SkipVerifyTLSCert, + } + if f.opt.TLSCacheSize > 0 { + tlsConfig.ClientSessionCache = tls.NewLRUClientSessionCache(f.opt.TLSCacheSize) + } + if f.opt.DisableTLS13 { + tlsConfig.MaxVersion = tls.VersionTLS12 + } + } + return tlsConfig +} + // Open a new connection to the FTP server. func (f *Fs) ftpConnection(ctx context.Context) (c *ftp.ServerConn, err error) { fs.Debugf(f, "Connecting to FTP server") + // tls.Config for this connection only. Will be used for data + // and control connections. + tlsConfig := f.tlsConfig() + // Make ftp library dial with fshttp dialer optionally using TLS initialConnection := true dial := func(network, address string) (conn net.Conn, err error) { @@ -383,7 +408,7 @@ func (f *Fs) ftpConnection(ctx context.Context) (c *ftp.ServerConn, err error) { return nil, err } // Connect using cleartext only for non TLS - if f.tlsConf == nil { + if tlsConfig == nil { return conn, nil } // Initial connection only needs to be cleartext for explicit TLS @@ -392,7 +417,7 @@ func (f *Fs) ftpConnection(ctx context.Context) (c *ftp.ServerConn, err error) { return conn, nil } // Upgrade connection to TLS - tlsConn := tls.Client(conn, f.tlsConf) + tlsConn := tls.Client(conn, tlsConfig) // Do the initial handshake - tls.Client doesn't do it for us // If we do this then connections to proftpd/pureftpd lock up // See: https://github.com/rclone/rclone/issues/6426 @@ -414,9 +439,9 @@ func (f *Fs) ftpConnection(ctx context.Context) (c *ftp.ServerConn, err error) { if f.opt.TLS { // Our dialer takes care of TLS but ftp library also needs tlsConf // as a trigger for sending PSBZ and PROT options to server. - ftpConfig = append(ftpConfig, ftp.DialWithTLS(f.tlsConf)) + ftpConfig = append(ftpConfig, ftp.DialWithTLS(tlsConfig)) } else if f.opt.ExplicitTLS { - ftpConfig = append(ftpConfig, ftp.DialWithExplicitTLS(f.tlsConf)) + ftpConfig = append(ftpConfig, ftp.DialWithExplicitTLS(tlsConfig)) } if f.opt.DisableEPSV { ftpConfig = append(ftpConfig, ftp.DialWithDisabledEPSV(true)) @@ -571,19 +596,6 @@ func NewFs(ctx context.Context, name, root string, m configmap.Mapper) (ff fs.Fs if opt.TLS && opt.ExplicitTLS { return nil, errors.New("implicit TLS and explicit TLS are mutually incompatible, please revise your config") } - var tlsConfig *tls.Config - if opt.TLS || opt.ExplicitTLS { - tlsConfig = &tls.Config{ - ServerName: opt.Host, - InsecureSkipVerify: opt.SkipVerifyTLSCert, - } - if opt.TLSCacheSize > 0 { - tlsConfig.ClientSessionCache = tls.NewLRUClientSessionCache(opt.TLSCacheSize) - } - if opt.DisableTLS13 { - tlsConfig.MaxVersion = tls.VersionTLS12 - } - } u := protocol + path.Join(dialAddr+"/", root) ci := fs.GetConfig(ctx) f := &Fs{ @@ -596,7 +608,6 @@ func NewFs(ctx context.Context, name, root string, m configmap.Mapper) (ff fs.Fs pass: pass, dialAddr: dialAddr, tokens: pacer.NewTokenDispenser(opt.Concurrency), - tlsConf: tlsConfig, pacer: fs.NewPacer(ctx, pacer.NewDefault(pacer.MinSleep(minSleep), pacer.MaxSleep(maxSleep), pacer.DecayConstant(decayConstant))), } f.features = (&fs.Features{ diff --git a/go.mod b/go.mod index d69e08373..dcc6ffe4e 100644 --- a/go.mod +++ b/go.mod @@ -33,6 +33,7 @@ require ( github.com/hirochachacha/go-smb2 v1.1.0 github.com/iguanesolutions/go-systemd/v5 v5.1.1 github.com/jcmturner/gokrb5/v8 v8.4.4 + github.com/jlaffaye/ftp v0.2.0 github.com/jzelinskie/whirlpool v0.0.0-20201016144138-0675e54bb004 github.com/klauspost/compress v1.16.7 github.com/koofr/go-httpclient v0.0.0-20230225102643-5d51a2e9dea6 @@ -49,7 +50,6 @@ require ( github.com/pmezard/go-difflib v1.0.0 github.com/prometheus/client_golang v1.15.1 github.com/putdotio/go-putio/putio v0.0.0-20200123120452-16d982cac2b8 - github.com/rclone/ftp v0.0.0-20230327202000-dadc1f64e87d github.com/rfjakob/eme v1.1.2 github.com/rivo/uniseg v0.4.4 github.com/shirou/gopsutil/v3 v3.23.6 @@ -121,7 +121,6 @@ require ( github.com/jcmturner/gofork v1.7.6 // indirect github.com/jcmturner/goidentity/v6 v6.0.1 // indirect github.com/jcmturner/rpc/v2 v2.0.3 // indirect - github.com/jlaffaye/ftp v0.1.1-0.20230214004652-d84bf4be2b6e // indirect github.com/jmespath/go-jmespath v0.4.0 // indirect github.com/jtolio/eventkit v0.0.0-20221004135224-074cf276595b // indirect github.com/jtolio/noiseconn v0.0.0-20230111204749-d7ec1a08b0b8 // indirect diff --git a/go.sum b/go.sum index 1169d457c..24259f3e2 100644 --- a/go.sum +++ b/go.sum @@ -322,8 +322,8 @@ github.com/jcmturner/gokrb5/v8 v8.4.4/go.mod h1:1btQEpgT6k+unzCwX1KdWMEwPPkkgBtP github.com/jcmturner/rpc/v2 v2.0.3 h1:7FXXj8Ti1IaVFpSAziCZWNzbNuZmnvw/i6CqLNdWfZY= github.com/jcmturner/rpc/v2 v2.0.3/go.mod h1:VUJYCIDm3PVOEHw8sgt091/20OJjskO/YJki3ELg/Hc= github.com/jlaffaye/ftp v0.0.0-20190624084859-c1312a7102bf/go.mod h1:lli8NYPQOFy3O++YmYbqVgOcQ1JPCwdOy+5zSjKJ9qY= -github.com/jlaffaye/ftp v0.1.1-0.20230214004652-d84bf4be2b6e h1:Xofa5zcfulLjSb9ZNpb7MI9TFCpVkPCy3JSwrL7xoWE= -github.com/jlaffaye/ftp v0.1.1-0.20230214004652-d84bf4be2b6e/go.mod h1:sRSt+7UoQ5BgrZhwta4kr7N5SenQsoIZHMJHY7+zqJg= +github.com/jlaffaye/ftp v0.2.0 h1:lXNvW7cBu7R/68bknOX3MrRIIqZ61zELs1P2RAiA3lg= +github.com/jlaffaye/ftp v0.2.0/go.mod h1:is2Ds5qkhceAPy2xD6RLI6hmp/qysSoymZ+Z2uTnspI= github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg= github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo= github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8= @@ -436,8 +436,6 @@ github.com/prometheus/procfs v0.11.1 h1:xRC8Iq1yyca5ypa9n1EZnWZkt7dwcoRPQwX/5gwa github.com/prometheus/procfs v0.11.1/go.mod h1:eesXgaPo1q7lBpVMoMy0ZOFTth9hBn4W/y0/p/ScXhY= github.com/putdotio/go-putio/putio v0.0.0-20200123120452-16d982cac2b8 h1:Y258uzXU/potCYnQd1r6wlAnoMB68BiCkCcCnKx1SH8= github.com/putdotio/go-putio/putio v0.0.0-20200123120452-16d982cac2b8/go.mod h1:bSJjRokAHHOhA+XFxplld8w2R/dXLH7Z3BZ532vhFwU= -github.com/rclone/ftp v0.0.0-20230327202000-dadc1f64e87d h1:ZyH6ZfA/PzxF4qQS2MgFLXRdw/pWOSNJA7Lq0pkX49Y= -github.com/rclone/ftp v0.0.0-20230327202000-dadc1f64e87d/go.mod h1:mWj8othLks994zO7BLHHfh9cpj1eM1n7XqWvX+DM6ic= github.com/relvacode/iso8601 v1.3.0 h1:HguUjsGpIMh/zsTczGN3DVJFxTU/GX+MMmzcKoMO7ko= github.com/relvacode/iso8601 v1.3.0/go.mod h1:FlNp+jz+TXpyRqgmM7tnzHHzBnz776kmAH2h3sZCn0I= github.com/rfjakob/eme v1.1.2 h1:SxziR8msSOElPayZNFfQw4Tjx/Sbaeeh3eRvrHVMUs4=