From 72b48f065ab5df5707068fe707b2d844669d9937 Mon Sep 17 00:00:00 2001
From: Nikhil Arora <narora@indeed.com>
Date: Tue, 30 Apr 2024 15:51:50 -0700
Subject: [PATCH] reset timestamps to integer when composer plugins are invoked
 for compatiblity with AWS

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>
---
 pkg/server/credtemplate/builder.go      | 12 +++++
 pkg/server/credtemplate/builder_test.go | 59 +++++++++++++++++++++++++
 2 files changed, 71 insertions(+)

diff --git a/pkg/server/credtemplate/builder.go b/pkg/server/credtemplate/builder.go
index e5be5e1dfd..df55a5c1ba 100644
--- a/pkg/server/credtemplate/builder.go
+++ b/pkg/server/credtemplate/builder.go
@@ -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
 }
 
diff --git a/pkg/server/credtemplate/builder_test.go b/pkg/server/credtemplate/builder_test.go
index 0a55fe38cd..442326ea59 100644
--- a/pkg/server/credtemplate/builder_test.go
+++ b/pkg/server/credtemplate/builder_test.go
@@ -8,6 +8,7 @@ import (
 	"encoding/asn1"
 	"errors"
 	"fmt"
+	"math"
 	"math/big"
 	"net/url"
 	"testing"
@@ -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) {
@@ -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) {
@@ -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
 }
 
@@ -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
+}