Skip to content

Commit

Permalink
reset timestamps to integer when composer plugins are invoked for com…
Browse files Browse the repository at this point in the history
…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>
  • Loading branch information
naroraindeed authored May 1, 2024
1 parent 59a3000 commit 9deb317
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 0 deletions.
12 changes: 12 additions & 0 deletions pkg/server/credtemplate/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,18 @@ func (b *Builder) BuildWorkloadJWTSVIDClaims(ctx context.Context, params Workloa
}
}

// AWS will otherwise reject validating timestamps serialized in scientific notation.
// Protobuf serializes large integers as float since Claims are represented as google.protobuf.Struct.
if len(b.config.CredentialComposers) > 0 {
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)
}
}

return attributes.Claims, nil
}

Expand Down
59 changes: 59 additions & 0 deletions pkg/server/credtemplate/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"encoding/asn1"
"errors"
"fmt"
"math"
"math/big"
"net/url"
"testing"
Expand Down Expand Up @@ -1161,6 +1162,43 @@ func TestBuildWorkloadJWTSVIDClaims(t *testing.T) {
config.CredentialComposers = []credentialcomposer.CredentialComposer{loadNoopV1Plugin(t)}
},
},
{
desc: "real grpc composer",
overrideConfig: func(config *credtemplate.Config) {
config.CredentialComposers = []credentialcomposer.CredentialComposer{loadGrpcPlugin(t)}
},
overrideExpected: func(expected map[string]any) {
expected["aud"] = []interface{}{"AUDIENCE"}
expected["iat"] = now.Unix()
expected["exp"] = jwtSVIDNotAfter.Unix()
},
},
{
desc: "real grpc composer overriding first composer",
overrideConfig: func(config *credtemplate.Config) {
config.CredentialComposers = []credentialcomposer.CredentialComposer{fakeCC{id: 1, onlyFoo: true, addInt64: true}, loadGrpcPlugin(t)}
},
overrideExpected: func(expected map[string]any) {
expected["aud"] = []interface{}{"AUDIENCE"}
expected["iat"] = now.Unix()
expected["exp"] = jwtSVIDNotAfter.Unix()
expected["foo"] = "VALUE-1"
expected["i64"] = float64(math.MaxInt64)
},
},
{
desc: "real grpc composer with second composer",
overrideConfig: func(config *credtemplate.Config) {
config.CredentialComposers = []credentialcomposer.CredentialComposer{loadGrpcPlugin(t), fakeCC{id: 1, onlyFoo: true, addInt64: true}}
},
overrideExpected: func(expected map[string]any) {
expected["aud"] = []interface{}{"AUDIENCE"}
expected["iat"] = now.Unix()
expected["exp"] = jwtSVIDNotAfter.Unix()
expected["foo"] = "VALUE-1"
expected["i64"] = math.MaxInt64
},
},
} {
t.Run(tc.desc, func(t *testing.T) {
testBuilder(t, tc.overrideConfig, func(t *testing.T, credBuilder *credtemplate.Builder) {
Expand Down Expand Up @@ -1239,6 +1277,7 @@ type fakeCC struct {
id byte
onlyCommonName bool
onlyFoo bool
addInt64 bool
}

func (cc fakeCC) ComposeServerX509CA(_ context.Context, attributes credentialcomposer.X509CAAttributes) (credentialcomposer.X509CAAttributes, error) {
Expand Down Expand Up @@ -1267,6 +1306,9 @@ func (cc fakeCC) ComposeWorkloadJWTSVID(_ context.Context, _ spiffeid.ID, attrib
if !cc.onlyFoo {
attributes.Claims["bar"] = cc.applySuffix("VALUE")
}
if cc.addInt64 {
attributes.Claims["i64"] = math.MaxInt64
}
return attributes, nil
}

Expand Down Expand Up @@ -1297,3 +1339,20 @@ func loadNoopV1Plugin(t *testing.T) credentialcomposer.CredentialComposer {
plugintest.Load(t, catalog.MakeBuiltIn("noop", server), cc)
return cc
}

type grpcPlugin struct {
credentialcomposerv1.UnimplementedCredentialComposerServer
}

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

func loadGrpcPlugin(t *testing.T) credentialcomposer.CredentialComposer {
server := credentialcomposerv1.CredentialComposerPluginServer(grpcPlugin{})
cc := new(credentialcomposer.V1)
plugintest.Load(t, catalog.MakeBuiltIn("grpcPlugin", server), cc)
return cc
}

0 comments on commit 9deb317

Please sign in to comment.