Skip to content

Commit 91c6ee6

Browse files
marainomaraino
committed
Return apiv1.ErrNotFound on softkms
This commit makes sure to return an apiv1.NotFoundError on sofkms.
1 parent 847a848 commit 91c6ee6

5 files changed

Lines changed: 132 additions & 94 deletions

File tree

‎kms/platform/kms_softkms.go‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@ import (
44
"bytes"
55
"context"
66
"encoding/pem"
7+
"errors"
78
"fmt"
89
"net/url"
910
"os"
11+
"syscall"
1012

1113
"go.step.sm/crypto/kms/apiv1"
1214
"go.step.sm/crypto/kms/softkms"
@@ -54,7 +56,11 @@ func (k *softKMS) DeleteKey(req *apiv1.DeleteKeyRequest) error {
5456
return fmt.Errorf("deleteKeyRequest 'name' cannot be empty")
5557
}
5658

57-
return os.Remove(req.Name)
59+
if err := os.Remove(req.Name); err != nil {
60+
return toKMSError(err, "key not found")
61+
}
62+
63+
return nil
5864
}
5965

6066
func (k *softKMS) StoreCertificate(req *apiv1.StoreCertificateRequest) error {
@@ -97,7 +103,11 @@ func (k *softKMS) DeleteCertificate(req *apiv1.DeleteCertificateRequest) error {
97103
return fmt.Errorf("deleteCertificateRequest 'name' cannot be empty")
98104
}
99105

100-
return os.Remove(req.Name)
106+
if err := os.Remove(req.Name); err != nil {
107+
return toKMSError(err, "certificate not found")
108+
}
109+
110+
return nil
101111
}
102112

103113
func transformToSoftKMS(rawuri string) (string, error) {
@@ -131,3 +141,14 @@ func transformFromSoftKMS(path string) (string, error) {
131141
}
132142
return uri.New(Scheme, uv).String(), nil
133143
}
144+
145+
func toKMSError(err error, message string) error {
146+
switch {
147+
case errors.Is(err, os.ErrNotExist), errors.Is(err, syscall.ENOENT):
148+
return apiv1.NotFoundError{
149+
Message: message,
150+
}
151+
default:
152+
return err
153+
}
154+
}

‎kms/platform/kms_test.go‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,13 @@ func mustEqualPrivateKey(t *testing.T, x, y crypto.PrivateKey) {
175175
require.True(t, xx.Equal(y))
176176
}
177177

178+
func assertNotFoundError(t assert.TestingT, err error, msgAndArgs ...any) bool {
179+
if h, ok := t.(interface{ Helper() }); ok {
180+
h.Helper()
181+
}
182+
return assert.ErrorIs(t, err, apiv1.NotFoundError{}, msgAndArgs...)
183+
}
184+
178185
type createOptions struct {
179186
name string
180187
noCleanup bool
@@ -686,7 +693,7 @@ func TestKMS_CreateSigner(t *testing.T) {
686693
}, assert.NoError},
687694
{"fail missing", softKMS, args{&apiv1.CreateSignerRequest{
688695
SigningKey: "kms:name=" + url.QueryEscape(filepath.Join(dir, "missing.key")),
689-
}}, assertNil, assert.Error},
696+
}}, assertNil, assertNotFoundError},
690697
{"fail parseURI", softKMS, args{&apiv1.CreateSignerRequest{
691698
SigningKey: privateKeyPath,
692699
}}, assertNil, assert.Error},
@@ -749,7 +756,7 @@ func TestKMS_DeleteKey(t *testing.T) {
749756
}}, assert.Error},
750757
{"fail platform missing", platformKMS, args{&apiv1.DeleteKeyRequest{
751758
Name: platformMissingName,
752-
}}, assert.Error},
759+
}}, assertNotFoundError},
753760

