From 654ce2e349fcdd56a1742401e4ebf23bf4c96861 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sun, 5 Nov 2023 19:27:11 +0100 Subject: [PATCH] s3: allow to skip TLS verification Signed-off-by: Nicola Murino --- docs/portable-mode.md | 7 +++++++ go.mod | 2 +- go.sum | 4 ++-- internal/cmd/portable.go | 9 +++++++++ internal/httpd/httpd_test.go | 6 ++++++ internal/httpd/webadmin.go | 1 + internal/httpdtest/httpdtest.go | 3 +++ internal/vfs/filesystem.go | 1 + internal/vfs/s3fs.go | 26 +++++++++++++++++++++----- internal/vfs/vfs.go | 4 +++- templates/webadmin/fsconfig.html | 11 +++++++++++ 11 files changed, 65 insertions(+), 9 deletions(-) diff --git a/docs/portable-mode.md b/docs/portable-mode.md index 47365fb5..a5eee3a4 100644 --- a/docs/portable-mode.md +++ b/docs/portable-mode.md @@ -130,6 +130,13 @@ Flags: prefix and its contents --s3-region string --s3-role-arn string + --s3-skip-tls-verify If enabled the S3 client accepts any TLS + certificate presented by the server and + any host name in that certificate. + In this mode, TLS is susceptible to + man-in-the-middle attacks. + This should be used only for testing. + --s3-storage-class string --s3-upload-concurrency int How many parts are uploaded in parallel (default 2) diff --git a/go.mod b/go.mod index c325de5c..b4dfe1e9 100644 --- a/go.mod +++ b/go.mod @@ -53,7 +53,7 @@ require ( github.com/rs/cors v1.10.1 github.com/rs/xid v1.5.0 github.com/rs/zerolog v1.31.0 - github.com/sftpgo/sdk v0.1.6-0.20231011150824-e8d35102c725 + github.com/sftpgo/sdk v0.1.6-0.20231105181545-b44c8058fc25 github.com/shirou/gopsutil/v3 v3.23.10 github.com/spf13/afero v1.10.0 github.com/spf13/cobra v1.8.0 diff --git a/go.sum b/go.sum index 1d7f2f82..253058e8 100644 --- a/go.sum +++ b/go.sum @@ -445,8 +445,8 @@ github.com/secsy/goftp v0.0.0-20200609142545-aa2de14babf4 h1:PT+ElG/UUFMfqy5HrxJ github.com/secsy/goftp v0.0.0-20200609142545-aa2de14babf4/go.mod h1:MnkX001NG75g3p8bhFycnyIjeQoOjGL6CEIsdE/nKSY= github.com/segmentio/asm v1.2.0 h1:9BQrFxC+YOHJlTlHGkTrFWf59nbL3XnCoFLTwDCI7ys= github.com/segmentio/asm v1.2.0/go.mod h1:BqMnlJP91P8d+4ibuonYZw9mfnzI9HfxselHZr5aAcs= -github.com/sftpgo/sdk v0.1.6-0.20231011150824-e8d35102c725 h1:bV58DOKSIzTqz+UI3q81kRRvsaV0p8b2189nOy7vn7Q= -github.com/sftpgo/sdk v0.1.6-0.20231011150824-e8d35102c725/go.mod h1:KE3KY7v13gSR+8PJMp5LYKQaWELafEM2ZPhmAo7rclM= +github.com/sftpgo/sdk v0.1.6-0.20231105181545-b44c8058fc25 h1:R8cTb41ZX5WSYw8q8ufTKQfOvXh7aLQWqdnteDY/96U= +github.com/sftpgo/sdk v0.1.6-0.20231105181545-b44c8058fc25/go.mod h1:6s/PFoLUd7FXG3wGlrdVhrA0SJOwri2h9kzTph/2oiU= github.com/shirou/gopsutil/v3 v3.23.10 h1:/N42opWlYzegYaVkWejXWJpbzKv2JDy3mrgGzKsh9hM= github.com/shirou/gopsutil/v3 v3.23.10/go.mod h1:JIE26kpucQi+innVlAUnIEOSBhBUkirr5b44yr55+WE= github.com/shoenig/go-m1cpu v0.1.6 h1:nxdKQNcEB6vzgA2E2bvzKIYRuNj7XNJ4S/aRSwKzFtM= diff --git a/internal/cmd/portable.go b/internal/cmd/portable.go index 56340114..677e2cc9 100644 --- a/internal/cmd/portable.go +++ b/internal/cmd/portable.go @@ -65,6 +65,7 @@ var ( portableS3ULPartSize int portableS3ULConcurrency int portableS3ForcePathStyle bool + portableS3SkipTLSVerify bool portableGCSBucket string portableGCSCredentialsFile string portableGCSAutoCredentials int @@ -240,6 +241,7 @@ Please take a look at the usage below to customize the serving parameters`, UploadPartSize: int64(portableS3ULPartSize), UploadConcurrency: portableS3ULConcurrency, ForcePathStyle: portableS3ForcePathStyle, + SkipTLSVerify: portableS3SkipTLSVerify, }, AccessSecret: kms.NewPlainSecret(portableS3AccessSecret), }, @@ -373,6 +375,13 @@ prefix and its contents`) portableCmd.Flags().IntVar(&portableS3ULConcurrency, "s3-upload-concurrency", 2, `How many parts are uploaded in parallel`) portableCmd.Flags().BoolVar(&portableS3ForcePathStyle, "s3-force-path-style", false, `Force path style bucket URL`) + portableCmd.Flags().BoolVar(&portableS3SkipTLSVerify, "s3-skip-tls-verify", false, `If enabled the S3 client accepts any TLS +certificate presented by the server and +any host name in that certificate. +In this mode, TLS is susceptible to +man-in-the-middle attacks. +This should be used only for testing. +`) portableCmd.Flags().StringVar(&portableGCSBucket, "gcs-bucket", "", "") portableCmd.Flags().StringVar(&portableGCSStorageClass, "gcs-storage-class", "", "") portableCmd.Flags().StringVar(&portableGCSKeyPrefix, "gcs-key-prefix", "", `Allows to restrict access to the diff --git a/internal/httpd/httpd_test.go b/internal/httpd/httpd_test.go index a0e9dde2..7fde2c28 100644 --- a/internal/httpd/httpd_test.go +++ b/internal/httpd/httpd_test.go @@ -5215,6 +5215,7 @@ func TestUserS3Config(t *testing.T) { user.FsConfig.S3Config.DownloadPartMaxTime = 60 user.FsConfig.S3Config.UploadPartMaxTime = 40 user.FsConfig.S3Config.ForcePathStyle = true + user.FsConfig.S3Config.SkipTLSVerify = true user.FsConfig.S3Config.DownloadPartSize = 6 folderName := "vfolderName" user.VirtualFolders = append(user.VirtualFolders, vfs.VirtualFolder{ @@ -5243,6 +5244,7 @@ func TestUserS3Config(t *testing.T) { assert.Empty(t, user.FsConfig.S3Config.AccessSecret.GetKey()) assert.Equal(t, 60, user.FsConfig.S3Config.DownloadPartMaxTime) assert.Equal(t, 40, user.FsConfig.S3Config.UploadPartMaxTime) + assert.True(t, user.FsConfig.S3Config.SkipTLSVerify) if assert.Len(t, user.VirtualFolders, 1) { folder := user.VirtualFolders[0] assert.Equal(t, sdkkms.SecretStatusSecretBox, folder.FsConfig.CryptConfig.Passphrase.GetStatus()) @@ -20957,6 +20959,7 @@ func TestWebUserS3Mock(t *testing.T) { user.FsConfig.S3Config.DownloadPartSize = 6 user.FsConfig.S3Config.DownloadConcurrency = 3 user.FsConfig.S3Config.ForcePathStyle = true + user.FsConfig.S3Config.SkipTLSVerify = true user.FsConfig.S3Config.ACL = "public-read" user.Description = "s3 tèst user" form := make(url.Values) @@ -21005,6 +21008,7 @@ func TestWebUserS3Mock(t *testing.T) { form.Set("password_strength", "0") form.Set("ftp_security", "1") form.Set("s3_force_path_style", "checked") + form.Set("s3_skip_tls_verify", "checked") form.Set("description", user.Description) form.Add("hooks", "pre_login_disabled") form.Add("allow_api_key_auth", "1") @@ -21093,6 +21097,7 @@ func TestWebUserS3Mock(t *testing.T) { assert.Equal(t, updateUser.FsConfig.S3Config.DownloadConcurrency, user.FsConfig.S3Config.DownloadConcurrency) assert.Equal(t, lastPwdChange, updateUser.LastPasswordChange) assert.True(t, updateUser.FsConfig.S3Config.ForcePathStyle) + assert.True(t, updateUser.FsConfig.S3Config.SkipTLSVerify) if assert.Equal(t, 2, len(updateUser.Filters.FilePatterns)) { for _, filter := range updateUser.Filters.FilePatterns { switch filter.Path { @@ -23538,6 +23543,7 @@ func TestS3WebFolderMock(t *testing.T) { assert.Equal(t, S3DownloadConcurrency, folder.FsConfig.S3Config.DownloadConcurrency) assert.Equal(t, int64(S3DownloadPartSize), folder.FsConfig.S3Config.DownloadPartSize) assert.False(t, folder.FsConfig.S3Config.ForcePathStyle) + assert.False(t, folder.FsConfig.S3Config.SkipTLSVerify) // update S3UploadConcurrency = 10 form.Set("s3_upload_concurrency", "b") diff --git a/internal/httpd/webadmin.go b/internal/httpd/webadmin.go index bd125f72..cf5e816a 100644 --- a/internal/httpd/webadmin.go +++ b/internal/httpd/webadmin.go @@ -1588,6 +1588,7 @@ func getS3Config(r *http.Request) (vfs.S3FsConfig, error) { return config, fmt.Errorf("invalid s3 download concurrency: %w", err) } config.ForcePathStyle = r.Form.Get("s3_force_path_style") != "" + config.SkipTLSVerify = r.Form.Get("s3_skip_tls_verify") != "" config.DownloadPartMaxTime, err = strconv.Atoi(r.Form.Get("s3_download_part_max_time")) if err != nil { return config, fmt.Errorf("invalid s3 download part max time: %w", err) diff --git a/internal/httpdtest/httpdtest.go b/internal/httpdtest/httpdtest.go index c559ebf2..6918edec 100644 --- a/internal/httpdtest/httpdtest.go +++ b/internal/httpdtest/httpdtest.go @@ -2199,6 +2199,9 @@ func compareS3Config(expected *vfs.Filesystem, actual *vfs.Filesystem) error { / if expected.S3Config.ForcePathStyle != actual.S3Config.ForcePathStyle { return errors.New("fs S3 force path style mismatch") } + if expected.S3Config.SkipTLSVerify != actual.S3Config.SkipTLSVerify { + return errors.New("fs S3 skip TLS verify mismatch") + } if expected.S3Config.DownloadPartMaxTime != actual.S3Config.DownloadPartMaxTime { return errors.New("fs S3 download part max time mismatch") } diff --git a/internal/vfs/filesystem.go b/internal/vfs/filesystem.go index a23afda7..94796742 100644 --- a/internal/vfs/filesystem.go +++ b/internal/vfs/filesystem.go @@ -321,6 +321,7 @@ func (f *Filesystem) GetACopy() Filesystem { DownloadPartMaxTime: f.S3Config.DownloadPartMaxTime, UploadPartMaxTime: f.S3Config.UploadPartMaxTime, ForcePathStyle: f.S3Config.ForcePathStyle, + SkipTLSVerify: f.S3Config.SkipTLSVerify, }, AccessSecret: f.S3Config.AccessSecret.Clone(), }, diff --git a/internal/vfs/s3fs.go b/internal/vfs/s3fs.go index 133e7a97..9081e29c 100644 --- a/internal/vfs/s3fs.go +++ b/internal/vfs/s3fs.go @@ -19,6 +19,7 @@ package vfs import ( "context" + "crypto/tls" "errors" "fmt" "mime" @@ -97,7 +98,9 @@ func NewS3Fs(connectionID, localTempDir, mountPath string, s3Config S3FsConfig) ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - awsConfig, err := config.LoadDefaultConfig(ctx, config.WithHTTPClient(getAWSHTTPClient(0, 30*time.Second))) + awsConfig, err := config.LoadDefaultConfig(ctx, config.WithHTTPClient( + getAWSHTTPClient(0, 30*time.Second, fs.config.SkipTLSVerify)), + ) if err != nil { return fs, fmt.Errorf("unable to get AWS config: %w", err) } @@ -215,7 +218,8 @@ func (fs *S3Fs) Open(name string, offset int64) (File, *PipeReader, func(), erro d.PartSize = fs.config.DownloadPartSize if offset == 0 && fs.config.DownloadPartMaxTime > 0 { d.ClientOptions = append(d.ClientOptions, func(o *s3.Options) { - o.HTTPClient = getAWSHTTPClient(fs.config.DownloadPartMaxTime, 100*time.Millisecond) + o.HTTPClient = getAWSHTTPClient(fs.config.DownloadPartMaxTime, 100*time.Millisecond, + fs.config.SkipTLSVerify) }) } }) @@ -264,7 +268,8 @@ func (fs *S3Fs) Create(name string, flag, checks int) (File, PipeWriter, func(), u.PartSize = fs.config.UploadPartSize if fs.config.UploadPartMaxTime > 0 { u.ClientOptions = append(u.ClientOptions, func(o *s3.Options) { - o.HTTPClient = getAWSHTTPClient(fs.config.UploadPartMaxTime, 100*time.Millisecond) + o.HTTPClient = getAWSHTTPClient(fs.config.UploadPartMaxTime, 100*time.Millisecond, + fs.config.SkipTLSVerify) }) } }) @@ -1071,7 +1076,8 @@ func (fs *S3Fs) downloadToWriter(name string, w PipeWriter) (int64, error) { d.PartSize = fs.config.DownloadPartSize if fs.config.DownloadPartMaxTime > 0 { d.ClientOptions = append(d.ClientOptions, func(o *s3.Options) { - o.HTTPClient = getAWSHTTPClient(fs.config.DownloadPartMaxTime, 100*time.Millisecond) + o.HTTPClient = getAWSHTTPClient(fs.config.DownloadPartMaxTime, 100*time.Millisecond, + fs.config.SkipTLSVerify) }) } }) @@ -1096,7 +1102,7 @@ func (fs *S3Fs) getStorageID() string { return fmt.Sprintf("s3://%v", fs.config.Bucket) } -func getAWSHTTPClient(timeout int, idleConnectionTimeout time.Duration) *awshttp.BuildableClient { +func getAWSHTTPClient(timeout int, idleConnectionTimeout time.Duration, skipTLSVerify bool) *awshttp.BuildableClient { c := awshttp.NewBuildableClient(). WithDialerOptions(func(d *net.Dialer) { d.Timeout = 8 * time.Second @@ -1105,6 +1111,16 @@ func getAWSHTTPClient(timeout int, idleConnectionTimeout time.Duration) *awshttp tr.IdleConnTimeout = idleConnectionTimeout tr.WriteBufferSize = s3TransferBufferSize tr.ReadBufferSize = s3TransferBufferSize + if skipTLSVerify { + if tr.TLSClientConfig != nil { + tr.TLSClientConfig.InsecureSkipVerify = skipTLSVerify + } else { + tr.TLSClientConfig = &tls.Config{ + MinVersion: awshttp.DefaultHTTPTransportTLSMinVersion, + InsecureSkipVerify: skipTLSVerify, + } + } + } }) if timeout > 0 { c = c.WithTimeout(time.Duration(timeout) * time.Second) diff --git a/internal/vfs/vfs.go b/internal/vfs/vfs.go index 217e08e4..86890dfa 100644 --- a/internal/vfs/vfs.go +++ b/internal/vfs/vfs.go @@ -262,10 +262,12 @@ func (c *S3FsConfig) isEqual(other S3FsConfig) bool { if !c.areMultipartFieldsEqual(other) { return false } - if c.ForcePathStyle != other.ForcePathStyle { return false } + if c.SkipTLSVerify != other.SkipTLSVerify { + return false + } return c.isSecretEqual(other) } diff --git a/templates/webadmin/fsconfig.html b/templates/webadmin/fsconfig.html index df2dda65..35e56c30 100644 --- a/templates/webadmin/fsconfig.html +++ b/templates/webadmin/fsconfig.html @@ -232,6 +232,17 @@ along with this program. If not, see . +
+
+ + + + In this mode, TLS is susceptible to man-in-the-middle attacks. This should be used only for testing + +
+
+