Merge pull request #728 from SiaFoundation/chris/validate-key-len-s3

Validate key length of S3 access keys in bus

api/settings.go

@@ -2,6 +2,7 @@ package api
 
 import (
 	"errors"
+	"fmt"
 	"time"
 
 	"go.sia.tech/core/types"
@@ -15,6 +16,11 @@ const (
 	SettingUploadPacking    = "uploadpacking"
 )
 
+const (
+	S3MinAccessKeyLen = 16
+	S3MaxAccessKeyLen = 40
+)
+
 var (
 	// ErrSettingNotFound is returned if a requested setting is not present in the
 	// database.
@@ -121,3 +127,16 @@ func (rs RedundancySettings) Validate() error {
 	}
 	return nil
 }
+
+// Validate returns an error if the authentication settings are not considered
+// valid.
+func (s3as S3AuthenticationSettings) Validate() error {
+	for id, key := range s3as.V4Keypairs {
+		if len(id) == 0 {
+			return fmt.Errorf("AccessKeyID cannot be empty")
+		} else if len(key) < S3MinAccessKeyLen || len(key) > S3MaxAccessKeyLen {
+			return fmt.Errorf("AccessKeyID must be between %d and %d characters long but was %d", S3MinAccessKeyLen, S3MaxAccessKeyLen, len(key))
+		}
+	}
+	return nil
+}

bus/bus.go

@@ -1409,6 +1409,15 @@ func (b *bus) settingKeyHandlerPUT(jc jape.Context) {
 			jc.Error(fmt.Errorf("couldn't update redundancy settings, error: %v", err), http.StatusBadRequest)
 			return
 		}
+	case api.SettingS3Authentication:
+		var s3as api.S3AuthenticationSettings
+		if err := json.Unmarshal(data, &s3as); err != nil {
+			jc.Error(fmt.Errorf("couldn't update s3 authentication settings, invalid request body"), http.StatusBadRequest)
+			return
+		} else if err := s3as.Validate(); err != nil {
+			jc.Error(fmt.Errorf("couldn't update s3 authentication settings, error: %v", err), http.StatusBadRequest)
+			return
+		}
 	}
 
 	jc.Check("could not update setting", b.ss.UpdateSetting(jc.Request.Context(), key, string(data)))

go.mod

@@ -18,7 +18,7 @@ require (
 	go.opentelemetry.io/otel/sdk v1.16.0
 	go.opentelemetry.io/otel/trace v1.16.0
 	go.sia.tech/core v0.1.12-0.20231011172826-6ca0ac7b3b6b
-	go.sia.tech/gofakes3 v0.0.0-20231003090232-776c144c0a19
+	go.sia.tech/gofakes3 v0.0.0-20231109151325-e0d47c10dce2
 	go.sia.tech/hostd v0.2.1-0.20231013174940-920057ff41c8
 	go.sia.tech/jape v0.10.1
 	go.sia.tech/mux v1.2.0

go.sum

@@ -307,8 +307,8 @@ go.opentelemetry.io/proto/otlp v1.0.0 h1:T0TX0tmXU8a3CbNXzEKGeU5mIVOdf0oykP+u2lI
 go.opentelemetry.io/proto/otlp v1.0.0/go.mod h1:Sy6pihPLfYHkr3NkUbEhGHFhINUSI/v80hjKIs5JXpM=
 go.sia.tech/core v0.1.12-0.20231011172826-6ca0ac7b3b6b h1:gHnhRiY1SMWCEFu+1Xo0a967RVzHg+g+0grMbHNuLfE=
 go.sia.tech/core v0.1.12-0.20231011172826-6ca0ac7b3b6b/go.mod h1:3EoY+rR78w1/uGoXXVqcYdwSjSJKuEMI5bL7WROA27Q=
-go.sia.tech/gofakes3 v0.0.0-20231003090232-776c144c0a19 h1:qCJHxn1RgRdmltGl3jehgbJcpTSTaWhzdDuOiWkjvm0=
-go.sia.tech/gofakes3 v0.0.0-20231003090232-776c144c0a19/go.mod h1:PlsiVCn6+wssrR7bsOIlZm0DahsVrDydrlbjY4F14sg=
+go.sia.tech/gofakes3 v0.0.0-20231109151325-e0d47c10dce2 h1:ulzfJNjxN5DjXHClkW2pTiDk+eJ+0NQhX87lFDZ03t0=
+go.sia.tech/gofakes3 v0.0.0-20231109151325-e0d47c10dce2/go.mod h1:PlsiVCn6+wssrR7bsOIlZm0DahsVrDydrlbjY4F14sg=
 go.sia.tech/hostd v0.2.1-0.20231013174940-920057ff41c8 h1:0kVIAauXG2+C5EZWlnJDVtf6TrLLgB+EcObuCJyDNfY=
 go.sia.tech/hostd v0.2.1-0.20231013174940-920057ff41c8/go.mod h1:B+jY+eJ2jlcowcXwYOb28N/A6/cy2dblX/WUec9pFM8=
 go.sia.tech/jape v0.10.1 h1:Vr7WoAwZM+kPM1q4dWRZ0q3qCDcoplTHTR9uivFDDEI=

internal/testing/cluster.go

@@ -92,8 +92,8 @@ var (
 		TotalShards: 3,
 	}
 
-	testS3Credentials = credentials.NewStaticV4("myaccesskeyidentifier", "mysecret", "")
-	testS3AuthPairs   = map[string]string{"myaccesskeyidentifier": "mysecret"}
+	testS3Credentials = credentials.NewStaticV4("myaccesskeyidentifier", "mysupersecretkey", "")
+	testS3AuthPairs   = map[string]string{"myaccesskeyidentifier": "mysupersecretkey"}
 )
 
 type TT struct {

internal/testing/s3_test.go

@@ -586,3 +586,61 @@ func TestS3SpecialChars(t *testing.T) {
 		}
 	}
 }
+
+func TestS3SettingsValidate(t *testing.T) {
+	if testing.Short() {
+		t.SkipNow()
+	}
+
+	cluster := newTestCluster(t, clusterOptsDefault)
+	defer cluster.Shutdown()
+
+	tests := []struct {
+		id         string
+		key        string
+		shouldFail bool
+	}{
+		{
+			// Min length
+			id:         "id",
+			key:        "aaaaaaaaaaaaaaaa",
+			shouldFail: false,
+		},
+		{
+			// Max length
+			id:         "id",
+			key:        "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+			shouldFail: false,
+		},
+		{
+			// Min length - 1
+			id:         "id",
+			key:        "aaaaaaaaaaaaaaa",
+			shouldFail: true,
+		},
+		{
+			// Max length + 1
+			id:         "id",
+			key:        "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+			shouldFail: true,
+		},
+		{
+			// No ID
+			id:         "",
+			key:        "aaaaaaaaaaaaaaaa",
+			shouldFail: true,
+		},
+	}
+	for i, test := range tests {
+		err := cluster.Bus.UpdateSetting(context.Background(), api.SettingS3Authentication, api.S3AuthenticationSettings{
+			V4Keypairs: map[string]string{
+				test.id: test.key,
+			},
+		})
+		if err != nil && !test.shouldFail {
+			t.Errorf("%d: unexpected error: %v", i, err)
+		} else if err == nil && test.shouldFail {
+			t.Errorf("%d: expected error", i)
+		}
+	}
+}