Skip to content

Commit 455d37b

Browse files
bobcallawaybobcallaway
authored
enable retries and timeouts on GCP KMS calls (#2548)
* enable retries and timeouts on GCP KMS calls Signed-off-by: Bob Callaway <[email protected]> * fix lint Signed-off-by: Bob Callaway <[email protected]> * fix more lint issues in e2e tests Signed-off-by: Bob Callaway <[email protected]> --------- Signed-off-by: Bob Callaway <[email protected]>
1 parent bfc05e0 commit 455d37b

File tree

12 files changed

+103
-35
lines changed

12 files changed

+103
-35
lines changed

‎cmd/rekor-server/app/root.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,8 @@ func init() {
104104
rootCmd.PersistentFlags().String("rekor_server.signer", "memory",
105105
`Rekor signer to use. Valid options are: [awskms://keyname, azurekms://keyname, gcpkms://keyname, hashivault://keyname, memory, tink, <filename containing PEM-encoded private key>].
106106
Memory and file-based signers should only be used for testing.`)
107+
rootCmd.PersistentFlags().Uint("rekor_server.signer.gcpkms.retries", 0, "Number of retries for GCP KMS requests")
108+
rootCmd.PersistentFlags().Uint("rekor_server.signer.gcpkms.timeout", 0, "sets the RPC timeout per call for GCP KMS requests in seconds, defaults to 0 (no timeout)")
107109
rootCmd.PersistentFlags().String("rekor_server.signer-passwd", "", "Password to decrypt signer private key")
108110
rootCmd.PersistentFlags().String("rekor_server.tink_kek_uri", "", "Key encryption key for decrypting Tink keyset. Valid options are [aws-kms://keyname, gcp-kms://keyname]")
109111
rootCmd.PersistentFlags().String("rekor_server.tink_keyset_path", "", "Path to encrypted Tink keyset, containing private key to sign log checkpoints")

‎cmd/rekor-server/e2e_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ func getTreeID(t *testing.T) int64 {
218218
tidStr := strings.TrimSpace(strings.Split(out, "TreeID: ")[1])
219219
tid, err := strconv.ParseInt(tidStr, 10, 64)
220220
if err != nil {
221-
t.Errorf(err.Error())
221+
t.Error(err)
222222
}
223223
t.Log("Tree ID:", tid)
224224
return tid

‎go.mod

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
module github.com/sigstore/rekor
22

3-
go 1.23.0
3+
go 1.24
44

5-
toolchain go1.24.1
5+
toolchain go1.24.5
66

77
require (
88
github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2
@@ -60,6 +60,7 @@ require (
6060
github.com/go-redis/redismock/v9 v9.2.0
6161
github.com/go-sql-driver/mysql v1.9.3
6262
github.com/golang/mock v1.7.0-rc.1
63+
github.com/grpc-ecosystem/go-grpc-middleware v1.4.0
6364
github.com/hashicorp/go-cleanhttp v0.5.2
6465
github.com/hashicorp/go-retryablehttp v0.7.8
6566
github.com/jmoiron/sqlx v1.4.0
@@ -68,7 +69,7 @@ require (
6869
github.com/sigstore/protobuf-specs v0.5.0
6970
github.com/sigstore/sigstore/pkg/signature/kms/aws v1.9.5
7071
github.com/sigstore/sigstore/pkg/signature/kms/azure v1.9.5
71-
github.com/sigstore/sigstore/pkg/signature/kms/gcp v1.9.5
72+
github.com/sigstore/sigstore/pkg/signature/kms/gcp v1.9.6-0.20250729224751-181c5d3339b3
7273
github.com/sigstore/sigstore/pkg/signature/kms/hashivault v1.9.5
7374
github.com/stretchr/testify v1.10.0
7475
github.com/tink-crypto/tink-go-awskms/v2 v2.1.0
@@ -126,7 +127,7 @@ require (
126127
github.com/envoyproxy/protoc-gen-validate v1.2.1 // indirect
127128
github.com/felixge/httpsnoop v1.0.4 // indirect
128129
github.com/go-jose/go-jose/v4 v4.0.5 // indirect
129-
github.com/go-logr/logr v1.4.2 // indirect
130+
github.com/go-logr/logr v1.4.3 // indirect
130131
github.com/go-logr/stdr v1.2.2 // indirect
131132
github.com/go-viper/mapstructure/v2 v2.3.0 // indirect
132133
github.com/golang-jwt/jwt/v5 v5.2.2 // indirect
@@ -139,7 +140,7 @@ require (
139140
github.com/hashicorp/go-secure-stdlib/strutil v0.1.2 // indirect
140141
github.com/hashicorp/go-sockaddr v1.0.2 // indirect
141142
github.com/hashicorp/vault/api v1.16.0 // indirect
142-
github.com/jellydator/ttlcache/v3 v3.3.0 // indirect
143+
github.com/jellydator/ttlcache/v3 v3.4.0 // indirect
143144
github.com/jmespath/go-jmespath v0.4.1-0.20220621161143-b0104c826a24 // indirect
144145
github.com/klauspost/compress v1.18.0 // indirect
145146
github.com/klauspost/pgzip v1.2.6 // indirect
@@ -188,7 +189,7 @@ require (
188189
github.com/go-openapi/jsonreference v0.21.0 // indirect
189190
github.com/godbus/dbus/v5 v5.1.0 // indirect
190191
github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 // indirect
191-
github.com/google/go-containerregistry v0.20.3 // indirect
192+
github.com/google/go-containerregistry v0.20.6 // indirect
192193
github.com/google/uuid v1.6.0 // indirect
193194
github.com/google/wire v0.6.0 // indirect
194195
github.com/googleapis/enterprise-certificate-proxy v0.3.6 // indirect

‎go.sum

Lines changed: 50 additions & 8 deletions
Large diffs are not rendered by default.

‎pkg/api/api.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,8 @@ func NewAPI(treeID uint) (*API, error) {
178178
FileSignerPassword: viper.GetString("rekor_server.signer-passwd"),
179179
TinkKEKURI: viper.GetString("rekor_server.tink_kek_uri"),
180180
TinkKeysetPath: viper.GetString("rekor_server.tink_keyset_path"),
181+
GCPKMSRetries: viper.GetUint("rekor_server.signer.gcpkms.retries"),
182+
GCPKMSTimeout: viper.GetUint("rekor_server.signer.gcpkms.timeout"),
181183
}
182184
ranges, err := sharding.NewLogRanges(ctx, logClient, shardingConfig, tid, signingConfig)
183185
if err != nil {

‎pkg/generated/restapi/configure_rekor_server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ func logAndServeError(w http.ResponseWriter, r *http.Request, err error) {
351351
for _, embeddedErr := range compErr.Errors {
352352
var maxBytesError *http.MaxBytesError
353353
if parseErr, ok := embeddedErr.(*errors.ParseError); ok && go_errors.As(parseErr.Reason, &maxBytesError) {
354-
err = errors.New(http.StatusRequestEntityTooLarge, http.StatusText(http.StatusRequestEntityTooLarge)) //nolint:govet
354+
err = errors.New(http.StatusRequestEntityTooLarge, "Request Entity Too Large")
355355
break
356356
}
357357
}

‎pkg/pki/x509/x509_test.go

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -40,22 +40,30 @@ import (
4040
// openssl genrsa -out myprivate.pem 512
4141
// openssl pkcs8 -topk8 -in myprivate.pem -nocrypt'
4242
const pkcs1v15Priv = `-----BEGIN PRIVATE KEY-----
43-
MIIBVQIBADANBgkqhkiG9w0BAQEFAASCAT8wggE7AgEAAkEAoLEL57Kd5w8b5LCl
44-
SM+5mJbVYj4GoFXP/Gynfk6mDj7aANYWAkU74xkjz0BX2Nq0IT9DyxWI8aXZ8B6R
45-
YtbsPwIDAQABAkA2WgwTz5eXKsYdgR421YQKN6JvO1mUa9IQqFOy5jlGgbR+W5HG
46-
JfQVJKhCGMYYmByHgR0QDk/6gvJjhuszTHuJAiEA0siY/vE20zC1UHpPgDXXVSNN
47-
dKtM6YKBKSo47oTKQHsCIQDDKZgal50Cd3W+lOWpNO23QGZgBhJrJ70TpcPWGEsS
48-
DQIhAIDIMLnq1G1Z4B2IbRRPUP3icMtscbRlmNZ2xovsM8oLAiBluZh+w+gjEQFe
49-
hV3wBJajnf2+r2uKTvxO8WhSf/chQQIhAKzYjX2chfvPN6hRqeGeoPpRLXS8cdxC
50-
A4hZJRvZgkO3
43+
MIICdgIBADANBgkqhkiG9w0BAQEFAASCAmAwggJcAgEAAoGBAOhrZXYKydvbMvE2
44+
9/1BLMUqLfZj0hADX1H4cynJH6+l3Ax3t8BP4S/g0ZFEH81CD83MVYj5FBfNGaj/
45+
GYm16enH/ZmdY7tI9EVVXjF7H+NK0WZp04BFJiUqra8QsMNrz1WOEekzdlWNeHpx
46+
4EPNsO61msIizaV4ly01HX58OXvHAgMBAAECgYBBL6X0VpBJDpCaIM2rBTWWUv8z
47+
JMoM3bVFW0aJiLRPYlh2UrmBwaWp9QcyFAZLXmTqVo4C7cEZ79drk6jI+/GPrEjL
48+
jlE5H1cSqnfJV3wjDFBp+tQK2H5fX+BLJL0Jf0w/zpujZ1fwyIeeUK4AiEcc3po6
49+
BOf246ENiYSk7Osx4QJBAPQTK4gg9+fRmT6LiKCqfiZVjAdYqd3QIz1JZWlJQfI/
50+
HSNGeVeYaX7bElJsLGd24WqFOVs4KJOUqFdJ3rRbnhECQQDzxnGa6VQ+om2sPnJS
51+
3lEfXbIzYIFa9yLAM65D4+4apsgGW5l8+XuSmXepZnQYq30e3Pr0bjOShLz1Tkbc
52+
NoRXAkBEOleg5hZmpyC/ayH2R7Kb5K4QH6jcaKJxL2M521Cj9yCeC8U/x0s2OucU
53+
Q0jmY0UAEd3GshwlpRipzeyDXlkBAkEA622id+aR+u+plai1hny4wd8eY+n246A7
54+
yn3e9igh41Fhamp6gKz8/+cBHvQYeV7dNrpaD0iCvCU/zHUtkC2CfwJAbwgIMm4B
55+
MUSwy+0Fzm+LHmdl+MbXWLNebW/DBRE+ZgHEnl2+4Mg8CEh+fl1fbJrqWtMxz89h
56+
w80MgPfqGZbHFw==
5157
-----END PRIVATE KEY-----
5258
`
5359

5460
// Extracted from above with:
5561
// openssl rsa -in myprivate.pem -pubout
5662
const pkcs1v15Pub = `-----BEGIN PUBLIC KEY-----
57-
MFwwDQYJKoZIhvcNAQEBBQADSwAwSAJBAKCxC+eynecPG+SwpUjPuZiW1WI+BqBV
58-
z/xsp35Opg4+2gDWFgJFO+MZI89AV9jatCE/Q8sViPGl2fAekWLW7D8CAwEAAQ==
63+
MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDoa2V2Csnb2zLxNvf9QSzFKi32
64+
Y9IQA19R+HMpyR+vpdwMd7fAT+Ev4NGRRB/NQg/NzFWI+RQXzRmo/xmJtenpx/2Z
65+
nWO7SPRFVV4xex/jStFmadOARSYlKq2vELDDa89VjhHpM3ZVjXh6ceBDzbDutZrC
66+
Is2leJctNR1+fDl7xwIDAQAB
5967
-----END PUBLIC KEY-----
6068
`
6169

‎pkg/sharding/ranges.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ func updateRange(ctx context.Context, logClient trillian.TrillianLogClient, r Lo
147147

148148
// Initialize shard signer
149149
s, err := signer.New(ctx, r.SigningConfig.SigningSchemeOrKeyPath, r.SigningConfig.FileSignerPassword,
150-
r.SigningConfig.TinkKEKURI, r.SigningConfig.TinkKeysetPath)
150+
r.SigningConfig.TinkKEKURI, r.SigningConfig.TinkKeysetPath, r.SigningConfig.GCPKMSRetries, r.SigningConfig.GCPKMSTimeout)
151151
if err != nil {
152152
return LogRange{}, err
153153
}

‎pkg/signer/memory_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424
func TestMemory(t *testing.T) {
2525
ctx := context.Background()
2626

27-
m, err := New(ctx, "memory", "", "", "")
27+
m, err := New(ctx, "memory", "", "", "", 0, 0)
2828
if err != nil {
2929
t.Fatalf("new memory: %v", err)
3030
}

‎pkg/signer/signer.go

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,21 @@ import (
2020
"context"
2121
"crypto"
2222
"strings"
23+
"time"
2324

25+
grpc_retry "github.com/grpc-ecosystem/go-grpc-middleware/retry"
2426
"github.com/sigstore/sigstore/pkg/signature"
2527
"github.com/sigstore/sigstore/pkg/signature/kms"
2628
"golang.org/x/exp/slices"
2729

30+
"google.golang.org/api/option"
31+
"google.golang.org/grpc"
32+
33+
"github.com/sigstore/sigstore/pkg/signature/kms/gcp"
34+
2835
// these are imported to load the providers via init() calls
2936
_ "github.com/sigstore/sigstore/pkg/signature/kms/aws"
3037
_ "github.com/sigstore/sigstore/pkg/signature/kms/azure"
31-
_ "github.com/sigstore/sigstore/pkg/signature/kms/gcp"
3238
_ "github.com/sigstore/sigstore/pkg/signature/kms/hashivault"
3339
)
3440

@@ -38,20 +44,27 @@ type SigningConfig struct {
3844
FileSignerPassword string `json:"fileSignerPassword" yaml:"fileSignerPassword"`
3945
TinkKEKURI string `json:"tinkKEKURI" yaml:"tinkKEKURI"`
4046
TinkKeysetPath string `json:"tinkKeysetPath" yaml:"tinkKeysetPath"`
47+
GCPKMSRetries uint `json:"gcpkmsRetries" yaml:"gcpkmsRetries"`
48+
GCPKMSTimeout uint `json:"gcpkmsTimeout" yaml:"gcpkmsTimeout"`
4149
}
4250

4351
func (sc SigningConfig) IsUnset() bool {
4452
return sc.SigningSchemeOrKeyPath == "" && sc.FileSignerPassword == "" &&
4553
sc.TinkKEKURI == "" && sc.TinkKeysetPath == ""
4654
}
4755

48-
func New(ctx context.Context, signer, pass, tinkKEKURI, tinkKeysetPath string) (signature.Signer, error) {
56+
func New(ctx context.Context, signer, pass, tinkKEKURI, tinkKeysetPath string, gcpkmsretries, gcpkmstimeout uint) (signature.Signer, error) {
4957
switch {
5058
case slices.ContainsFunc(kms.SupportedProviders(),
5159
func(s string) bool {
5260
return strings.HasPrefix(signer, s)
5361
}):
54-
return kms.Get(ctx, signer, crypto.SHA256)
62+
opts := make([]signature.RPCOption, 0)
63+
if strings.HasPrefix(signer, gcp.ReferenceScheme) {
64+
callOpts := []grpc_retry.CallOption{grpc_retry.WithMax(gcpkmsretries), grpc_retry.WithPerRetryTimeout(time.Duration(gcpkmstimeout) * time.Second)}
65+
opts = append(opts, gcp.WithGoogleAPIClientOption(option.WithGRPCDialOption(grpc.WithUnaryInterceptor(grpc_retry.UnaryClientInterceptor(callOpts...)))))
66+
}
67+
return kms.Get(ctx, signer, crypto.SHA256, opts...)
5568
case signer == MemoryScheme:
5669
return NewMemory()
5770
case signer == TinkScheme:

0 commit comments

Comments
 (0)