754761
// SoftKMS
755762
{"ok softKMS", softKMS, args{&apiv1.DeleteKeyRequest{
@@ -760,7 +767,7 @@ func TestKMS_DeleteKey(t *testing.T) {
760767
}},
761768
{"fail missing", softKMS, args{&apiv1.DeleteKeyRequest{
762769
Name: "kms:name=" + url.QueryEscape(filepath.Join(dir, "missing.key")),
763-
}}, assert.Error},
770+
}}, assertNotFoundError},
764771
{"fail parseURI", softKMS, args{&apiv1.DeleteKeyRequest{
765772
Name: keyPath2,
766773
}}, func(tt assert.TestingT, err error, i ...interface{}) bool {
@@ -815,7 +822,7 @@ func TestKMS_LoadCertificate(t *testing.T) {
815822
}}, platformChain[0], assert.NoError},
816823
{"fail platform missing", platformKMS, args{&apiv1.LoadCertificateRequest{
817824
Name: platformMissingName,
818-
}}, nil, assert.Error},
825+
}}, nil, assertNotFoundError},
819826

820827
// SoftKMS
821828
{"ok softKMS", softKMS, args{&apiv1.LoadCertificateRequest{
@@ -826,7 +833,7 @@ func TestKMS_LoadCertificate(t *testing.T) {
826833
}}, chain[0], assert.NoError},
827834
{"fail missing", softKMS, args{&apiv1.LoadCertificateRequest{
828835
Name: "kms:name=" + filepath.Join(dir, "missing.crt"),
829-
}}, nil, assert.Error},
836+
}}, nil, assertNotFoundError},
830837
{"fail parseURI", softKMS, args{&apiv1.LoadCertificateRequest{
831838
Name: "foo:name=" + certPath,
832839
}}, nil, assert.Error},
@@ -973,7 +980,7 @@ func TestKMS_LoadCertificateChain(t *testing.T) {
973980
}}, nil, assert.Error},
974981
{"fail missing", softKMS, args{&apiv1.LoadCertificateChainRequest{
975982
Name: "kms:name=" + filepath.Join(dir, "missing.crt"),
976-
}}, nil, assert.Error},
983+
}}, nil, assertNotFoundError},
977984
{"fail parseuri", softKMS, args{&apiv1.LoadCertificateChainRequest{
978985
Name: "softkms:name=" + chainPath,
979986
}}, nil, assert.Error},
@@ -1111,7 +1118,7 @@ func TestKMS_DeleteCertificate(t *testing.T) {
11111118
}},
11121119
{"fail platform missing", platformKMS, args{&apiv1.DeleteCertificateRequest{
11131120
Name: platformMissingName,
1114-
}}, assert.Error},
1121+
}}, assertNotFoundError},
11151122

