Skip to content
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.

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

CredentialComposer plugin serializes integer claims as float #4982

Open
naroraindeed opened this issue Mar 14, 2024 · 4 comments
Open

CredentialComposer plugin serializes integer claims as float #4982

naroraindeed opened this issue Mar 14, 2024 · 4 comments
Labels
help wanted Issues with this label are ready to start work but are in need of someone to do it priority/backlog Issue is approved and in the backlog unscoped The issue needs more design or understanding in order for the work to progress

Comments

@naroraindeed
Copy link
Contributor

  • Version: b5879e2
  • Platform: Linux server-1 5.15.0-1053-aws #58~20.04.1-Ubuntu SMP Mon Jan 22 17:15:01 UTC 2024 x86_64 Linux
  • Subsystem: server\credentialcomposer

Problem: Using the composer plugin for modifying JWT-SVIDs yields in timestamp claims like iat, exp etc. to be formatted as type float. While numeric type is valid for timestamps and the the token can be successfully validated using jwt.io, AWS rejects it. I've already reached out to AWS support and they have asked us to fix the issue on our end.

...based on IAM service design, the float timestamp is currently not supported in token claims. While there might not be any restrictions in JWT specification on timestamps being integers or float, IAM service does not currently support float timestamps, hence the error of "Invalid timestamp in token claims" being subsequently thrown. Kindly update your timestamp accordingly and retry your workflow.

Here's a sample spire JWT-SVID payload when not using the composer plugin

eyJhdWQiOlsic3RzLmFtYXpvbmF3cy5jb20iXSwiZXhwIjoxNzEwNzA0NDk1LCJpYXQiOjE3MTA0NDUyOTUsImlzcyI6Imh0dHBzOi8vd29ya2xvYWRpZC5pbmRlZWQudGVjaCIsInN1YiI6InNwaWZmZTovL3dvcmtsb2FkaWQuaW5kZWVkLnRlY2gvZm9vLXNhIn0

Note, exp and iat are integer.

Here's a sample spire JWT-SVID payload when using the credential composer plugin (our plugin implementation never modifies those claims)

eyJhdWQiOlsic3RzLmFtYXpvbmF3cy5jb20iXSwiZXhwIjoxLjcxMDYxODIxZSswOSwiaHR0cHM6Ly9hd3MuYW1hem9uLmNvbS90YWdzIjp7InByaW5jaXBhbF90YWdzIjp7IkQwIjpbImFiYyJdfSwidHJhbnNpdGl2ZV90YWdfa2V5cyI6WyJEMCJdfSwiaWF0IjoxLjcxMDQ0NTQxZSswOSwiaXNzIjoiaHR0cHM6Ly93b3JrbG9hZGlkLmluZGVlZC50ZWNoIiwicHVycG9zZSI6ImFjY2Vzc190b2tlbiIsInNjb3BlIjoiRDA6YSBEMDpiIEQwOmMiLCJzdWIiOiJzcGlmZmU6Ly93b3JrbG9hZGlkLmluZGVlZC50ZWNoL2Zvby1zYSJ9

Note, exp and iat are floats. Still valid JWTs and valid timestamps, but AWS SDK will reject the use of such JWT with:

An error occurred (InvalidIdentityToken) when calling the AssumeRoleWithWebIdentity operation: Invalid timestamp in token claims.

Upon initial investigation is looks like an issue with the proto definition of Claims. We believe it’s due to the following representation:
Claims *structpb.Struct
The protobuf struct doesn’t support integers and prefers to represent numbers as double.
double number_value = 2;

The translations of Claims as protobuf Struct and then further JSON serialization is likely yielding the final format of float type.

Goal: Stick to integer representation for well known timestamp claims for compatibility with AWS.

@evan2645 evan2645 added the triage/in-progress Issue triage is in progress label Mar 19, 2024
@MarcosDY MarcosDY added priority/backlog Issue is approved and in the backlog unscoped The issue needs more design or understanding in order for the work to progress help wanted Issues with this label are ready to start work but are in need of someone to do it and removed triage/in-progress Issue triage is in progress labels Apr 2, 2024
@naroraindeed
Copy link
Contributor Author

Wanted to add that we distilled our composer plugin to:

func (p *Plugin) ComposeWorkloadJWTSVID(_ context.Context, req *credentialcomposerv1.ComposeWorkloadJWTSVIDRequest) (*credentialcomposerv1.ComposeWorkloadJWTSVIDResponse, error) {
	return &credentialcomposerv1.ComposeWorkloadJWTSVIDResponse{
		Attributes: req.Attributes,
	}, nil
}

and verified the timestamps were still float. The issue is in the implementation of the plugin model. This essentially makes integer claims impossible when using the plugin.

