S3 AuthenticationSettings validation

api/setting.go

@@ -18,7 +18,8 @@ const (
 
 const (
 	S3MinAccessKeyLen = 16
-	S3MaxAccessKeyLen = 40
+	S3MaxAccessKeyLen = 128
+	S3SecretKeyLen    = 40
 )
 
 var (
@@ -137,11 +138,15 @@ func (rs RedundancySettings) Validate() error {
 // 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 {
+	for accessKeyID, secretAccessKey := range s3as.V4Keypairs {
+		if len(accessKeyID) == 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))
+		} else if len(accessKeyID) < S3MinAccessKeyLen || len(accessKeyID) > S3MaxAccessKeyLen {
+			return fmt.Errorf("AccessKeyID must be between %d and %d characters long but was %d", S3MinAccessKeyLen, S3MaxAccessKeyLen, len(accessKeyID))
+		} else if len(secretAccessKey) == 0 {
+			return fmt.Errorf("SecretAccessKey cannot be empty")
+		} else if len(secretAccessKey) != S3SecretKeyLen {
+			return fmt.Errorf("SecretAccessKey must be %d characters long but was %d", S3SecretKeyLen, len(secretAccessKey))
 		}
 	}
 	return nil

cmd/renterd/main.go

@@ -591,6 +591,17 @@ func main() {
 		} else if as.V4Keypairs == nil {
 			as.V4Keypairs = make(map[string]string)
 		}
+
+		// S3 key pair validation was broken at one point, we need to remove the
+		// invalid key pairs here to ensure we don't fail when we update the
+		// setting below.
+		for k, v := range as.V4Keypairs {
+			if err := (api.S3AuthenticationSettings{V4Keypairs: map[string]string{k: v}}).Validate(); err != nil {
+				logger.Sugar().Infof("removing invalid S3 keypair for AccessKeyID %s, reason: %v", k, err)
+				delete(as.V4Keypairs, k)
+			}
+		}
+
 		// merge keys
 		for k, v := range cfg.S3.KeypairsV4 {
 			as.V4Keypairs[k] = v

internal/testing/cluster.go

@@ -92,8 +92,9 @@ var (
 		TotalShards: 3,
 	}
 
-	testS3Credentials = credentials.NewStaticV4("myaccesskeyidentifier", "mysupersecretkey", "")
-	testS3AuthPairs   = map[string]string{"myaccesskeyidentifier": "mysupersecretkey"}
+	testS3AccessKeyID     = "TESTINGYNHUWCPKOPSYQ"
+	testS3SecretAccessKey = "Rh30BNyj+qNI4ftYRteoZbHJ3X4Ln71QtZkRXzJ9"
+	testS3Credentials     = credentials.NewStaticV4(testS3AccessKeyID, testS3SecretAccessKey, "")
 )
 
 type TT struct {
@@ -521,7 +522,7 @@ func newTestCluster(t *testing.T, opts testClusterOptions) *TestCluster {
 	tt.OK(busClient.UpdateSetting(context.Background(), api.SettingRedundancy, testRedundancySettings))
 	tt.OK(busClient.UpdateSetting(context.Background(), api.SettingContractSet, testContractSetSettings))
 	tt.OK(busClient.UpdateSetting(context.Background(), api.SettingS3Authentication, api.S3AuthenticationSettings{
-		V4Keypairs: testS3AuthPairs,
+		V4Keypairs: map[string]string{testS3AccessKeyID: testS3SecretAccessKey},
 	}))
 	tt.OK(busClient.UpdateSetting(context.Background(), api.SettingUploadPacking, api.UploadPackingSettings{Enabled: enableUploadPacking}))
 

internal/testing/s3_test.go

@@ -603,6 +603,15 @@ func TestS3SettingsValidate(t *testing.T) {
 		t.SkipNow()
 	}
 
+	charset := "doesntreallymatter"
+	stringOfLength := func(n int) string {
+		b := make([]byte, n)
+		for i := range b {
+			b[i] = charset[frand.Intn(len(charset))]
+		}
+		return string(b)
+	}
+
 	cluster := newTestCluster(t, clusterOptsDefault)
 	defer cluster.Shutdown()
 
@@ -612,33 +621,38 @@ func TestS3SettingsValidate(t *testing.T) {
 		shouldFail bool
 	}{
 		{
-			// Min length
-			id:         "id",
-			key:        "aaaaaaaaaaaaaaaa",
+			id:         stringOfLength(api.S3MinAccessKeyLen),
+			key:        stringOfLength(api.S3SecretKeyLen),
 			shouldFail: false,
 		},
 		{
-			// Max length
-			id:         "id",
-			key:        "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+			id:         stringOfLength(api.S3MaxAccessKeyLen),
+			key:        stringOfLength(api.S3SecretKeyLen),
 			shouldFail: false,
 		},
 		{
-			// Min length - 1
-			id:         "id",
-			key:        "aaaaaaaaaaaaaaa",
+			id:         stringOfLength(api.S3MinAccessKeyLen - 1),
+			key:        stringOfLength(api.S3SecretKeyLen),
 			shouldFail: true,
 		},
 		{
-			// Max length + 1
-			id:         "id",
-			key:        "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+			id:         stringOfLength(api.S3MaxAccessKeyLen + 1),
+			key:        stringOfLength(api.S3SecretKeyLen),
 			shouldFail: true,
 		},
 		{
-			// No ID
 			id:         "",
-			key:        "aaaaaaaaaaaaaaaa",
+			key:        stringOfLength(api.S3SecretKeyLen),
+			shouldFail: true,
+		},
+		{
+			id:         stringOfLength(api.S3MinAccessKeyLen),
+			key:        "",
+			shouldFail: true,
+		},
+		{
+			id:         stringOfLength(api.S3MinAccessKeyLen),
+			key:        stringOfLength(api.S3SecretKeyLen + 1),
 			shouldFail: true,
 		},
 	}

stores/metadata_test.go

@@ -3769,11 +3769,11 @@
 		}
 
 		// assert it's validity is within expected bounds
-		min := now.Add(refreshHealthMinHealthValidity)
-		max := now.Add(refreshHealthMaxHealthValidity)
+		min := now.Add(refreshHealthMinHealthValidity).Add(-time.Second) // avoid NDF
+		max := now.Add(refreshHealthMaxHealthValidity).Add(time.Second)  // avoid NDF
 		validUntil := time.Unix(slab.HealthValidUntil, 0)
 		if !(min.Before(validUntil) && max.After(validUntil)) {
 			t.Fatal("valid until not in boundaries", min, max, validUntil, now)
 		}
 	}
 }