From 61369a0103781dd074e4dd8cd65bad775c97ee24 Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Mon, 8 Jan 2024 00:11:14 +1100 Subject: [PATCH 1/9] Implement `--client-signing-algorithms` Signed-off-by: Alex Cameron --- cmd/app/grpc.go | 4 +- cmd/app/serve.go | 15 ++++++-- pkg/server/algorithm_registry.go | 63 ++++++++++++++++++++++++++++++++ pkg/server/grpc_server.go | 38 ++++++++++++++++--- 4 files changed, 108 insertions(+), 12 deletions(-) create mode 100644 pkg/server/algorithm_registry.go diff --git a/cmd/app/grpc.go b/cmd/app/grpc.go index 05dccbfb4..a7abf272c 100644 --- a/cmd/app/grpc.go +++ b/cmd/app/grpc.go @@ -154,7 +154,7 @@ func (c *cachedTLSCert) GRPCCreds() grpc.ServerOption { })) } -func createGRPCServer(cfg *config.FulcioConfig, ctClient *ctclient.LogClient, baseca ca.CertificateAuthority, ip identity.IssuerPool) (*grpcServer, error) { +func createGRPCServer(cfg *config.FulcioConfig, ctClient *ctclient.LogClient, baseca ca.CertificateAuthority, algorithmRegistry *server.AlgorithmRegistry, ip identity.IssuerPool) (*grpcServer, error) { logger, opts := log.SetupGRPCLogging() serverOpts := []grpc.ServerOption{ @@ -182,7 +182,7 @@ func createGRPCServer(cfg *config.FulcioConfig, ctClient *ctclient.LogClient, ba myServer := grpc.NewServer(serverOpts...) - grpcCAServer := server.NewGRPCCAServer(ctClient, baseca, ip) + grpcCAServer := server.NewGRPCCAServer(ctClient, baseca, algorithmRegistry, ip) health.RegisterHealthServer(myServer, grpcCAServer) // Register your gRPC service implementations. diff --git a/cmd/app/serve.go b/cmd/app/serve.go index 265ac31ec..785f4dd5a 100644 --- a/cmd/app/serve.go +++ b/cmd/app/serve.go @@ -106,6 +106,7 @@ func newServeCmd() *cobra.Command { cmd.Flags().Duration("read-header-timeout", 10*time.Second, "The time allowed to read the headers of the requests in seconds") cmd.Flags().String("grpc-tls-certificate", "", "the certificate file to use for secure connections - only applies to grpc-port") cmd.Flags().String("grpc-tls-key", "", "the private key file to use for secure connections (without passphrase) - only applies to grpc-port") + cmd.Flags().StringSlice("client-signing-algorithms", []string{"ecdsa-sha2-256-nistp256", "ed25519"}, "the list of allowed client signing algorithms") // convert "http-host" flag to "host" and "http-port" flag to be "port" cmd.Flags().SetNormalizeFunc(func(f *pflag.FlagSet, name string) pflag.NormalizedName { @@ -204,6 +205,12 @@ func runServeCmd(cmd *cobra.Command, args []string) { //nolint: revive // Setup the logger to dev/prod log.ConfigureLogger(viper.GetString("log_type")) + algorithmConfig := viper.GetStringSlice("client-signing-algorithms") + algorithmRegistry, err := server.NewAlgorithmRegistry(algorithmConfig) + if err != nil { + log.Logger.Fatalf("error loading --client-signing-algorithms=%s: %v", algorithmConfig, err) + } + // from https://github.com/golang/glog/commit/fca8c8854093a154ff1eb580aae10276ad6b1b5f _ = flag.CommandLine.Parse([]string{}) @@ -284,7 +291,7 @@ func runServeCmd(cmd *cobra.Command, args []string) { //nolint: revive port := viper.GetInt("port") metricsPort := viper.GetInt("metrics-port") // StartDuplexServer will always return an error, log fatally if it's non-nil - if err := StartDuplexServer(ctx, cfg, ctClient, baseca, viper.GetString("host"), port, metricsPort, ip); err != http.ErrServerClosed { + if err := StartDuplexServer(ctx, cfg, ctClient, baseca, algorithmRegistry, viper.GetString("host"), port, metricsPort, ip); err != http.ErrServerClosed { log.Logger.Fatal(err) } return @@ -297,7 +304,7 @@ func runServeCmd(cmd *cobra.Command, args []string) { //nolint: revive reg := prometheus.NewRegistry() - grpcServer, err := createGRPCServer(cfg, ctClient, baseca, ip) + grpcServer, err := createGRPCServer(cfg, ctClient, baseca, algorithmRegistry, ip) if err != nil { log.Logger.Fatal(err) } @@ -375,7 +382,7 @@ func checkServeCmdConfigFile() error { return nil } -func StartDuplexServer(ctx context.Context, cfg *config.FulcioConfig, ctClient *ctclient.LogClient, baseca certauth.CertificateAuthority, host string, port, metricsPort int, ip identity.IssuerPool) error { +func StartDuplexServer(ctx context.Context, cfg *config.FulcioConfig, ctClient *ctclient.LogClient, baseca certauth.CertificateAuthority, algorithmRegistry *server.AlgorithmRegistry, host string, port, metricsPort int, ip identity.IssuerPool) error { logger, opts := log.SetupGRPCLogging() d := duplex.New( @@ -394,7 +401,7 @@ func StartDuplexServer(ctx context.Context, cfg *config.FulcioConfig, ctClient * ) // GRPC server - grpcCAServer := server.NewGRPCCAServer(ctClient, baseca, ip) + grpcCAServer := server.NewGRPCCAServer(ctClient, baseca, algorithmRegistry, ip) protobuf.RegisterCAServer(d.Server, grpcCAServer) if err := d.RegisterHandler(ctx, protobuf.RegisterCAHandlerFromEndpoint); err != nil { return fmt.Errorf("registering grpc ca handler: %w", err) diff --git a/pkg/server/algorithm_registry.go b/pkg/server/algorithm_registry.go new file mode 100644 index 000000000..7eeddf473 --- /dev/null +++ b/pkg/server/algorithm_registry.go @@ -0,0 +1,63 @@ +package server + +import ( + "crypto" + "crypto/ecdsa" + "crypto/ed25519" + "fmt" +) + +type Algorithm interface { + CheckKeyAlgorithm(crypto.PublicKey) bool + CheckHashAlgorithm(crypto.Hash) bool +} + +type EcdsaSha256Algorithm struct{} + +func (EcdsaSha256Algorithm) CheckKeyAlgorithm(key crypto.PublicKey) bool { + _, ok := key.(*ecdsa.PublicKey) + return ok +} + +func (EcdsaSha256Algorithm) CheckHashAlgorithm(hash crypto.Hash) bool { + return hash == crypto.SHA256 +} + +type Ed25519Algorithm struct{} + +func (Ed25519Algorithm) CheckKeyAlgorithm(key crypto.PublicKey) bool { + _, ok := key.(*ed25519.PublicKey) + return ok +} + +func (Ed25519Algorithm) CheckHashAlgorithm(hash crypto.Hash) bool { + return hash == crypto.Hash(0) +} + +type AlgorithmRegistry struct { + permittedAlgorithms []Algorithm +} + +func NewAlgorithmRegistry(algorithmConfig []string) (*AlgorithmRegistry, error) { + var permittedAlgorithms []Algorithm + for _, algorithm := range algorithmConfig { + switch algorithm { + case "ecdsa-sha2-256-nistp256": + permittedAlgorithms = append(permittedAlgorithms, EcdsaSha256Algorithm{}) + case "ed25519": + permittedAlgorithms = append(permittedAlgorithms, Ed25519Algorithm{}) + default: + return nil, fmt.Errorf("unknown algorithm: %s", algorithm) + } + } + return &AlgorithmRegistry{permittedAlgorithms: permittedAlgorithms}, nil +} + +func (registry AlgorithmRegistry) CheckAlgorithm(key crypto.PublicKey, hash crypto.Hash) error { + for _, algorithm := range registry.permittedAlgorithms { + if algorithm.CheckKeyAlgorithm(key) && algorithm.CheckHashAlgorithm(hash) { + return nil + } + } + return fmt.Errorf("signing algorithm not permitted: %T, %s", key, hash) +} diff --git a/pkg/server/grpc_server.go b/pkg/server/grpc_server.go index 73e8b4072..0f2aaa265 100644 --- a/pkg/server/grpc_server.go +++ b/pkg/server/grpc_server.go @@ -18,6 +18,7 @@ package server import ( "context" "crypto" + "crypto/x509" "encoding/json" "errors" "fmt" @@ -43,11 +44,12 @@ type GRPCCAServer interface { health.HealthServer } -func NewGRPCCAServer(ct *ctclient.LogClient, ca certauth.CertificateAuthority, ip identity.IssuerPool) GRPCCAServer { +func NewGRPCCAServer(ct *ctclient.LogClient, ca certauth.CertificateAuthority, algorithmRegistry *AlgorithmRegistry, ip identity.IssuerPool) GRPCCAServer { return &grpcaCAServer{ - ct: ct, - ca: ca, - IssuerPool: ip, + ct: ct, + ca: ca, + algorithmRegistry: algorithmRegistry, + IssuerPool: ip, } } @@ -57,8 +59,9 @@ const ( type grpcaCAServer struct { fulciogrpc.UnimplementedCAServer - ct *ctclient.LogClient - ca certauth.CertificateAuthority + ct *ctclient.LogClient + ca certauth.CertificateAuthority + algorithmRegistry *AlgorithmRegistry identity.IssuerPool } @@ -87,6 +90,7 @@ func (g *grpcaCAServer) CreateSigningCertificate(ctx context.Context, request *f } var publicKey crypto.PublicKey + var hashFunc crypto.Hash // Verify caller is in possession of their private key and extract // public key from request. if len(request.GetCertificateSigningRequest()) > 0 { @@ -105,6 +109,11 @@ func (g *grpcaCAServer) CreateSigningCertificate(ctx context.Context, request *f if err := csr.CheckSignature(); err != nil { return nil, handleFulcioGRPCError(ctx, codes.InvalidArgument, err, invalidSignature) } + + hashFunc, err = getHashFuncForSignatureAlgorithm(csr.SignatureAlgorithm) + if err != nil { + return nil, handleFulcioGRPCError(ctx, codes.InvalidArgument, err, err.Error()) + } } else { // Option 2: Check the signature for proof of possession of a private key var ( @@ -132,6 +141,13 @@ func (g *grpcaCAServer) CreateSigningCertificate(ctx context.Context, request *f if err := challenges.CheckSignature(publicKey, proofOfPossession, principal.Name(ctx)); err != nil { return nil, handleFulcioGRPCError(ctx, codes.InvalidArgument, err, invalidSignature) } + + hashFunc = crypto.SHA256 + } + + // Check whether the public-key/hash algorithm combination is allowed + if err := g.algorithmRegistry.CheckAlgorithm(publicKey, hashFunc); err != nil { + return nil, handleFulcioGRPCError(ctx, codes.InvalidArgument, err, err.Error()) } var csc *certauth.CodeSigningCertificate @@ -279,3 +295,13 @@ func (g *grpcaCAServer) Check(_ context.Context, _ *health.HealthCheckRequest) ( func (g *grpcaCAServer) Watch(_ *health.HealthCheckRequest, _ health.Health_WatchServer) error { return status.Error(codes.Unimplemented, "unimplemented") } + +func getHashFuncForSignatureAlgorithm(signatureAlgorithm x509.SignatureAlgorithm) (crypto.Hash, error) { + switch signatureAlgorithm { + case x509.ECDSAWithSHA256: + return crypto.SHA256, nil + case x509.PureEd25519: + return crypto.Hash(0), nil + } + return crypto.Hash(0), fmt.Errorf("unrecognized signature algorithm: %s", signatureAlgorithm) +} From 71fe98ec54962a5b8d8801e571c92e0ca5d33a51 Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Mon, 8 Jan 2024 22:18:16 +1100 Subject: [PATCH 2/9] Fix type check Signed-off-by: Alex Cameron --- pkg/server/algorithm_registry.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/server/algorithm_registry.go b/pkg/server/algorithm_registry.go index 7eeddf473..9ef0d9286 100644 --- a/pkg/server/algorithm_registry.go +++ b/pkg/server/algorithm_registry.go @@ -26,7 +26,7 @@ func (EcdsaSha256Algorithm) CheckHashAlgorithm(hash crypto.Hash) bool { type Ed25519Algorithm struct{} func (Ed25519Algorithm) CheckKeyAlgorithm(key crypto.PublicKey) bool { - _, ok := key.(*ed25519.PublicKey) + _, ok := key.(ed25519.PublicKey) return ok } From d95ff74a85eae11b9c6eca4e5e98b59e59b3de5a Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Tue, 9 Jan 2024 19:34:29 +1100 Subject: [PATCH 3/9] Use `SupportedAlgorithm` spec type to parse configuration options Signed-off-by: Alex Cameron --- cmd/app/serve.go | 14 ++++++++++++-- go.mod | 3 +++ go.sum | 2 ++ pkg/server/algorithm_registry.go | 7 ++++--- 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/cmd/app/serve.go b/cmd/app/serve.go index 785f4dd5a..909d0ade7 100644 --- a/cmd/app/serve.go +++ b/cmd/app/serve.go @@ -22,6 +22,8 @@ import ( "errors" "flag" "fmt" + "github.com/sigstore/protobuf-specs/gen/pb-go/common/v1" + "golang.org/x/exp/maps" "net" "net/http" "os" @@ -106,7 +108,7 @@ func newServeCmd() *cobra.Command { cmd.Flags().Duration("read-header-timeout", 10*time.Second, "The time allowed to read the headers of the requests in seconds") cmd.Flags().String("grpc-tls-certificate", "", "the certificate file to use for secure connections - only applies to grpc-port") cmd.Flags().String("grpc-tls-key", "", "the private key file to use for secure connections (without passphrase) - only applies to grpc-port") - cmd.Flags().StringSlice("client-signing-algorithms", []string{"ecdsa-sha2-256-nistp256", "ed25519"}, "the list of allowed client signing algorithms") + cmd.Flags().StringSlice("client-signing-algorithms", maps.Keys(v1.SupportedAlgorithm_value), "the list of allowed client signing algorithms") // convert "http-host" flag to "host" and "http-port" flag to be "port" cmd.Flags().SetNormalizeFunc(func(f *pflag.FlagSet, name string) pflag.NormalizedName { @@ -205,7 +207,15 @@ func runServeCmd(cmd *cobra.Command, args []string) { //nolint: revive // Setup the logger to dev/prod log.ConfigureLogger(viper.GetString("log_type")) - algorithmConfig := viper.GetStringSlice("client-signing-algorithms") + algorithmStrings := viper.GetStringSlice("client-signing-algorithms") + var algorithmConfig []v1.SupportedAlgorithm + for _, s := range algorithmStrings { + algorithmValue, ok := v1.SupportedAlgorithm_value[s] + if !ok { + log.Logger.Fatalf("%s is not a valid value for \"client-signing-algorithms\". Try: %s", s, maps.Keys(v1.SupportedAlgorithm_value)) + } + algorithmConfig = append(algorithmConfig, v1.SupportedAlgorithm(algorithmValue)) + } algorithmRegistry, err := server.NewAlgorithmRegistry(algorithmConfig) if err != nil { log.Logger.Fatalf("error loading --client-signing-algorithms=%s: %v", algorithmConfig, err) diff --git a/go.mod b/go.mod index c7a76d7a4..d93b62555 100644 --- a/go.mod +++ b/go.mod @@ -126,6 +126,7 @@ require ( github.com/sagikazarmark/slog-shim v0.1.0 // indirect github.com/secure-systems-lab/go-securesystemslib v0.8.0 // indirect github.com/segmentio/ksuid v1.0.4 // indirect + github.com/sigstore/protobuf-specs v0.2.1 // indirect github.com/skratchdot/open-golang v0.0.0-20200116055534-eef842397966 // indirect github.com/sourcegraph/conc v0.3.0 // indirect github.com/spf13/afero v1.11.0 // indirect @@ -167,3 +168,5 @@ require ( sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect ) + +replace github.com/sigstore/protobuf-specs => /Users/tetsuo/Code/protobuf-specs \ No newline at end of file diff --git a/go.sum b/go.sum index c4e15aeb0..e74c9c9a3 100644 --- a/go.sum +++ b/go.sum @@ -306,6 +306,8 @@ github.com/secure-systems-lab/go-securesystemslib v0.8.0 h1:mr5An6X45Kb2nddcFlbm github.com/secure-systems-lab/go-securesystemslib v0.8.0/go.mod h1:UH2VZVuJfCYR8WgMlCU1uFsOUU+KeyrTWcSS73NBOzU= github.com/segmentio/ksuid v1.0.4 h1:sBo2BdShXjmcugAMwjugoGUdUV0pcxY5mW4xKRn3v4c= github.com/segmentio/ksuid v1.0.4/go.mod h1:/XUiZBD3kVx5SmUOl55voK5yeAbBNNIed+2O73XgrPE= +github.com/sigstore/protobuf-specs v0.2.1 h1:KIoM7E3C4uaK092q8YoSj/XSf9720f8dlsbYwwOmgEA= +github.com/sigstore/protobuf-specs v0.2.1/go.mod h1:xPqQGnH/HllKuZ4VFPz/g+78epWM/NLRGl7Fuy45UdE= github.com/sigstore/sigstore v1.8.0 h1:sSRWXv1JiDsK4T2wNWVYcvKCgxcSrhQ/QUJxsfCO4OM= github.com/sigstore/sigstore v1.8.0/go.mod h1:l12B1gFlLIpBIVeqk/q1Lb+6YSOGNuN3xLExIjYH+qc= github.com/sigstore/sigstore/pkg/signature/kms/aws v1.8.0 h1:nLaaOX85YjBKQOQHWY2UlDkbx+je8ozTEM+t1ySAb78= diff --git a/pkg/server/algorithm_registry.go b/pkg/server/algorithm_registry.go index 9ef0d9286..acb88172a 100644 --- a/pkg/server/algorithm_registry.go +++ b/pkg/server/algorithm_registry.go @@ -5,6 +5,7 @@ import ( "crypto/ecdsa" "crypto/ed25519" "fmt" + "github.com/sigstore/protobuf-specs/gen/pb-go/common/v1" ) type Algorithm interface { @@ -38,13 +39,13 @@ type AlgorithmRegistry struct { permittedAlgorithms []Algorithm } -func NewAlgorithmRegistry(algorithmConfig []string) (*AlgorithmRegistry, error) { +func NewAlgorithmRegistry(algorithmConfig []v1.SupportedAlgorithm) (*AlgorithmRegistry, error) { var permittedAlgorithms []Algorithm for _, algorithm := range algorithmConfig { switch algorithm { - case "ecdsa-sha2-256-nistp256": + case v1.SupportedAlgorithm_ECDSA_SHA2_256_NISTP256: permittedAlgorithms = append(permittedAlgorithms, EcdsaSha256Algorithm{}) - case "ed25519": + case v1.SupportedAlgorithm_ED25519: permittedAlgorithms = append(permittedAlgorithms, Ed25519Algorithm{}) default: return nil, fmt.Errorf("unknown algorithm: %s", algorithm) From 6208232c97e5e7c8093e88ec7e2df91da848954c Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Wed, 10 Jan 2024 23:39:48 +1100 Subject: [PATCH 4/9] Override the SHA-256 default in the case of ED25519 Signed-off-by: Alex Cameron --- pkg/server/grpc_server.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/server/grpc_server.go b/pkg/server/grpc_server.go index 0f2aaa265..29f37ac4a 100644 --- a/pkg/server/grpc_server.go +++ b/pkg/server/grpc_server.go @@ -18,6 +18,7 @@ package server import ( "context" "crypto" + "crypto/ed25519" "crypto/x509" "encoding/json" "errors" @@ -142,7 +143,11 @@ func (g *grpcaCAServer) CreateSigningCertificate(ctx context.Context, request *f return nil, handleFulcioGRPCError(ctx, codes.InvalidArgument, err, invalidSignature) } + // The proof of possession signature always uses SHA-256, unless the key algorithm is ED25519 hashFunc = crypto.SHA256 + if _, ok := publicKey.(*ed25519.PublicKey); ok { + hashFunc = crypto.Hash(0) + } } // Check whether the public-key/hash algorithm combination is allowed From ed2c983d0e78c1490099f6fcc2112ad5aeebbccd Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Mon, 29 Jan 2024 01:04:14 +1100 Subject: [PATCH 5/9] Update to use `AlgorithmRegistryConfig` Signed-off-by: Alex Cameron --- cmd/app/grpc.go | 3 +- cmd/app/serve.go | 30 ++++++++++----- go.mod | 2 +- pkg/server/algorithm_registry.go | 64 -------------------------------- pkg/server/grpc_server.go | 12 ++++-- 5 files changed, 33 insertions(+), 78 deletions(-) delete mode 100644 pkg/server/algorithm_registry.go diff --git a/cmd/app/grpc.go b/cmd/app/grpc.go index a7abf272c..5ce4d7f64 100644 --- a/cmd/app/grpc.go +++ b/cmd/app/grpc.go @@ -19,6 +19,7 @@ import ( "context" "crypto/tls" "fmt" + "github.com/sigstore/sigstore/pkg/signature" "net" "os" "os/signal" @@ -154,7 +155,7 @@ func (c *cachedTLSCert) GRPCCreds() grpc.ServerOption { })) } -func createGRPCServer(cfg *config.FulcioConfig, ctClient *ctclient.LogClient, baseca ca.CertificateAuthority, algorithmRegistry *server.AlgorithmRegistry, ip identity.IssuerPool) (*grpcServer, error) { +func createGRPCServer(cfg *config.FulcioConfig, ctClient *ctclient.LogClient, baseca ca.CertificateAuthority, algorithmRegistry *signature.AlgorithmRegistryConfig, ip identity.IssuerPool) (*grpcServer, error) { logger, opts := log.SetupGRPCLogging() serverOpts := []grpc.ServerOption{ diff --git a/cmd/app/serve.go b/cmd/app/serve.go index 909d0ade7..20cf3a4ee 100644 --- a/cmd/app/serve.go +++ b/cmd/app/serve.go @@ -23,7 +23,7 @@ import ( "flag" "fmt" "github.com/sigstore/protobuf-specs/gen/pb-go/common/v1" - "golang.org/x/exp/maps" + "github.com/sigstore/sigstore/pkg/signature" "net" "net/http" "os" @@ -108,7 +108,7 @@ func newServeCmd() *cobra.Command { cmd.Flags().Duration("read-header-timeout", 10*time.Second, "The time allowed to read the headers of the requests in seconds") cmd.Flags().String("grpc-tls-certificate", "", "the certificate file to use for secure connections - only applies to grpc-port") cmd.Flags().String("grpc-tls-key", "", "the private key file to use for secure connections (without passphrase) - only applies to grpc-port") - cmd.Flags().StringSlice("client-signing-algorithms", maps.Keys(v1.SupportedAlgorithm_value), "the list of allowed client signing algorithms") + cmd.Flags().StringSlice("client-signing-algorithms", buildDefaultClientSigningAlgorithms([]v1.KnownSignatureAlgorithm{v1.KnownSignatureAlgorithm_ECDSA_SHA2_256_NISTP256, v1.KnownSignatureAlgorithm_ECDSA_SHA2_384_NISTP384, v1.KnownSignatureAlgorithm_ECDSA_SHA2_512_NISTP521, v1.KnownSignatureAlgorithm_ED25519}), "the list of allowed client signing algorithms") // convert "http-host" flag to "host" and "http-port" flag to be "port" cmd.Flags().SetNormalizeFunc(func(f *pflag.FlagSet, name string) pflag.NormalizedName { @@ -208,15 +208,15 @@ func runServeCmd(cmd *cobra.Command, args []string) { //nolint: revive log.ConfigureLogger(viper.GetString("log_type")) algorithmStrings := viper.GetStringSlice("client-signing-algorithms") - var algorithmConfig []v1.SupportedAlgorithm + var algorithmConfig []v1.KnownSignatureAlgorithm for _, s := range algorithmStrings { - algorithmValue, ok := v1.SupportedAlgorithm_value[s] - if !ok { - log.Logger.Fatalf("%s is not a valid value for \"client-signing-algorithms\". Try: %s", s, maps.Keys(v1.SupportedAlgorithm_value)) + algorithmValue, err := signature.ParseSignatureAlgorithmFlag(s) + if err != nil { + log.Logger.Fatal(err) } - algorithmConfig = append(algorithmConfig, v1.SupportedAlgorithm(algorithmValue)) + algorithmConfig = append(algorithmConfig, algorithmValue) } - algorithmRegistry, err := server.NewAlgorithmRegistry(algorithmConfig) + algorithmRegistry, err := signature.NewAlgorithmRegistryConfig(algorithmConfig) if err != nil { log.Logger.Fatalf("error loading --client-signing-algorithms=%s: %v", algorithmConfig, err) } @@ -392,7 +392,7 @@ func checkServeCmdConfigFile() error { return nil } -func StartDuplexServer(ctx context.Context, cfg *config.FulcioConfig, ctClient *ctclient.LogClient, baseca certauth.CertificateAuthority, algorithmRegistry *server.AlgorithmRegistry, host string, port, metricsPort int, ip identity.IssuerPool) error { +func StartDuplexServer(ctx context.Context, cfg *config.FulcioConfig, ctClient *ctclient.LogClient, baseca certauth.CertificateAuthority, algorithmRegistry *signature.AlgorithmRegistryConfig, host string, port, metricsPort int, ip identity.IssuerPool) error { logger, opts := log.SetupGRPCLogging() d := duplex.New( @@ -444,3 +444,15 @@ func StartDuplexServer(ctx context.Context, cfg *config.FulcioConfig, ctClient * } return nil } + +func buildDefaultClientSigningAlgorithms(allowedAlgorithms []v1.KnownSignatureAlgorithm) []string { + var algorithmStrings []string + for _, algorithm := range allowedAlgorithms { + algorithmString, err := signature.FormatSignatureAlgorithmFlag(algorithm) + if err != nil { + log.Logger.Fatal(err) + } + algorithmStrings = append(algorithmStrings, algorithmString) + } + return algorithmStrings +} diff --git a/go.mod b/go.mod index c419a22cd..65d5a696b 100644 --- a/go.mod +++ b/go.mod @@ -38,7 +38,6 @@ require ( github.com/spiffe/go-spiffe/v2 v2.1.6 go.step.sm/crypto v0.42.0 go.uber.org/zap v1.26.0 - golang.org/x/exp v0.0.0-20230905200255-921286631fa9 google.golang.org/api v0.157.0 google.golang.org/genproto/googleapis/api v0.0.0-20240102182953-50ed04b92917 google.golang.org/grpc v1.60.1 @@ -148,6 +147,7 @@ require ( go.uber.org/multierr v1.11.0 // indirect goa.design/goa v2.2.5+incompatible // indirect golang.org/x/crypto v0.18.0 // indirect + golang.org/x/exp v0.0.0-20230905200255-921286631fa9 // indirect golang.org/x/net v0.20.0 // indirect golang.org/x/oauth2 v0.16.0 // indirect golang.org/x/sync v0.6.0 // indirect diff --git a/pkg/server/algorithm_registry.go b/pkg/server/algorithm_registry.go deleted file mode 100644 index acb88172a..000000000 --- a/pkg/server/algorithm_registry.go +++ /dev/null @@ -1,64 +0,0 @@ -package server - -import ( - "crypto" - "crypto/ecdsa" - "crypto/ed25519" - "fmt" - "github.com/sigstore/protobuf-specs/gen/pb-go/common/v1" -) - -type Algorithm interface { - CheckKeyAlgorithm(crypto.PublicKey) bool - CheckHashAlgorithm(crypto.Hash) bool -} - -type EcdsaSha256Algorithm struct{} - -func (EcdsaSha256Algorithm) CheckKeyAlgorithm(key crypto.PublicKey) bool { - _, ok := key.(*ecdsa.PublicKey) - return ok -} - -func (EcdsaSha256Algorithm) CheckHashAlgorithm(hash crypto.Hash) bool { - return hash == crypto.SHA256 -} - -type Ed25519Algorithm struct{} - -func (Ed25519Algorithm) CheckKeyAlgorithm(key crypto.PublicKey) bool { - _, ok := key.(ed25519.PublicKey) - return ok -} - -func (Ed25519Algorithm) CheckHashAlgorithm(hash crypto.Hash) bool { - return hash == crypto.Hash(0) -} - -type AlgorithmRegistry struct { - permittedAlgorithms []Algorithm -} - -func NewAlgorithmRegistry(algorithmConfig []v1.SupportedAlgorithm) (*AlgorithmRegistry, error) { - var permittedAlgorithms []Algorithm - for _, algorithm := range algorithmConfig { - switch algorithm { - case v1.SupportedAlgorithm_ECDSA_SHA2_256_NISTP256: - permittedAlgorithms = append(permittedAlgorithms, EcdsaSha256Algorithm{}) - case v1.SupportedAlgorithm_ED25519: - permittedAlgorithms = append(permittedAlgorithms, Ed25519Algorithm{}) - default: - return nil, fmt.Errorf("unknown algorithm: %s", algorithm) - } - } - return &AlgorithmRegistry{permittedAlgorithms: permittedAlgorithms}, nil -} - -func (registry AlgorithmRegistry) CheckAlgorithm(key crypto.PublicKey, hash crypto.Hash) error { - for _, algorithm := range registry.permittedAlgorithms { - if algorithm.CheckKeyAlgorithm(key) && algorithm.CheckHashAlgorithm(hash) { - return nil - } - } - return fmt.Errorf("signing algorithm not permitted: %T, %s", key, hash) -} diff --git a/pkg/server/grpc_server.go b/pkg/server/grpc_server.go index 29f37ac4a..48b08f0ab 100644 --- a/pkg/server/grpc_server.go +++ b/pkg/server/grpc_server.go @@ -23,6 +23,7 @@ import ( "encoding/json" "errors" "fmt" + "github.com/sigstore/sigstore/pkg/signature" health "google.golang.org/grpc/health/grpc_health_v1" @@ -45,7 +46,7 @@ type GRPCCAServer interface { health.HealthServer } -func NewGRPCCAServer(ct *ctclient.LogClient, ca certauth.CertificateAuthority, algorithmRegistry *AlgorithmRegistry, ip identity.IssuerPool) GRPCCAServer { +func NewGRPCCAServer(ct *ctclient.LogClient, ca certauth.CertificateAuthority, algorithmRegistry *signature.AlgorithmRegistryConfig, ip identity.IssuerPool) GRPCCAServer { return &grpcaCAServer{ ct: ct, ca: ca, @@ -62,7 +63,7 @@ type grpcaCAServer struct { fulciogrpc.UnimplementedCAServer ct *ctclient.LogClient ca certauth.CertificateAuthority - algorithmRegistry *AlgorithmRegistry + algorithmRegistry *signature.AlgorithmRegistryConfig identity.IssuerPool } @@ -151,7 +152,12 @@ func (g *grpcaCAServer) CreateSigningCertificate(ctx context.Context, request *f } // Check whether the public-key/hash algorithm combination is allowed - if err := g.algorithmRegistry.CheckAlgorithm(publicKey, hashFunc); err != nil { + isPermitted, err := g.algorithmRegistry.IsAlgorithmPermitted(publicKey, hashFunc) + if err != nil { + return nil, handleFulcioGRPCError(ctx, codes.InvalidArgument, err, err.Error()) + } + if !isPermitted { + err = fmt.Errorf("Signing algorithm not permitted: %T, %s", publicKey, hashFunc) return nil, handleFulcioGRPCError(ctx, codes.InvalidArgument, err, err.Error()) } From a4c6e1f3f82e2325108083ec75016cc205a9472f Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Mon, 29 Jan 2024 01:10:16 +1100 Subject: [PATCH 6/9] Add missing signature algorithms to `getHashFuncForSignatureAlgorithm` Signed-off-by: Alex Cameron --- pkg/server/grpc_server.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/server/grpc_server.go b/pkg/server/grpc_server.go index 48b08f0ab..9e967900a 100644 --- a/pkg/server/grpc_server.go +++ b/pkg/server/grpc_server.go @@ -310,7 +310,14 @@ func (g *grpcaCAServer) Watch(_ *health.HealthCheckRequest, _ health.Health_Watc func getHashFuncForSignatureAlgorithm(signatureAlgorithm x509.SignatureAlgorithm) (crypto.Hash, error) { switch signatureAlgorithm { case x509.ECDSAWithSHA256: + case x509.SHA256WithRSA: return crypto.SHA256, nil + case x509.ECDSAWithSHA384: + case x509.SHA384WithRSA: + return crypto.SHA384, nil + case x509.ECDSAWithSHA512: + case x509.SHA512WithRSA: + return crypto.SHA512, nil case x509.PureEd25519: return crypto.Hash(0), nil } From bde6e2752b86a97db34e52e5dab32709138ebdfa Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Mon, 29 Jan 2024 12:24:11 +1100 Subject: [PATCH 7/9] Get unit tests passing Signed-off-by: Alex Cameron --- cmd/app/grpc.go | 3 ++- cmd/app/http_test.go | 14 ++++++++++++-- cmd/app/serve.go | 5 +++-- cmd/app/serve_test.go | 8 +++++++- pkg/server/grpc_server.go | 8 ++++++-- pkg/server/grpc_server_test.go | 8 +++++++- 6 files changed, 37 insertions(+), 9 deletions(-) diff --git a/cmd/app/grpc.go b/cmd/app/grpc.go index 5ce4d7f64..b4167c4af 100644 --- a/cmd/app/grpc.go +++ b/cmd/app/grpc.go @@ -19,7 +19,6 @@ import ( "context" "crypto/tls" "fmt" - "github.com/sigstore/sigstore/pkg/signature" "net" "os" "os/signal" @@ -27,6 +26,8 @@ import ( "sync" "syscall" + "github.com/sigstore/sigstore/pkg/signature" + "github.com/fsnotify/fsnotify" "github.com/goadesign/goa/grpc/middleware" ctclient "github.com/google/certificate-transparency-go/client" diff --git a/cmd/app/http_test.go b/cmd/app/http_test.go index debb70036..6fc78fa61 100644 --- a/cmd/app/http_test.go +++ b/cmd/app/http_test.go @@ -32,6 +32,8 @@ import ( "github.com/sigstore/fulcio/pkg/ca" "github.com/sigstore/fulcio/pkg/identity" + v1 "github.com/sigstore/protobuf-specs/gen/pb-go/common/v1" + "github.com/sigstore/sigstore/pkg/signature" "github.com/spf13/viper" "google.golang.org/grpc" @@ -47,7 +49,11 @@ func setupHTTPServer(t *testing.T) (httpServer, string) { viper.Set("grpc-host", "") viper.Set("grpc-port", 0) - grpcServer, err := createGRPCServer(nil, nil, &TrivialCertificateAuthority{}, nil) + algorithmRegistry, err := signature.NewAlgorithmRegistryConfig([]v1.KnownSignatureAlgorithm{}) + if err != nil { + t.Error(err) + } + grpcServer, err := createGRPCServer(nil, nil, &TrivialCertificateAuthority{}, algorithmRegistry, nil) if err != nil { t.Error(err) } @@ -93,7 +99,11 @@ func setupHTTPServerWithGRPCTLS(t *testing.T) (httpServer, string) { viper.Set("grpc-host", "") viper.Set("grpc-port", 0) - grpcServer, err := createGRPCServer(nil, nil, &TrivialCertificateAuthority{}, nil) + algorithmRegistry, err := signature.NewAlgorithmRegistryConfig([]v1.KnownSignatureAlgorithm{}) + if err != nil { + t.Error(err) + } + grpcServer, err := createGRPCServer(nil, nil, &TrivialCertificateAuthority{}, algorithmRegistry, nil) if err != nil { t.Error(err) } diff --git a/cmd/app/serve.go b/cmd/app/serve.go index 20cf3a4ee..4cabd54fe 100644 --- a/cmd/app/serve.go +++ b/cmd/app/serve.go @@ -22,8 +22,6 @@ import ( "errors" "flag" "fmt" - "github.com/sigstore/protobuf-specs/gen/pb-go/common/v1" - "github.com/sigstore/sigstore/pkg/signature" "net" "net/http" "os" @@ -34,6 +32,9 @@ import ( "syscall" "time" + v1 "github.com/sigstore/protobuf-specs/gen/pb-go/common/v1" + "github.com/sigstore/sigstore/pkg/signature" + "chainguard.dev/go-grpc-kit/pkg/duplex" "github.com/goadesign/goa/grpc/middleware" ctclient "github.com/google/certificate-transparency-go/client" diff --git a/cmd/app/serve_test.go b/cmd/app/serve_test.go index b3e82fa43..22ae6eaa0 100644 --- a/cmd/app/serve_test.go +++ b/cmd/app/serve_test.go @@ -31,6 +31,8 @@ import ( "github.com/sigstore/fulcio/pkg/ca/ephemeralca" "github.com/sigstore/fulcio/pkg/config" "github.com/sigstore/fulcio/pkg/generated/protobuf" + v1 "github.com/sigstore/protobuf-specs/gen/pb-go/common/v1" + "github.com/sigstore/sigstore/pkg/signature" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" ) @@ -48,9 +50,13 @@ func TestDuplex(t *testing.T) { t.Fatal(err) } metricsPort := 2114 + algorithmRegistry, err := signature.NewAlgorithmRegistryConfig([]v1.KnownSignatureAlgorithm{}) + if err != nil { + t.Error(err) + } go func() { - if err := StartDuplexServer(ctx, config.DefaultConfig, nil, ca, "localhost", port, metricsPort, nil); err != nil { + if err := StartDuplexServer(ctx, config.DefaultConfig, nil, ca, algorithmRegistry, "localhost", port, metricsPort, nil); err != nil { log.Fatalf("error starting duplex server: %v", err) } }() diff --git a/pkg/server/grpc_server.go b/pkg/server/grpc_server.go index 9e967900a..66129ccae 100644 --- a/pkg/server/grpc_server.go +++ b/pkg/server/grpc_server.go @@ -23,6 +23,7 @@ import ( "encoding/json" "errors" "fmt" + "github.com/sigstore/sigstore/pkg/signature" health "google.golang.org/grpc/health/grpc_health_v1" @@ -310,12 +311,15 @@ func (g *grpcaCAServer) Watch(_ *health.HealthCheckRequest, _ health.Health_Watc func getHashFuncForSignatureAlgorithm(signatureAlgorithm x509.SignatureAlgorithm) (crypto.Hash, error) { switch signatureAlgorithm { case x509.ECDSAWithSHA256: - case x509.SHA256WithRSA: return crypto.SHA256, nil case x509.ECDSAWithSHA384: - case x509.SHA384WithRSA: return crypto.SHA384, nil case x509.ECDSAWithSHA512: + return crypto.SHA512, nil + case x509.SHA256WithRSA: + return crypto.SHA256, nil + case x509.SHA384WithRSA: + return crypto.SHA384, nil case x509.SHA512WithRSA: return crypto.SHA512, nil case x509.PureEd25519: diff --git a/pkg/server/grpc_server_test.go b/pkg/server/grpc_server_test.go index 4558277e0..757cb8883 100644 --- a/pkg/server/grpc_server_test.go +++ b/pkg/server/grpc_server_test.go @@ -46,7 +46,9 @@ import ( "github.com/sigstore/fulcio/pkg/config" "github.com/sigstore/fulcio/pkg/generated/protobuf" "github.com/sigstore/fulcio/pkg/identity" + v1 "github.com/sigstore/protobuf-specs/gen/pb-go/common/v1" "github.com/sigstore/sigstore/pkg/cryptoutils" + "github.com/sigstore/sigstore/pkg/signature" "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials/insecure" @@ -84,7 +86,11 @@ func setupGRPCForTest(ctx context.Context, t *testing.T, cfg *config.FulcioConfi lis = bufconn.Listen(bufSize) s := grpc.NewServer(grpc.UnaryInterceptor(passFulcioConfigThruContext(cfg))) ip := NewIssuerPool(cfg) - protobuf.RegisterCAServer(s, NewGRPCCAServer(ctl, ca, ip)) + algorithmRegistry, err := signature.NewAlgorithmRegistryConfig([]v1.KnownSignatureAlgorithm{v1.KnownSignatureAlgorithm_ECDSA_SHA2_256_NISTP256}) + if err != nil { + t.Error(err) + } + protobuf.RegisterCAServer(s, NewGRPCCAServer(ctl, ca, algorithmRegistry, ip)) go func() { if err := s.Serve(lis); err != nil && !errors.Is(err, grpc.ErrServerStopped) { t.Errorf("Server exited with error: %v", err) From bda9c016408306ade1ed25baf20c76ef52627588 Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Mon, 29 Jan 2024 17:03:15 +1100 Subject: [PATCH 8/9] Add CSR unit tests Signed-off-by: Alex Cameron --- pkg/server/grpc_server_test.go | 164 ++++++++++++++++++++++++++++++++- 1 file changed, 161 insertions(+), 3 deletions(-) diff --git a/pkg/server/grpc_server_test.go b/pkg/server/grpc_server_test.go index 757cb8883..f35176cbc 100644 --- a/pkg/server/grpc_server_test.go +++ b/pkg/server/grpc_server_test.go @@ -86,7 +86,7 @@ func setupGRPCForTest(ctx context.Context, t *testing.T, cfg *config.FulcioConfi lis = bufconn.Listen(bufSize) s := grpc.NewServer(grpc.UnaryInterceptor(passFulcioConfigThruContext(cfg))) ip := NewIssuerPool(cfg) - algorithmRegistry, err := signature.NewAlgorithmRegistryConfig([]v1.KnownSignatureAlgorithm{v1.KnownSignatureAlgorithm_ECDSA_SHA2_256_NISTP256}) + algorithmRegistry, err := signature.NewAlgorithmRegistryConfig([]v1.KnownSignatureAlgorithm{v1.KnownSignatureAlgorithm_ECDSA_SHA2_256_NISTP256, v1.KnownSignatureAlgorithm_RSA_SIGN_PKCS1_2048_SHA256}) if err != nil { t.Error(err) } @@ -1188,8 +1188,8 @@ func TestAPIWithIssuerClaimConfig(t *testing.T) { } } -// Tests API with challenge sent as CSR -func TestAPIWithCSRChallenge(t *testing.T) { +// Tests API with challenge sent as CSR with an ECDSA key +func TestAPIWithCSRChallengeECDSA(t *testing.T) { emailSigner, emailIssuer := newOIDCIssuer(t) // Create a FulcioConfig that supports this issuer. @@ -1270,6 +1270,89 @@ func TestAPIWithCSRChallenge(t *testing.T) { } } +// Tests API with challenge sent as CSR with an RSA key +func TestAPIWithCSRChallengeRSA(t *testing.T) { + emailSigner, emailIssuer := newOIDCIssuer(t) + + // Create a FulcioConfig that supports these issuers. + cfg, err := config.Read([]byte(fmt.Sprintf(`{ + "OIDCIssuers": { + %q: { + "IssuerURL": %q, + "ClientID": "sigstore", + "Type": "email" + } + } + }`, emailIssuer, emailIssuer))) + if err != nil { + t.Fatalf("config.Read() = %v", err) + } + + emailSubject := "foo@example.com" + + // Create an OIDC token using this issuer's signer. + tok, err := jwt.Signed(emailSigner).Claims(jwt.Claims{ + Issuer: emailIssuer, + IssuedAt: jwt.NewNumericDate(time.Now()), + Expiry: jwt.NewNumericDate(time.Now().Add(30 * time.Minute)), + Subject: emailSubject, + Audience: jwt.Audience{"sigstore"}, + }).Claims(customClaims{Email: emailSubject, EmailVerified: true}).CompactSerialize() + if err != nil { + t.Fatalf("CompactSerialize() = %v", err) + } + + ctClient, eca := createCA(cfg, t) + ctx := context.Background() + server, conn := setupGRPCForTest(ctx, t, cfg, ctClient, eca) + defer func() { + server.Stop() + conn.Close() + }() + + client := protobuf.NewCAClient(conn) + + priv, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("GenerateKey() = %v", err) + } + csrTmpl := &x509.CertificateRequest{Subject: pkix.Name{CommonName: "test"}} + derCSR, err := x509.CreateCertificateRequest(rand.Reader, csrTmpl, priv) + if err != nil { + t.Fatalf("error creating CSR: %v", err) + } + pemCSR := pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE REQUEST", + Bytes: derCSR, + }) + + // Hit the API to have it sign our certificate. + // Hit the API to have it sign our certificate. + resp, err := client.CreateSigningCertificate(ctx, &protobuf.CreateSigningCertificateRequest{ + Credentials: &protobuf.Credentials{ + Credentials: &protobuf.Credentials_OidcIdentityToken{ + OidcIdentityToken: tok, + }, + }, + Key: &protobuf.CreateSigningCertificateRequest_CertificateSigningRequest{ + CertificateSigningRequest: pemCSR, + }, + }) + if err != nil { + t.Fatalf("SigningCert() = %v", err) + } + + leafCert := verifyResponse(resp, eca, emailIssuer, t) + + // Expect email subject + if len(leafCert.EmailAddresses) != 1 { + t.Fatalf("unexpected length of leaf certificate URIs, expected 1, got %d", len(leafCert.URIs)) + } + if leafCert.EmailAddresses[0] != emailSubject { + t.Fatalf("subjects do not match: Expected %v, got %v", emailSubject, leafCert.EmailAddresses[0]) + } +} + // Tests API with insecure pub key func TestAPIWithInsecurePublicKey(t *testing.T) { emailSigner, emailIssuer := newOIDCIssuer(t) @@ -1628,6 +1711,81 @@ func TestAPIWithInvalidCSRSignature(t *testing.T) { } } +// Tests API with CSR, containing ECDSA key with unpermitted curve +func TestAPIWithInvalidCSRPublicKey(t *testing.T) { + emailSigner, emailIssuer := newOIDCIssuer(t) + + // Create a FulcioConfig that supports this issuer. + cfg, err := config.Read([]byte(fmt.Sprintf(`{ + "OIDCIssuers": { + %q: { + "IssuerURL": %q, + "ClientID": "sigstore", + "Type": "email" + } + } + }`, emailIssuer, emailIssuer))) + if err != nil { + t.Fatalf("config.Read() = %v", err) + } + + emailSubject := "foo@example.com" + + // Create an OIDC token using this issuer's signer. + tok, err := jwt.Signed(emailSigner).Claims(jwt.Claims{ + Issuer: emailIssuer, + IssuedAt: jwt.NewNumericDate(time.Now()), + Expiry: jwt.NewNumericDate(time.Now().Add(30 * time.Minute)), + Subject: emailSubject, + Audience: jwt.Audience{"sigstore"}, + }).Claims(customClaims{Email: emailSubject, EmailVerified: true}).CompactSerialize() + if err != nil { + t.Fatalf("CompactSerialize() = %v", err) + } + + ctClient, eca := createCA(cfg, t) + ctx := context.Background() + server, conn := setupGRPCForTest(ctx, t, cfg, ctClient, eca) + defer func() { + server.Stop() + conn.Close() + }() + + client := protobuf.NewCAClient(conn) + + priv, err := ecdsa.GenerateKey(elliptic.P521(), rand.Reader) + if err != nil { + t.Fatalf("error generating private key: %v", err) + } + csrTmpl := &x509.CertificateRequest{Subject: pkix.Name{CommonName: "test"}} + derCSR, err := x509.CreateCertificateRequest(rand.Reader, csrTmpl, priv) + if err != nil { + t.Fatalf("error creating CSR: %v", err) + } + pemCSR := pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE REQUEST", + Bytes: derCSR, + }) + + // Hit the API to have it sign our certificate. + _, err = client.CreateSigningCertificate(ctx, &protobuf.CreateSigningCertificateRequest{ + Credentials: &protobuf.Credentials{ + Credentials: &protobuf.Credentials_OidcIdentityToken{ + OidcIdentityToken: tok, + }, + }, + Key: &protobuf.CreateSigningCertificateRequest_CertificateSigningRequest{ + CertificateSigningRequest: pemCSR, + }, + }) + if err == nil || !strings.Contains(err.Error(), "Signing algorithm not permitted") { + t.Fatalf("expected signing algorithm not permitted, got %v", err) + } + if status.Code(err) != codes.InvalidArgument { + t.Fatalf("expected invalid argument, got %v", status.Code(err)) + } +} + // Stand up a very simple OIDC endpoint. func newOIDCIssuer(t *testing.T) (jose.Signer, string) { t.Helper() From 05a012b0dfa1d654770561b9f67564621d96aa86 Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Mon, 29 Jan 2024 23:07:18 +1100 Subject: [PATCH 9/9] Add non-CSR unit tests Signed-off-by: Alex Cameron --- pkg/server/grpc_server_test.go | 173 ++++++++++++++++++++++++++++++++- 1 file changed, 171 insertions(+), 2 deletions(-) diff --git a/pkg/server/grpc_server_test.go b/pkg/server/grpc_server_test.go index f35176cbc..d32c9fa16 100644 --- a/pkg/server/grpc_server_test.go +++ b/pkg/server/grpc_server_test.go @@ -1188,8 +1188,96 @@ func TestAPIWithIssuerClaimConfig(t *testing.T) { } } -// Tests API with challenge sent as CSR with an ECDSA key -func TestAPIWithCSRChallengeECDSA(t *testing.T) { +// Tests API with an RSA key +func TestAPIWithRSA(t *testing.T) { + emailSigner, emailIssuer := newOIDCIssuer(t) + + // Create a FulcioConfig that supports these issuers. + cfg, err := config.Read([]byte(fmt.Sprintf(`{ + "OIDCIssuers": { + %q: { + "IssuerURL": %q, + "ClientID": "sigstore", + "Type": "email" + } + } + }`, emailIssuer, emailIssuer))) + if err != nil { + t.Fatalf("config.Read() = %v", err) + } + + emailSubject := "foo@example.com" + + // Create an OIDC token using this issuer's signer. + tok, err := jwt.Signed(emailSigner).Claims(jwt.Claims{ + Issuer: emailIssuer, + IssuedAt: jwt.NewNumericDate(time.Now()), + Expiry: jwt.NewNumericDate(time.Now().Add(30 * time.Minute)), + Subject: emailSubject, + Audience: jwt.Audience{"sigstore"}, + }).Claims(customClaims{Email: emailSubject, EmailVerified: true}).CompactSerialize() + if err != nil { + t.Fatalf("CompactSerialize() = %v", err) + } + + ctClient, eca := createCA(cfg, t) + ctx := context.Background() + server, conn := setupGRPCForTest(ctx, t, cfg, ctClient, eca) + defer func() { + server.Stop() + conn.Close() + }() + + client := protobuf.NewCAClient(conn) + + priv, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("GenerateKey() = %v", err) + } + pubBytes, err := x509.MarshalPKIXPublicKey(&priv.PublicKey) + if err != nil { + t.Fatalf("x509.MarshalPKIXPublicKey() = %v", err) + } + hash := sha256.Sum256([]byte(emailSubject)) + proof, err := rsa.SignPKCS1v15(rand.Reader, priv, crypto.SHA256, hash[:]) + if err != nil { + t.Fatalf("SignPKCS1v15() = %v", err) + } + pemBytes := string(cryptoutils.PEMEncode(cryptoutils.PublicKeyPEMType, pubBytes)) + + // Hit the API to have it sign our certificate. + resp, err := client.CreateSigningCertificate(ctx, &protobuf.CreateSigningCertificateRequest{ + Credentials: &protobuf.Credentials{ + Credentials: &protobuf.Credentials_OidcIdentityToken{ + OidcIdentityToken: tok, + }, + }, + Key: &protobuf.CreateSigningCertificateRequest_PublicKeyRequest{ + PublicKeyRequest: &protobuf.PublicKeyRequest{ + PublicKey: &protobuf.PublicKey{ + Content: pemBytes, + }, + ProofOfPossession: proof, + }, + }, + }) + if err != nil { + t.Fatalf("SigningCert() = %v", err) + } + + leafCert := verifyResponse(resp, eca, emailIssuer, t) + + // Expect email subject + if len(leafCert.EmailAddresses) != 1 { + t.Fatalf("unexpected length of leaf certificate URIs, expected 1, got %d", len(leafCert.URIs)) + } + if leafCert.EmailAddresses[0] != emailSubject { + t.Fatalf("subjects do not match: Expected %v, got %v", emailSubject, leafCert.EmailAddresses[0]) + } +} + +// Tests API with challenge sent as CSR +func TestAPIWithCSRChallenge(t *testing.T) { emailSigner, emailIssuer := newOIDCIssuer(t) // Create a FulcioConfig that supports this issuer. @@ -1572,6 +1660,87 @@ func TestAPIWithInvalidChallenge(t *testing.T) { } } +// Tests API with an ECDSA key with an unpermitted curve +func TestAPIWithInvalidPublicKey(t *testing.T) { + emailSigner, emailIssuer := newOIDCIssuer(t) + + // Create a FulcioConfig that supports these issuers. + cfg, err := config.Read([]byte(fmt.Sprintf(`{ + "OIDCIssuers": { + %q: { + "IssuerURL": %q, + "ClientID": "sigstore", + "Type": "email" + } + } + }`, emailIssuer, emailIssuer))) + if err != nil { + t.Fatalf("config.Read() = %v", err) + } + + emailSubject := "foo@example.com" + + // Create an OIDC token using this issuer's signer. + tok, err := jwt.Signed(emailSigner).Claims(jwt.Claims{ + Issuer: emailIssuer, + IssuedAt: jwt.NewNumericDate(time.Now()), + Expiry: jwt.NewNumericDate(time.Now().Add(30 * time.Minute)), + Subject: emailSubject, + Audience: jwt.Audience{"sigstore"}, + }).Claims(customClaims{Email: emailSubject, EmailVerified: true}).CompactSerialize() + if err != nil { + t.Fatalf("CompactSerialize() = %v", err) + } + + ctClient, eca := createCA(cfg, t) + ctx := context.Background() + server, conn := setupGRPCForTest(ctx, t, cfg, ctClient, eca) + defer func() { + server.Stop() + conn.Close() + }() + + client := protobuf.NewCAClient(conn) + + // Generate an ECDSA key with an unpermitted curve + priv, err := ecdsa.GenerateKey(elliptic.P521(), rand.Reader) + if err != nil { + t.Fatalf("GenerateKey() = %v", err) + } + pubBytes, err := x509.MarshalPKIXPublicKey(&priv.PublicKey) + if err != nil { + t.Fatalf("x509.MarshalPKIXPublicKey() = %v", err) + } + hash := sha256.Sum256([]byte(emailSubject)) + proof, err := ecdsa.SignASN1(rand.Reader, priv, hash[:]) + if err != nil { + t.Fatalf("SignASN1() = %v", err) + } + pemBytes := string(cryptoutils.PEMEncode(cryptoutils.PublicKeyPEMType, pubBytes)) + + _, err = client.CreateSigningCertificate(ctx, &protobuf.CreateSigningCertificateRequest{ + Credentials: &protobuf.Credentials{ + Credentials: &protobuf.Credentials_OidcIdentityToken{ + OidcIdentityToken: tok, + }, + }, + Key: &protobuf.CreateSigningCertificateRequest_PublicKeyRequest{ + PublicKeyRequest: &protobuf.PublicKeyRequest{ + PublicKey: &protobuf.PublicKey{ + Content: pemBytes, + }, + ProofOfPossession: proof, + }, + }, + }) + if err == nil || !strings.Contains(err.Error(), "Signing algorithm not permitted") { + t.Fatalf("expected signing algorithm not permitted, got %v", err) + } + if status.Code(err) != codes.InvalidArgument { + t.Fatalf("expected invalid argument, got %v", status.Code(err)) + } +} + // Tests API with an invalid CSR. func TestAPIWithInvalidCSR(t *testing.T) { emailSigner, emailIssuer := newOIDCIssuer(t)