Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow configurable client signing algorithms #1517

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
6 changes: 4 additions & 2 deletions cmd/app/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,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"
Expand Down Expand Up @@ -154,7 +156,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 *signature.AlgorithmRegistryConfig, ip identity.IssuerPool) (*grpcServer, error) {
logger, opts := log.SetupGRPCLogging()

serverOpts := []grpc.ServerOption{
Expand Down Expand Up @@ -182,7 +184,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.
Expand Down
14 changes: 12 additions & 2 deletions cmd/app/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
38 changes: 34 additions & 4 deletions cmd/app/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,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"
Expand Down Expand Up @@ -106,6 +109,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", 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")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we default to the 256 algos only right now? I think that's the most conservative option as it does not add new algos. Other SHAs were not working before.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering the same for Rekor as well.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I think that'd be a reasonable default.


// convert "http-host" flag to "host" and "http-port" flag to be "port"
cmd.Flags().SetNormalizeFunc(func(f *pflag.FlagSet, name string) pflag.NormalizedName {
Expand Down Expand Up @@ -204,6 +208,20 @@ func runServeCmd(cmd *cobra.Command, args []string) { //nolint: revive
// Setup the logger to dev/prod
log.ConfigureLogger(viper.GetString("log_type"))

algorithmStrings := viper.GetStringSlice("client-signing-algorithms")
var algorithmConfig []v1.KnownSignatureAlgorithm
for _, s := range algorithmStrings {
algorithmValue, err := signature.ParseSignatureAlgorithmFlag(s)
if err != nil {
log.Logger.Fatal(err)
}
algorithmConfig = append(algorithmConfig, algorithmValue)
}
algorithmRegistry, err := signature.NewAlgorithmRegistryConfig(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{})

Expand Down Expand Up @@ -284,7 +302,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
Expand All @@ -297,7 +315,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)
}
Expand Down Expand Up @@ -375,7 +393,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 *signature.AlgorithmRegistryConfig, host string, port, metricsPort int, ip identity.IssuerPool) error {
logger, opts := log.SetupGRPCLogging()

d := duplex.New(
Expand All @@ -394,7 +412,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)
Expand Down Expand Up @@ -427,3 +445,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
}
8 changes: 7 additions & 1 deletion cmd/app/serve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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)
}
}()
Expand Down
5 changes: 4 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ require (
github.com/prometheus/client_model v0.5.0
github.com/prometheus/common v0.46.0
github.com/rs/cors v1.10.1
github.com/sigstore/protobuf-specs v0.3.0-beta.2
github.com/sigstore/sigstore v1.8.1
github.com/sigstore/sigstore/pkg/signature/kms/aws v1.8.0
github.com/sigstore/sigstore/pkg/signature/kms/azure v1.8.1
Expand Down Expand Up @@ -89,7 +90,7 @@ require (
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang-jwt/jwt/v5 v5.0.0 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/google/go-containerregistry v0.17.0 // indirect
github.com/google/go-containerregistry v0.18.0 // indirect
github.com/google/gofuzz v1.2.0 // indirect
github.com/google/s2a-go v0.1.7 // indirect
github.com/google/uuid v1.5.0 // indirect
Expand Down Expand Up @@ -169,3 +170,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/sigstore => /Users/tetsuo/Code/sigstore
12 changes: 6 additions & 6 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ github.com/go-logr/logr v1.4.1 h1:pKouT5E8xu9zeFC39JXRDukb6JFQPXM5p5I91188VAQ=
github.com/go-logr/logr v1.4.1/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag=
github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE=
github.com/go-rod/rod v0.114.5 h1:1x6oqnslwFVuXJbJifgxspJUd3O4ntaGhRLHt+4Er9c=
github.com/go-rod/rod v0.114.5/go.mod h1:aiedSEFg5DwG/fnNbUOTPMTTWX3MRj6vIs/a684Mthw=
github.com/go-rod/rod v0.114.6 h1:NrutWvLGn6Vea+0ZpLSHQ2cT5UMTqk9DeO+V6xeJBxw=
github.com/go-rod/rod v0.114.6/go.mod h1:aiedSEFg5DwG/fnNbUOTPMTTWX3MRj6vIs/a684Mthw=
github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY=
github.com/go-test/deep v1.1.0 h1:WOcxcdHcvdgThNXjw0t76K42FXTU7HpNQWHpA2HHNlg=
github.com/go-test/deep v1.1.0/go.mod h1:5C2ZWiW0ErCdrYzpqxLbTX7MG14M9iiw8DgHncVwcsE=
Expand Down Expand Up @@ -165,8 +165,8 @@ github.com/google/go-cmp v0.5.3/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/go-containerregistry v0.17.0 h1:5p+zYs/R4VGHkhyvgWurWrpJ2hW4Vv9fQI+GzdcwXLk=
github.com/google/go-containerregistry v0.17.0/go.mod h1:u0qB2l7mvtWVR5kNcbFIhFY1hLbf8eeGapA+vbFDCtQ=
github.com/google/go-containerregistry v0.18.0 h1:ShE7erKNPqRh5ue6Z9DUOlk04WsnFWPO6YGr3OxnfoQ=
github.com/google/go-containerregistry v0.18.0/go.mod h1:u0qB2l7mvtWVR5kNcbFIhFY1hLbf8eeGapA+vbFDCtQ=
github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0=
github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
Expand Down Expand Up @@ -310,8 +310,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/sigstore v1.8.1 h1:mAVposMb14oplk2h/bayPmIVdzbq2IhCgy4g6R0ZSjo=
github.com/sigstore/sigstore v1.8.1/go.mod h1:02SL1158BSj15bZyOFz7m+/nJzLZfFd9A8ab3Kz7w/E=
github.com/sigstore/protobuf-specs v0.3.0-beta.2 h1:neHS0O1z7qz4q21vyXqSaKuKYxA0upzJERT88NrgYlM=
github.com/sigstore/protobuf-specs v0.3.0-beta.2/go.mod h1:ynKzXpqr3dUj2Xk9O/5ZUhjnpi0F53DNi5AdH6pS3jc=
github.com/sigstore/sigstore/pkg/signature/kms/aws v1.8.0 h1:nLaaOX85YjBKQOQHWY2UlDkbx+je8ozTEM+t1ySAb78=
github.com/sigstore/sigstore/pkg/signature/kms/aws v1.8.0/go.mod h1:fLxrKqPP9lIz/B3UBD4ZK6j6984eX2czu/0zxm99fkE=
github.com/sigstore/sigstore/pkg/signature/kms/azure v1.8.1 h1:DvRWG99QGWZC5mp42SEde2Xke/Q384Idnj2da7yB+Mk=
Expand Down
60 changes: 54 additions & 6 deletions pkg/server/grpc_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,14 @@ package server
import (
"context"
"crypto"
"crypto/ed25519"
"crypto/x509"
"encoding/json"
"errors"
"fmt"

"github.com/sigstore/sigstore/pkg/signature"

health "google.golang.org/grpc/health/grpc_health_v1"

ctclient "github.com/google/certificate-transparency-go/client"
Expand All @@ -43,11 +47,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 *signature.AlgorithmRegistryConfig, ip identity.IssuerPool) GRPCCAServer {
return &grpcaCAServer{
ct: ct,
ca: ca,
IssuerPool: ip,
ct: ct,
ca: ca,
algorithmRegistry: algorithmRegistry,
IssuerPool: ip,
}
}

Expand All @@ -57,8 +62,9 @@ const (

type grpcaCAServer struct {
fulciogrpc.UnimplementedCAServer
ct *ctclient.LogClient
ca certauth.CertificateAuthority
ct *ctclient.LogClient
ca certauth.CertificateAuthority
algorithmRegistry *signature.AlgorithmRegistryConfig
identity.IssuerPool
}

Expand Down Expand Up @@ -87,6 +93,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 {
Expand All @@ -105,6 +112,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)
tetsuo-cpp marked this conversation as resolved.
Show resolved Hide resolved
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 (
Expand Down Expand Up @@ -132,6 +144,22 @@ 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)
}

// The proof of possession signature always uses SHA-256, unless the key algorithm is ED25519
hashFunc = crypto.SHA256
tetsuo-cpp marked this conversation as resolved.
Show resolved Hide resolved
if _, ok := publicKey.(*ed25519.PublicKey); ok {
hashFunc = crypto.Hash(0)
}
}

// Check whether the public-key/hash algorithm combination is allowed
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())
}

var csc *certauth.CodeSigningCertificate
Expand Down Expand Up @@ -279,3 +307,23 @@ 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:
tetsuo-cpp marked this conversation as resolved.
Show resolved Hide resolved
return crypto.SHA256, nil
case x509.ECDSAWithSHA384:
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:
return crypto.Hash(0), nil
}
return crypto.Hash(0), fmt.Errorf("unrecognized signature algorithm: %s", signatureAlgorithm)
}
Loading