spiffe / spire

The SPIFFE Runtime Environment
https://spiffe.io
Apache License 2.0
1.8k stars 474 forks source link

CredentialComposer plugin serializes integer claims as float #4982

Open naroraindeed opened 7 months ago

naroraindeed commented 7 months ago

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.

naroraindeed commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

I think the long term fix will involve changing the proto definition of Claims: https://github.com/spiffe/spire-plugin-sdk/blob/a2e5ba68e76045c0ca332264af935ea8c3a59f99/proto/spire/plugin/server/credentialcomposer/v1/credentialcomposer.pb.go#L991

naroraindeed commented 6 months ago

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 https://github.com/spiffe/spire/blob/9e23dbe97dc017ab3c24675836026c38c733457d/pkg/server/credtemplate/builder.go#L348