Skip to content

Fix AWS CLI access using AWS OIDC integration #57843

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
r0mant merged 2 commits into from
Aug 15, 2025
Merged

Fix AWS CLI access using AWS OIDC integration #57843

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3db2745
Fix AWS CLI access using AWS OIDC integration
marcoandredinis Aug 13, 2025
00e30aa
fix test
marcoandredinis Aug 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions lib/auth/app_access_aws_credentials.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,22 @@ import (
"github.com/gravitational/teleport/lib/integrations/awsra"
)

var errNotIntegrationApp = errors.New("target resource is not an application with an associated integration")
var errAppWithoutAWSClientSideCredentials = errors.New("target resource is not an application that sends credentials to the client")

// generateAWSConfigCredentialProcessCredentials generates AWS Credentials for the given application.
// If the target resource is not an application with an associated integration, it returns errNotIntegrationApp error,
// which should be handled gracefully by the caller.
// generateAWSClientSideCredentials generates AWS Credentials for the given application.
// If the target resource is not an application with an associated AWS Roles Anywhere integration,
// it returns errAppWithoutAWSClientSideCredentials error, which should be handled gracefully by the caller.
//
// The AWS Credentials returned are in the format expected by the AWS CLI/SDK config file `credential_process` (usually located at ~/.aws/config).
// Expected format documentation: https://docs.aws.amazon.com/sdkref/latest/guide/feature-process-credentials.html
func generateAWSConfigCredentialProcessCredentials(
func generateAWSClientSideCredentials(
ctx context.Context,
a *Server,
req certRequest,
notAfter time.Time,
) (string, error) {
if req.appName == "" || req.awsRoleARN == "" {
return "", errNotIntegrationApp
return "", errAppWithoutAWSClientSideCredentials
}

appInfo, err := getAppServerByName(ctx, a, req.appName)
Expand All @@ -56,7 +56,7 @@ func generateAWSConfigCredentialProcessCredentials(
// Only integrations can generate credentials.
integrationName := appInfo.GetIntegration()
if integrationName == "" {
return "", errNotIntegrationApp
return "", errAppWithoutAWSClientSideCredentials
}

integration, err := a.GetIntegration(ctx, integrationName)
Expand All @@ -69,6 +69,10 @@ func generateAWSConfigCredentialProcessCredentials(
// Only AWS Roles Anywhere integrations can generate credentials.
return generateAWSRolesAnywhereCredentials(ctx, a, req, appInfo, integration, notAfter)

case types.IntegrationSubKindAWSOIDC:
// AWS access using AWS OIDC integration will proxy requests, but will not send any credentials to the client.
return "", errAppWithoutAWSClientSideCredentials

default:
return "", trace.BadParameter("application %q is using integration %q for access, which does not support AWS credential generation", req.appName, integrationName)
}
Expand Down
6 changes: 3 additions & 3 deletions lib/auth/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3565,10 +3565,10 @@ func generateCert(ctx context.Context, a *Server, req certRequest, caType types.
return nil, trace.Wrap(err)
}

// Generate AWS credential process credentials if the user is trying to access an App with an AWS Roles Anywhere Integration.
awsCredentialProcessCredentials, err := generateAWSConfigCredentialProcessCredentials(ctx, a, req, notAfter)
// Generate AWS client side credentials if the user is trying to access an AWS App with an AWS Roles Anywhere Integration.
awsCredentialProcessCredentials, err := generateAWSClientSideCredentials(ctx, a, req, notAfter)
switch {
case errors.Is(err, errNotIntegrationApp):
case errors.Is(err, errAppWithoutAWSClientSideCredentials):
case err != nil:
return nil, trace.Wrap(err)
}
Comment on lines +3569 to 3574
Copy link
Copy Markdown
Collaborator

@r0mant r0mant Aug 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that trying to determine whether to generate credentials or not based on the returned error is very awkward and error-prone (and not very Go idiomatic) - which is why I suspect we got this regression in the first place.

Can this be done by doing explicit checks on the integration type and request instead of doing this opportunistically based on the error?

Copy link
Copy Markdown
Contributor Author

@marcoandredinis marcoandredinis Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Integration type is explicitly checked.
The request is not checked because it is un-changed from regular AWS App access, the client does not explicitly ask for client side credentials.

I can add other sentinel errors, and check all of them here.

	// Generate AWS client side credentials if the user is trying to access an AWS App with an AWS Roles Anywhere Integration.
	awsCredentialProcessCredentials, err := generateAWSClientSideCredentials(ctx, a, req, notAfter)
	switch {
	case errors.Is(err, errNotAWSAccessApp) || errors.Is(err, errAWSAppWithoutIntegration) || errors.Is(err, errProxiedAWSApp):
	case err != nil:
		return nil, trace.Wrap(err)
	}

I'm not convinced by this, it adds a list of errors which can become an oversight later on.
On 2nd thought, maybe this is not what you meant.

I don't think moving the logic from the function to here is a good approach as it would add quite a lot of code to an already large method.

Copy link
Copy Markdown
Contributor

@GavinFrazar GavinFrazar Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not strongly opposed to leaving it as-is, but you could refactor this to just return nil in cases where you would return the errAppWithoutAWSClientSideCredentials?

It seems like the graceful error handling behavior just ignores the error anyway and allowing for empty credentials is the expected behavior, so return "", nil makes sense to me.

I'm not convinced the error handling scheme had anything to do with the bug though. We just forgot to check for the AWS OIDC subkind, and for other subkinds it makes sense to return an error e.g. it should be an error to request an --aws-role with an Azure OIDC integration subkind.

Copy link
Copy Markdown
Contributor

@GavinFrazar GavinFrazar Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo we should have an exported list of integration subkinds and then table test against that.
That way if a subkind is added in the future that supports aws client side credentials, the tests will force us to update the switch statement to account for it

Copy link
Copy Markdown
Contributor Author

@marcoandredinis marcoandredinis Aug 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so return "", nil makes sense to me.

(when I first implemented, it was like that before code review 😅)

As for adding all the integration subkinds to table test them, maybe that's a good idea.
For now, their features barely intercept, so that's why it wasn't an issue in the past.

Expand Down
78 changes: 78 additions & 0 deletions lib/auth/auth_with_roles_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -869,6 +869,84 @@ func TestAWSRolesAnywhereCredentialGenerationForApps(t *testing.T) {
require.JSONEq(t, `{"Version":1,"AccessKeyId":"aki","SecretAccessKey":"sak","SessionToken":"st","Expiration":"2025-06-25T12:07:02.474135Z"}`, identity.RouteToApp.AWSCredentialProcessCredentials)
}

func TestAppAccessUsingAWSOIDC_doesntGenerateClientCredentials(t *testing.T) {
ctx := t.Context()
srv := newTestTLSServer(t)

// Set up a user with the necessary roles to access the AWS App.
username := "aws-access-user"
roleARN := "arn:aws:iam::123456789012:role/MyRole"
awsAccessRole, err := authtest.CreateRole(ctx, srv.Auth(), "aws-access", types.RoleSpecV6{
Allow: types.RoleConditions{
AppLabels: types.Labels{
types.Wildcard: []string{types.Wildcard},
},
AWSRoleARNs: []string{roleARN},
},
})
require.NoError(t, err)

awsOIDCIntegration := "aws-oidc-integration"
ig, err := types.NewIntegrationAWSOIDC(types.Metadata{Name: awsOIDCIntegration}, &types.AWSOIDCIntegrationSpecV1{
RoleARN: "arn:aws:iam::123456789012:role/MyRole",
})
require.NoError(t, err)
_, err = srv.Auth().Integrations.CreateIntegration(ctx, ig)
require.NoError(t, err)

// Set up a new AWS App created from a Roles Anywhere Profile.
appName := "my-aws-access-using-awsoidc"
app, err := types.NewAppServerV3(
types.Metadata{Name: appName},
types.AppServerSpecV3{
HostID: uuid.NewString(),
App: &types.AppV3{
Metadata: types.Metadata{Name: appName},
Spec: types.AppSpecV3{
URI: constants.AWSConsoleURL,
Integration: awsOIDCIntegration,
PublicAddr: "my-app.example.com",
},
},
},
)
require.NoError(t, err)
_, err = srv.Auth().UpsertApplicationServer(ctx, app)
require.NoError(t, err)

user, err := authtest.CreateUser(ctx, srv.Auth(), username, awsAccessRole)
require.NoError(t, err)

client, err := srv.NewClient(authtest.TestUser(user.GetName()))
require.NoError(t, err)

// Create credentials for the AWS App
priv, err := cryptosuites.GenerateKeyWithAlgorithm(cryptosuites.ECDSAP256)
require.NoError(t, err)
pub, err := keys.MarshalPublicKey(priv.Public())
require.NoError(t, err)

certs, err := client.GenerateUserCerts(ctx, proto.UserCertsRequest{
TLSPublicKey: pub,
Username: user.GetName(),
Expires: time.Now().Add(time.Hour),
RouteToApp: proto.RouteToApp{
Name: appName,
AWSRoleARN: roleARN,
},
})
require.NoError(t, err)

// Parse the Identity and check the AWS Role ARN and credentials.
tlsCert, err := tlsca.ParseCertificatePEM(certs.TLS)
require.NoError(t, err)
identity, err := tlsca.FromSubject(tlsCert.Subject, tlsCert.NotAfter)
require.NoError(t, err)

require.Equal(t, roleARN, identity.AWSRoleARNs[0], "Expected AWS Role ARN to match the one requested")
require.Empty(t, identity.RouteToApp.AWSCredentialProcessCredentials)
}

func TestSSODiagnosticInfo(t *testing.T) {
ctx := context.Background()
srv := newTestTLSServer(t)
Expand Down
Loading