Fix a bug in loading .ini file as single column CSV file which causes panic.

- Make sure errors related to malformed csv credential file or bad path are thrown.

PiperOrigin-RevId: 269639332

go/integration/awskms/aws_kms_aead_test.go

@@ -40,7 +40,9 @@ const (
 
 var (
 	// lint placeholder header, please ignore
-	credFile = os.Getenv("TEST_SRCDIR") + "/" + os.Getenv("TEST_WORKSPACE") + "/" + "testdata/credentials_aws.csv"
+	credFile    = os.Getenv("TEST_SRCDIR") + "/" + os.Getenv("TEST_WORKSPACE") + "/" + "testdata/credentials_aws.csv"
+	badCredFile = os.Getenv("TEST_SRCDIR") + "/" + os.Getenv("TEST_WORKSPACE") + "/" + "testdata/bad_access_keys_aws.csv"
+	credINIFile = os.Getenv("TEST_SRCDIR") + "/" + os.Getenv("TEST_WORKSPACE") + "/" + "testdata/credentials_aws.cred"
 	// lint placeholder footer, please ignore
 )
 
@@ -53,13 +55,13 @@ func init() {
 
 // lint placeholder footer, please ignore
 
-func setupKMS(t *testing.T) {
+func setupKMS(t *testing.T, cf string) {
 	t.Helper()
 	g, err := NewAWSClient(keyURI)
 	if err != nil {
 		t.Fatalf("error setting up aws client: %v", err)
 	}
-	_, err = g.LoadCredentials(credFile)
+	_, err = g.LoadCredentials(cf)
 	if err != nil {
 		t.Fatalf("error loading credentials : %v", err)
 	}
@@ -87,48 +89,66 @@ func basicAEADTest(t *testing.T, a tink.AEAD) error {
 }
 
 func TestBasicAead(t *testing.T) {
-	setupKMS(t)
-	// ignore-placeholder4
-	dek := aead.AES128CTRHMACSHA256KeyTemplate()
-	kh, err := keyset.NewHandle(aead.KMSEnvelopeAEADKeyTemplate(keyURI, dek))
-	if err != nil {
-		t.Fatalf("error getting a new keyset handle: %v", err)
-	}
-	a, err := awsaead(kh)
-	if err != nil {
-		t.Fatalf("error getting the primitive: %v", err)
-	}
-	if err := basicAEADTest(t, a); err != nil {
-		t.Errorf("error in basic aead tests: %v", err)
+	for _, file := range []string{credFile, credINIFile} {
+		setupKMS(t, file)
+		// ignore-placeholder4
+		dek := aead.AES128CTRHMACSHA256KeyTemplate()
+		kh, err := keyset.NewHandle(aead.KMSEnvelopeAEADKeyTemplate(keyURI, dek))
+		if err != nil {
+			t.Fatalf("error getting a new keyset handle: %v", err)
+		}
+		a, err := awsaead(kh)
+		if err != nil {
+			t.Fatalf("error getting the primitive: %v", err)
+		}
+		if err := basicAEADTest(t, a); err != nil {
+			t.Errorf("error in basic aead tests: %v", err)
+		}
 	}
 }
 
 func TestBasicAeadWithoutAdditionalData(t *testing.T) {
-	setupKMS(t)
-	// ignore-placeholder4
-	dek := aead.AES128CTRHMACSHA256KeyTemplate()
-	kh, err := keyset.NewHandle(aead.KMSEnvelopeAEADKeyTemplate(keyURI, dek))
-	if err != nil {
-		t.Fatalf("error getting a new keyset handle: %v", err)
-	}
-	a, err := awsaead(kh)
-	if err != nil {
-		t.Fatalf("error getting the primitive: %v", err)
-	}
-	for i := 0; i < 100; i++ {
-		pt := random.GetRandomBytes(20)
-		ct, err := a.Encrypt(pt, nil)
+	for _, file := range []string{credFile, credINIFile} {
+		setupKMS(t, file)
+		// ignore-placeholder4
+		dek := aead.AES128CTRHMACSHA256KeyTemplate()
+		kh, err := keyset.NewHandle(aead.KMSEnvelopeAEADKeyTemplate(keyURI, dek))
 		if err != nil {
-			t.Fatalf("error encrypting data: %v", err)
+			t.Fatalf("error getting a new keyset handle: %v", err)
 		}
-		dt, err := a.Decrypt(ct, nil)
+		a, err := awsaead(kh)
 		if err != nil {
-			t.Fatalf("error decrypting data: %v", err)
+			t.Fatalf("error getting the primitive: %v", err)
 		}
-		if !bytes.Equal(dt, pt) {
-			t.Fatalf("decrypt not inverse of encrypt")
+		for i := 0; i < 100; i++ {
+			pt := random.GetRandomBytes(20)
+			ct, err := a.Encrypt(pt, nil)
+			if err != nil {
+				t.Fatalf("error encrypting data: %v", err)
+			}
+			dt, err := a.Decrypt(ct, nil)
+			if err != nil {
+				t.Fatalf("error decrypting data: %v", err)
+			}
+			if !bytes.Equal(dt, pt) {
+				t.Fatalf("decrypt not inverse of encrypt")
+			}
 		}
 	}
 }
 
 // ignore-placeholder5
+
+func TestLoadBadCSVCredential(t *testing.T) {
+	g, err := NewAWSClient(keyURI)
+	if err != nil {
+		t.Fatalf("error setting up aws client: %v", err)
+	}
+	_, err = g.LoadCredentials(badCredFile)
+	if err == nil {
+		t.Fatalf("does not reject two-column csv file, expect error : %v", errCredCSV)
+	}
+	if err != errCredCSV {
+		t.Fatalf("expect error : %v, got: %v", errCredCSV, err)
+	}
+}

go/integration/awskms/aws_kms_client.go

@@ -37,7 +37,9 @@ const (
 )
 
 var (
-	errCred = errors.New("invalid credential path")
+	errCred    = errors.New("invalid credential path")
+	errBadFile = errors.New("cannot open credential path")
+	errCredCSV = errors.New("malformed credential csv file")
 )
 
 // AWSClient represents a client that connects to the AWS KMS backend.
@@ -73,18 +75,21 @@ func (g *AWSClient) Supported(keyURI string) bool {
 	return ((len(g.keyURI) == 0) && (strings.HasPrefix(strings.ToLower(keyURI), awsPrefix)))
 }
 
-// LoadCredentials loads the credentials in credentialPath. If credentialPath is  null, loads the
-// default credentials.
+// LoadCredentials loads the credentials in credentialPath.
 func (g *AWSClient) LoadCredentials(credentialPath string) (*AWSClient, error) {
 	var creds *credentials.Credentials
 	if len(credentialPath) <= 0 {
 		return nil, errCred
 	}
 	c, err := extractCredsCSV(credentialPath)
-	if err != nil {
-		creds = credentials.NewSharedCredentials(credentialPath, "default")
-	} else {
+	switch err {
+	case nil:
 		creds = credentials.NewStaticCredentialsFromCreds(*c)
+	case errBadFile, errCredCSV:
+		return nil, err
+	default:
+		// fallback to load the credential path as .ini shared credentials.
+		creds = credentials.NewSharedCredentials(credentialPath, "default")
 	}
 	session := session.Must(session.NewSession(&aws.Config{
 		Credentials: creds,
@@ -143,7 +148,7 @@ func validateTrimKMSPrefix(keyURI, prefix string) (string, error) {
 func extractCredsCSV(file string) (*credentials.Value, error) {
 	f, err := os.Open(file)
 	if err != nil {
-		return nil, err
+		return nil, errBadFile
 	}
 	defer f.Close()
 
@@ -151,9 +156,27 @@ func extractCredsCSV(file string) (*credentials.Value, error) {
 	if err != nil {
 		return nil, err
 	}
+
+	// It is possible that the file is an AWS .ini credential file, and it can be
+	// parsed as 1-column CSV file as well. A real AWS credentials.csv is never 1 column.
+	if len(lines) > 0 && len(lines[0]) == 1 {
+		return nil, errors.New("not a valid CSV credential file")
+	}
+
+	// credentials.csv can be obtained when a AWS IAM user is created through IAM console.
+	// The first line of the csv file is "User name,Password,Access key ID,Secret access key,Console login link"
+	// The 2nd line of it contains 5 comma separated values.
+	// Parse the file with a strict format assumption as follows:
+	// 1. There must be at least 4 columns and 2 rows.
+	// 2. The access key id and the secret access key must be on (0-based) column 2 and 3.
 	if len(lines) < 2 {
-		return nil, errors.New("invalid csv file")
+		return nil, errCredCSV
 	}
+
+	if len(lines[1]) < 4 {
+		return nil, errCredCSV
+	}
+
 	return &credentials.Value{
 		AccessKeyID:     lines[1][2],
 		SecretAccessKey: lines[1][3],

testdata/BUILD.bazel

@@ -8,6 +8,7 @@ filegroup(
     srcs = [
         "aws_credentials_cc.txt",
         "aws_key_arn.txt",
+        "bad_access_keys_aws.csv",
         "bad_aws_key_arn.txt",
         "bad_aws_credentials_cc.txt",
         "bad_credentials_aws.csv",

testdata/bad_access_keys_aws.csv

+Access key ID,Secret access key
+AKIAIOSFODNN7EXAMPLE,wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY