Skip to content

Commit

Permalink
Revert "Fix acceptance test to use identity doc and RSA sig"
Browse files Browse the repository at this point in the history
This reverts commit c342691.

The underlying issue causing the need for the workaround has been fixed,
and additional testing changes have been added in hashicorp#4031 so this is no
longer necessary.
  • Loading branch information
joelthompson committed Feb 23, 2018
1 parent 71bd5e1 commit 41c364f
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 35 deletions.
50 changes: 17 additions & 33 deletions builtin/credential/aws/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -958,29 +958,18 @@ func TestBackend_PathBlacklistRoleTag(t *testing.T) {
}
}

/* This is an acceptance test.
Requires the following env vars:
TEST_AWS_EC2_IDENTITY_DOCUMENT
TEST_AWS_EC2_IDENTITY_DOCUMENT_SIG
TEST_AWS_EC2_AMI_ID
TEST_AWS_EC2_ACCOUNT_ID
TEST_AWS_EC2_IAM_ROLE_ARN
If this is being run on an EC2 instance, you can set the environment vars using this bash snippet:
export TEST_AWS_EC2_IDENTITY_DOCUMENT=$(curl -s http://169.254.169.254/latest/dynamic/instance-identity/document | base64 -w 0)
export TEST_AWS_EC2_IDENTITY_DOCUMENT_SIG=$(curl -s http://169.254.169.254/latest/dynamic/instance-identity/signature | tr -d '\n')
export TEST_AWS_EC2_AMI_ID=$(curl -s http://169.254.169.254/latest/meta-data/ami-id)
export TEST_AWS_EC2_IAM_ROLE_ARN=$(aws iam get-role --role-name $(curl -q http://169.254.169.254/latest/meta-data/iam/security-credentials/ -S -s) --query Role.Arn --output text)
export TEST_AWS_EC2_ACCOUNT_ID=$(aws sts get-caller-identity --query Account --output text)
make testacc TEST=./builtin/credential/aws/
If the test is not being run on an EC2 instance that has access to
credentials using EC2RoleProvider, on top of the above vars, following
needs to be set:
TEST_AWS_SECRET_KEY
TEST_AWS_ACCESS_KEY
*/
// This is an acceptance test.
// Requires the following env vars:
// TEST_AWS_EC2_PKCS7
// TEST_AWS_EC2_AMI_ID
// TEST_AWS_EC2_ACCOUNT_ID
// TEST_AWS_EC2_IAM_ROLE_ARN
//
// If the test is not being run on an EC2 instance that has access to
// credentials using EC2RoleProvider, on top of the above vars, following
// needs to be set:
// TEST_AWS_SECRET_KEY
// TEST_AWS_ACCESS_KEY
func TestBackendAcc_LoginWithInstanceIdentityDocAndWhitelistIdentity(t *testing.T) {
// This test case should be run only when certain env vars are set and
// executed as an acceptance test.
Expand All @@ -989,13 +978,9 @@ func TestBackendAcc_LoginWithInstanceIdentityDocAndWhitelistIdentity(t *testing.
return
}

identityDoc := os.Getenv("TEST_AWS_EC2_IDENTITY_DOCUMENT")
if identityDoc == "" {
t.Fatalf("env var TEST_AWS_EC2_IDENTITY_DOCUMENT not set")
}
identityDocSig := os.Getenv("TEST_AWS_EC2_IDENTITY_DOCUMENT_SIG")
if identityDoc == "" {
t.Fatalf("env var TEST_AWS_EC2_IDENTITY_DOCUMENT_SIG not set")
pkcs7 := os.Getenv("TEST_AWS_EC2_PKCS7")
if pkcs7 == "" {
t.Fatalf("env var TEST_AWS_EC2_PKCS7 not set")
}

amiID := os.Getenv("TEST_AWS_EC2_AMI_ID")
Expand Down Expand Up @@ -1059,9 +1044,8 @@ func TestBackendAcc_LoginWithInstanceIdentityDocAndWhitelistIdentity(t *testing.
}

loginInput := map[string]interface{}{
"identity": identityDoc,
"signature": identityDocSig,
"nonce": "vault-client-nonce",
"pkcs7": pkcs7,
"nonce": "vault-client-nonce",
}

// Perform the login operation with a AMI ID that is not matching
Expand Down
4 changes: 2 additions & 2 deletions builtin/credential/aws/path_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -330,8 +330,8 @@ func (b *backend) parseIdentityDocument(ctx context.Context, s logical.Storage,

// Verify extracts the authenticated attributes in the PKCS#7 signature, and verifies
// the authenticity of the content using 'dsa.PublicKey' embedded in the public certificate.
if err = pkcs7Data.Verify(); err != nil {
return nil, fmt.Errorf("failed to verify the signature: %v", err)
if pkcs7Data.Verify() != nil {
return nil, fmt.Errorf("failed to verify the signature")
}

// Check if the signature has content inside of it
Expand Down

0 comments on commit 41c364f

Please sign in to comment.