11161123
// SoftKMS
11171124
{"ok softKMS", softKMS, args{&apiv1.DeleteCertificateRequest{
@@ -1122,7 +1129,7 @@ func TestKMS_DeleteCertificate(t *testing.T) {
11221129
}},
11231130
{"fail missing", softKMS, args{&apiv1.DeleteCertificateRequest{
11241131
Name: "kms:name=" + url.QueryEscape(filepath.Join(dir, "chain.crt")),
1125-
}}, assert.Error},
1132+
}}, assertNotFoundError},
11261133
{"fail parseURI", softKMS, args{&apiv1.DeleteCertificateRequest{
11271134
Name: "foo",
11281135
}}, assert.Error},
@@ -1186,7 +1193,7 @@ func TestKMS_CreateAttestation(t *testing.T) {
11861193
{"fail missing key", softKMS, args{&apiv1.CreateAttestationRequest{
11871194
Name: "kms:" + platformMissingName,
11881195
AttestationClient: okClient,
1189-
}}, nil, assert.Error},
1196+
}}, nil, assertNotFoundError},
11901197
{"fail custom attestation", softKMS, args{&apiv1.CreateAttestationRequest{
11911198
Name: "kms:" + privateKeyPath,
11921199
AttestationClient: failClient,

‎kms/platform/kms_tpmsimulator_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -377,10 +377,10 @@ func TestKMS_DeleteKey_tpm(t *testing.T) {
377377
}},
378378
{"fail missing key", km, args{&apiv1.DeleteKeyRequest{
379379
Name: "kms:name=key-2",
380-
}}, assert.Error},
380+
}}, assertNotFoundError},
381381
{"fail missing ak", km, args{&apiv1.DeleteKeyRequest{
382382
Name: "kms:name=ak-2;ak=true",
383-
}}, assert.Error},
383+
}}, assertNotFoundError},
384384
}
385385
for _, tt := range tests {
386386
t.Run(tt.name, func(t *testing.T) {
@@ -443,7 +443,7 @@ func TestKMS_LoadCertificate_tpm(t *testing.T) {
443443
}}, nil, assert.Error},
444444
{"fail missing", km, args{&apiv1.LoadCertificateRequest{
445445
Name: "kms:name=missing-key",
446-
}}, nil, assert.Error},
446+
}}, nil, assertNotFoundError},
447447
}
448448
for _, tt := range tests {
449449
t.Run(tt.name, func(t *testing.T) {
@@ -589,7 +589,7 @@ func TestKMS_LoadCertificateChain_tpm(t *testing.T) {
589589
}}, nil, assert.Error},
590590
{"fail missing", km, args{&apiv1.LoadCertificateChainRequest{
591591
Name: "kms:name=missing-key",
592-
}}, nil, assert.Error},
592+
}}, nil, assertNotFoundError},
593593
}
594594
for _, tt := range tests {
595595
t.Run(tt.name, func(t *testing.T) {

‎kms/softkms/softkms.go‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"crypto/rsa"
99
"crypto/x509"
1010
"fmt"
11+
"os"
12+
"syscall"
1113

1214
"github.com/pkg/errors"
1315

@@ -95,7 +97,7 @@ func (k *SoftKMS) CreateSigner(req *apiv1.CreateSignerRequest) (crypto.Signer, e
9597
case req.SigningKey != "":
9698
v, err := pemutil.Read(filename(req.SigningKey), opts...)
9799
if err != nil {
98-
return nil, err
100+
return nil, toKMSError(err, "key not found")
99101
}
100102
sig, ok := v.(crypto.Signer)
101103
if !ok {
@@ -140,7 +142,7 @@ func (k *SoftKMS) CreateKey(req *apiv1.CreateKeyRequest) (*apiv1.CreateKeyRespon
140142
func (k *SoftKMS) GetPublicKey(req *apiv1.GetPublicKeyRequest) (crypto.PublicKey, error) {
141143
v, err := pemutil.Read(filename(req.Name))
142144
if err != nil {
143-
return nil, err
145+
return nil, toKMSError(err, "key not found")
144146
}
145147

146148
switch vv := v.(type) {
@@ -180,7 +182,7 @@ func (k *SoftKMS) CreateDecrypter(req *apiv1.CreateDecrypterRequest) (crypto.Dec
180182
case req.DecryptionKey != "":
181183
v, err := pemutil.Read(filename(req.DecryptionKey), opts...)
182184
if err != nil {
183-
return nil, err
185+
return nil, toKMSError(err, "key not found")
184186
}
185187
decrypter, ok := v.(crypto.Decrypter)
186188
if !ok {
@@ -201,7 +203,7 @@ func (k *SoftKMS) LoadCertificate(req *apiv1.LoadCertificateRequest) (*x509.Cert
201203

202204
bundle, err := pemutil.ReadCertificateBundle(filename(req.Name))
203205
if err != nil {
204-
return nil, err
206+
return nil, toKMSError(err, "certificate not found")
205207
}
206208

207209
return bundle[0], nil
@@ -214,7 +216,12 @@ func (k *SoftKMS) LoadCertificateChain(req *apiv1.LoadCertificateChainRequest) (
214216
return nil, fmt.Errorf("loadCertificateChainRequest 'name' cannot be empty")
215217
}
216218

217-
return pemutil.ReadCertificateBundle(filename(req.Name))
219+
chain, err := pemutil.ReadCertificateBundle(filename(req.Name))
220+
if err != nil {
221+
return nil, toKMSError(err, "certificate not found")
222+
}
223+
224+
return chain, nil
218225
}
219226

220227
func filename(s string) string {
@@ -231,3 +238,14 @@ func filename(s string) string {
231238
}
232239
return s
233240
}
241+
242+
func toKMSError(err error, message string) error {
243+
switch {
244+
case errors.Is(err, os.ErrNotExist), errors.Is(err, syscall.ENOENT):
245+
return apiv1.NotFoundError{
246+
Message: message,
247+
}
248+
default:
249+
return err
250+
}
251+
}

0 commit comments

Comments
 (0)