diff --git a/cmd/ateapi/internal/actoridentity/actoridentity.go b/cmd/ateapi/internal/actoridentity/actoridentity.go index 1a5a7cbd6..6b9fb2fca 100644 --- a/cmd/ateapi/internal/actoridentity/actoridentity.go +++ b/cmd/ateapi/internal/actoridentity/actoridentity.go @@ -130,8 +130,13 @@ func (s *Server) MintJWT(ctx context.Context, req *ateapipb.MintJWTRequest) (*at return nil, fmt.Errorf("while making actor JWT claims: %w", err) } - // Assume the first authority is the one to use for signing. - actorJWT, err := actoridjwt.Sign(actorWireClaims, signingPool.Authorities[0].SigningKey, signingPool.Authorities[0].Algorithm, signingPool.Authorities[0].ID) + authority, err := signingAuthority(signingPool) + if err != nil { + slog.ErrorContext(ctx, "Failed to select an actor JWT signing authority", slog.Any("err", err)) + return nil, status.Errorf(codes.Internal, "Failed to load actor signing key") + } + + actorJWT, err := actoridjwt.Sign(actorWireClaims, authority.SigningKey, authority.Algorithm, authority.ID) if err != nil { return nil, fmt.Errorf("while signing actor JWT: %w", err) } @@ -141,6 +146,20 @@ func (s *Server) MintJWT(ctx context.Context, req *ateapipb.MintJWTRequest) (*at }, nil } +// signingAuthority picks the authority to sign an actor JWT with. +// +// localjwtauthority.Unmarshal reports no error for JSON that carries no +// authorities, so an empty or incompletely written signing pool file arrives +// here as an empty pool. Report that instead of indexing into it, the way +// MintCert already handles an empty actor CA pool. +func signingAuthority(pool *localjwtauthority.Pool) (*localjwtauthority.Authority, error) { + if pool == nil || len(pool.Authorities) == 0 { + return nil, fmt.Errorf("signing pool contains no authorities") + } + // Assume the first authority is the one to use for signing. + return pool.Authorities[0], nil +} + func (s *Server) MintCert(ctx context.Context, req *ateapipb.MintCertRequest) (*ateapipb.MintCertResponse, error) { p, ok := peer.FromContext(ctx) if !ok { diff --git a/cmd/ateapi/internal/actoridentity/actoridentity_test.go b/cmd/ateapi/internal/actoridentity/actoridentity_test.go new file mode 100644 index 000000000..5e768fbe6 --- /dev/null +++ b/cmd/ateapi/internal/actoridentity/actoridentity_test.go @@ -0,0 +1,68 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package actoridentity + +import ( + "testing" + + "github.com/agent-substrate/substrate/internal/localjwtauthority" +) + +// TestSigningAuthorityEmptyPool covers the pool shapes that carry no signing +// authority. localjwtauthority.Unmarshal accepts JSON with no authorities +// without reporting an error, so MintJWT reaches this selection with an empty +// pool whenever the signing pool file is empty or was written incompletely. +// Selecting from it must report an error rather than panic and take the whole +// ate-api-server down, matching how MintCert treats an empty actor CA pool. +func TestSigningAuthorityEmptyPool(t *testing.T) { + tests := []struct { + name string + pool *localjwtauthority.Pool + }{ + {"nil pool", nil}, + {"no authorities", &localjwtauthority.Pool{}}, + {"empty authority slice", &localjwtauthority.Pool{Authorities: []*localjwtauthority.Authority{}}}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := signingAuthority(tc.pool) + if err == nil { + t.Fatalf("signingAuthority() error = nil, want an error; authority = %+v", got) + } + if got != nil { + t.Errorf("signingAuthority() = %+v alongside an error, want nil", got) + } + }) + } +} + +// TestSigningAuthorityPicksFirst pins the selection rule the signer relies on. +func TestSigningAuthorityPicksFirst(t *testing.T) { + first := &localjwtauthority.Authority{ID: "kid-first", Algorithm: "ES256"} + pool := &localjwtauthority.Pool{ + Authorities: []*localjwtauthority.Authority{ + first, + {ID: "kid-second", Algorithm: "RS256"}, + }, + } + + got, err := signingAuthority(pool) + if err != nil { + t.Fatalf("signingAuthority() error = %v, want nil", err) + } + if got != first { + t.Errorf("signingAuthority() = %+v, want the first authority %+v", got, first) + } +} diff --git a/cmd/ateapi/internal/actoridjwt/actoridjwt.go b/cmd/ateapi/internal/actoridjwt/actoridjwt.go index 980269d6c..cf94d5d67 100644 --- a/cmd/ateapi/internal/actoridjwt/actoridjwt.go +++ b/cmd/ateapi/internal/actoridjwt/actoridjwt.go @@ -120,21 +120,30 @@ func Sign(wireClaims *WireClaims, signingKey crypto.PrivateKey, algorithm, keyID var sigBytes []byte switch algorithm { case "RS256": - rsaKey := signingKey.(*rsa.PrivateKey) + rsaKey, ok := signingKey.(*rsa.PrivateKey) + if !ok { + return "", fmt.Errorf("algorithm %s requires an RSA key, got %T", algorithm, signingKey) + } toBeSignedDigest := hashBytes(crypto.SHA256.New(), []byte(toBeSigned)) sigBytes, err = rsa.SignPKCS1v15(rand.Reader, rsaKey, crypto.SHA256, toBeSignedDigest) if err != nil { return "", fmt.Errorf("while performing RSA PKCS1v15 signature: %w", err) } case "RS384": - rsaKey := signingKey.(*rsa.PrivateKey) + rsaKey, ok := signingKey.(*rsa.PrivateKey) + if !ok { + return "", fmt.Errorf("algorithm %s requires an RSA key, got %T", algorithm, signingKey) + } toBeSignedDigest := hashBytes(crypto.SHA384.New(), []byte(toBeSigned)) sigBytes, err = rsa.SignPKCS1v15(rand.Reader, rsaKey, crypto.SHA384, toBeSignedDigest) if err != nil { return "", fmt.Errorf("while performing RSA PKCS1v15 signature: %w", err) } case "RS512": - rsaKey := signingKey.(*rsa.PrivateKey) + rsaKey, ok := signingKey.(*rsa.PrivateKey) + if !ok { + return "", fmt.Errorf("algorithm %s requires an RSA key, got %T", algorithm, signingKey) + } toBeSignedDigest := hashBytes(crypto.SHA512.New(), []byte(toBeSigned)) sigBytes, err = rsa.SignPKCS1v15(rand.Reader, rsaKey, crypto.SHA512, toBeSignedDigest) if err != nil { @@ -142,7 +151,10 @@ func Sign(wireClaims *WireClaims, signingKey crypto.PrivateKey, algorithm, keyID } case "ES256": // JOSE ES256 defined at https://datatracker.ietf.org/doc/rfc7518/ section 3.4 - ecdsaKey := signingKey.(*ecdsa.PrivateKey) + ecdsaKey, ok := signingKey.(*ecdsa.PrivateKey) + if !ok { + return "", fmt.Errorf("algorithm %s requires an ECDSA key, got %T", algorithm, signingKey) + } if ecdsaKey.Curve != elliptic.P256() { return "", fmt.Errorf("ES256 requires a P256 key") } diff --git a/cmd/ateapi/internal/actoridjwt/actoridjwt_test.go b/cmd/ateapi/internal/actoridjwt/actoridjwt_test.go new file mode 100644 index 000000000..21b002767 --- /dev/null +++ b/cmd/ateapi/internal/actoridjwt/actoridjwt_test.go @@ -0,0 +1,115 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package actoridjwt + +import ( + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" + "strings" + "testing" +) + +func testRSAKey(t *testing.T) *rsa.PrivateKey { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("generating RSA key: %v", err) + } + return key +} + +func testECKey(t *testing.T, curve elliptic.Curve) *ecdsa.PrivateKey { + t.Helper() + key, err := ecdsa.GenerateKey(curve, rand.Reader) + if err != nil { + t.Fatalf("generating EC key: %v", err) + } + return key +} + +// TestSignKeyAlgorithmMismatch covers every algorithm branch with a key of the +// wrong type. The algorithm and the key come from the same signing pool entry, +// which is operator-supplied, so a mismatched pair is a configuration error and +// must surface as an error rather than taking the process down. +func TestSignKeyAlgorithmMismatch(t *testing.T) { + rsaKey := testRSAKey(t) + ecKey := testECKey(t, elliptic.P256()) + + tests := []struct { + name string + algorithm string + key crypto.PrivateKey + }{ + {"RS256 with EC key", "RS256", ecKey}, + {"RS384 with EC key", "RS384", ecKey}, + {"RS512 with EC key", "RS512", ecKey}, + {"ES256 with RSA key", "ES256", rsaKey}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := Sign(&WireClaims{Issuer: "https://issuer.example"}, tc.key, tc.algorithm, "kid-1") + if err == nil { + t.Fatalf("Sign(%s) error = nil, want an error; token = %q", tc.algorithm, got) + } + if got != "" { + t.Errorf("Sign(%s) returned token %q alongside an error, want empty", tc.algorithm, got) + } + }) + } +} + +// TestSignUnimplementedAlgorithm pins the pre-existing behavior for an algorithm +// the signer does not implement, so the mismatch handling above cannot swallow it. +func TestSignUnimplementedAlgorithm(t *testing.T) { + if _, err := Sign(&WireClaims{}, testRSAKey(t), "HS256", "kid-1"); err == nil { + t.Fatal("Sign(HS256) error = nil, want an error") + } +} + +// TestSignMatchingKey checks the algorithms that do have a matching key still +// produce a three-segment compact JWT. +func TestSignMatchingKey(t *testing.T) { + tests := []struct { + algorithm string + key crypto.PrivateKey + }{ + {"RS256", testRSAKey(t)}, + {"RS384", testRSAKey(t)}, + {"RS512", testRSAKey(t)}, + {"ES256", testECKey(t, elliptic.P256())}, + } + for _, tc := range tests { + t.Run(tc.algorithm, func(t *testing.T) { + token, err := Sign(&WireClaims{Issuer: "https://issuer.example"}, tc.key, tc.algorithm, "kid-1") + if err != nil { + t.Fatalf("Sign(%s) error = %v, want nil", tc.algorithm, err) + } + if n := len(strings.Split(token, ".")); n != 3 { + t.Errorf("Sign(%s) produced %d segments, want 3", tc.algorithm, n) + } + }) + } +} + +// TestSignES256WrongCurve keeps the existing curve check covered: ES256 is +// defined over P-256 only, and a P-384 key must be rejected, not signed. +func TestSignES256WrongCurve(t *testing.T) { + if _, err := Sign(&WireClaims{}, testECKey(t, elliptic.P384()), "ES256", "kid-1"); err == nil { + t.Fatal("Sign(ES256) with a P-384 key error = nil, want an error") + } +}