@naroraindeed
Copy link
Contributor Author

naroraindeed commented Apr 25, 2024

We made some more progress in the investigation. I added custom claims like so:

attributes.Claims["i64"] = math.MaxInt64
attributes.Claims["i32"] = math.MaxInt32
attributes.Claims["i16"] = math.MaxInt16
attributes.Claims["i8"] = math.MaxInt8

and here's the resultant payload:
eyJhdWQiOlsic3RzLmFtYXpvbmF3cy5jb20iXSwiZXhwIjoxLjcxNDA3MjUxN2UrMDksImh0dHBzOi8vYXdzLmFtYXpvbi5jb20vdGFncyI6eyJwcmluY2lwYWxfdGFncyI6eyJEMCI6WyJhYmMiXX0sInRyYW5zaXRpdmVfdGFnX2tleXMiOlsiRDAiXX0sImkxNiI6MzI3NjcsImkzMiI6Mi4xNDc0ODM2NDdlKzA5LCJpNjQiOjkuMjIzMzcyMDM2ODU0Nzc2ZSsxOCwiaTgiOjEyNywiaWF0IjoxLjcxNDA3MDcxN2UrMDksInB1cnBvc2UiOiJhY2Nlc3NfdG9rZW4iLCJzY29wZSI6IkQwOmEgRDA6YiBEMDpjIiwic3ViIjoic3BpZmZlOi8vZXhhbXBsZS5jb20vdGVzdDEifQ

The i8 and i16 claim are formatted as integer only, i32 and i64 are in exponent format. Of course all of these claims are represented asstructpb.Value. E.g.
image

But the final serialization preserves integer format for small integers. I'm wondering if we can implement a custom marshaller to revert floats back to integers inside SPIRE when composer plugins are used. Any ideas are appreciated.

@naroraindeed
Copy link
Contributor Author

@naroraindeed
Copy link
Contributor Author

Temporarily fixed with:

	if len(b.config.CredentialComposers) > 0 {
		// reset iat and exp from float64 to jwt.NumericDate type
		if iat, ok := attributes.Claims["iat"].(float64); ok {
			attributes.Claims["iat"] = int64(iat)
		}

		if exp, ok := attributes.Claims["exp"].(float64); ok {
			attributes.Claims["exp"] = int64(exp)
		}
	}

right before

return attributes.Claims, nil

naroraindeed added a commit to naroraindeed/spire that referenced this issue Apr 30, 2024
…patiblity with AWS

Using credentialcomposer plugins forces Claims to be translated as protobuf structs which serializes integers as floats (spiffe#4982). AWS rejects validating JWT issued by SPIRE with timestamps that are in scientific notation. AWS STS only accepts integer timestamps as valid. We've discussed this with AWS, and while they agree it's an issue in AWS STS, there's no recourse available with them. This fix helps reset value type for timestamps and also includes unit tests that make the problem obvious. This is the minimal change needed for SPIRE to produce verifiable JWT for AWS when using credentialcomposer plugin.

Signed-off-by: narora@indeed.com
naroraindeed added a commit to naroraindeed/spire that referenced this issue May 1, 2024
…patiblity with AWS

Using credentialcomposer plugins forces Claims to be translated as protobuf structs which serializes integers as floats (spiffe#4982). AWS rejects validating JWT issued by SPIRE with timestamps that are in scientific notation. AWS STS only accepts integer timestamps as valid. We've discussed this with AWS, and while they agree it's an issue in AWS STS, there's no recourse available with them. This fix helps reset value type for timestamps and also includes unit tests that make the problem obvious. This is the minimal change needed for SPIRE to produce verifiable JWT for AWS when using credentialcomposer plugin.

Signed-off-by: Nikhil Arora <narora@indeed.com>
azdagron pushed a commit that referenced this issue May 1, 2024
…patiblity with AWS (#5115)

Using credentialcomposer plugins forces Claims to be translated as protobuf structs which serializes integers as floats (#4982). AWS rejects validating JWT issued by SPIRE with timestamps that are in scientific notation. AWS STS only accepts integer timestamps as valid. We've discussed this with AWS, and while they agree it's an issue in AWS STS, there's no recourse available with them. This fix helps reset value type for timestamps and also includes unit tests that make the problem obvious. This is the minimal change needed for SPIRE to produce verifiable JWT for AWS when using credentialcomposer plugin.

Signed-off-by: Nikhil Arora <narora@indeed.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issues with this label are ready to start work but are in need of someone to do it priority/backlog Issue is approved and in the backlog unscoped The issue needs more design or understanding in order for the work to progress
Projects
None yet
Development

No branches or pull requests

3 participants