Skip to content

Commit 3041b83

Browse files
authored
fix: error explicitly when file signer flags are set without a key path (#806)
1 parent 69402a9 commit 3041b83

2 files changed

Lines changed: 36 additions & 0 deletions

File tree

cmd/keyloader.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/in-toto/go-witness/cryptoutil"
2323
"github.com/in-toto/go-witness/log"
2424
"github.com/in-toto/go-witness/signer"
25+
"github.com/in-toto/go-witness/signer/file"
2526
"github.com/in-toto/go-witness/signer/kms"
2627
"github.com/in-toto/witness/options"
2728
"github.com/spf13/pflag"
@@ -69,6 +70,18 @@ func loadSigners(ctx context.Context, so options.SignerOptions, ko options.KMSSi
6970
}
7071
}
7172

73+
// The file signer provider is activated as soon as any --signer-file-*
74+
// flag is set, but a certificate, intermediate, or passphrase is useless
75+
// without a key to pair it with. Left unchecked, this provider silently
76+
// fails to build below while any other configured signer (e.g. KMS)
77+
// still succeeds, so the command exits 0 with the certificate quietly
78+
// dropped instead of surfacing a clear, actionable error.
79+
if fsp, ok := sp.(file.FileSignerProvider); ok && fsp.KeyPath == "" {
80+
if fsp.CertPath != "" || len(fsp.IntermediatePaths) > 0 || fsp.PassphrasePath != "" || fsp.Passphrase != nil {
81+
return nil, fmt.Errorf("--signer-file-key-path is required when other --signer-file-* flags (e.g. --signer-file-cert-path) are set; attaching a file-based certificate to a non-file signer such as KMS is not currently supported")
82+
}
83+
}
84+
7285
s, err := sp.Signer(ctx)
7386
if err != nil {
7487
log.Errorf("failed to create %v signer: %w", signerProvider, err)

cmd/root_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,29 @@ func Test_loadSignersCertificate(t *testing.T) {
105105
require.IsType(t, &cryptoutil.X509Signer{}, signers[0])
106106
}
107107

108+
// Test_loadSignersCertificateWithoutKey reproduces
109+
// https://github.com/in-toto/witness/issues/573: passing --signer-file-cert-path
110+
// without --signer-file-key-path (e.g. because the certificate was meant for a
111+
// KMS-based signer) must fail loudly with an actionable error instead of being
112+
// silently swallowed when another signer provider succeeds.
113+
func Test_loadSignersCertificateWithoutKey(t *testing.T) {
114+
_, _, leafcert, _ := fullChain(t)
115+
116+
signerOptions := options.SignerOptions{}
117+
signerOptions["file"] = []func(signer.SignerProvider) (signer.SignerProvider, error){
118+
func(sp signer.SignerProvider) (signer.SignerProvider, error) {
119+
fsp := sp.(file.FileSignerProvider)
120+
fsp.CertPath = leafcert.Name()
121+
return fsp, nil
122+
},
123+
}
124+
125+
signers, err := loadSigners(context.Background(), signerOptions, options.KMSSignerProviderOptions{}, map[string]struct{}{"file": {}})
126+
require.Error(t, err)
127+
require.Contains(t, err.Error(), "signer-file-key-path")
128+
require.Len(t, signers, 0)
129+
}
130+
108131
func rsakeypair(t *testing.T) (privatePem *os.File, publicPem *os.File) {
109132
privatekey, err := rsa.GenerateKey(rand.Reader, keybits)
110133
if err != nil {

0 commit comments

Comments
 (0)