Skip to content

Commit 92c6fe6

Browse files
yaronfyaronf
committed
Disallow nil keys
1 parent 30f1bf2 commit 92c6fe6

File tree

3 files changed

+65
-81
lines changed

3 files changed

+65
-81
lines changed

‎crypto.go‎

Lines changed: 34 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,7 @@ func NewHMACSHA256Signer(keyID string, key []byte, config *SignConfig, fields Fi
4848

4949
// NewRSASigner returns a new Signer structure. Key is an RSA private key.
5050
// Config may be nil for a default configuration.
51-
func NewRSASigner(keyID string, key *rsa.PrivateKey, config *SignConfig, fields Fields) (*Signer, error) {
52-
if key == nil {
53-
return nil, fmt.Errorf("key must not be nil")
54-
}
51+
func NewRSASigner(keyID string, key rsa.PrivateKey, config *SignConfig, fields Fields) (*Signer, error) {
5552
if keyID == "" {
5653
return nil, fmt.Errorf("keyID must not be empty")
5754
}
@@ -69,10 +66,7 @@ func NewRSASigner(keyID string, key *rsa.PrivateKey, config *SignConfig, fields
6966

7067
// NewRSAPSSSigner returns a new Signer structure. Key is an RSA private key.
7168
// Config may be nil for a default configuration.
72-
func NewRSAPSSSigner(keyID string, key *rsa.PrivateKey, config *SignConfig, fields Fields) (*Signer, error) {
73-
if key == nil {
74-
return nil, fmt.Errorf("key must not be nil")
75-
}
69+
func NewRSAPSSSigner(keyID string, key rsa.PrivateKey, config *SignConfig, fields Fields) (*Signer, error) {
7670
if keyID == "" {
7771
return nil, fmt.Errorf("keyID must not be empty")
7872
}
@@ -90,10 +84,7 @@ func NewRSAPSSSigner(keyID string, key *rsa.PrivateKey, config *SignConfig, fiel
9084

9185
// NewP256Signer returns a new Signer structure. Key is an elliptic curve P-256 private key.
9286
// Config may be nil for a default configuration.
93-
func NewP256Signer(keyID string, key *ecdsa.PrivateKey, config *SignConfig, fields Fields) (*Signer, error) {
94-
if key == nil {
95-
return nil, fmt.Errorf("key must not be nil")
96-
}
87+
func NewP256Signer(keyID string, key ecdsa.PrivateKey, config *SignConfig, fields Fields) (*Signer, error) {
9788
if keyID == "" {
9889
return nil, fmt.Errorf("keyID must not be empty")
9990
}
@@ -111,7 +102,7 @@ func NewP256Signer(keyID string, key *ecdsa.PrivateKey, config *SignConfig, fiel
111102

112103
// NewEd25519Signer returns a new Signer structure. Key is an EdDSA Curve 25519 private key.
113104
// Config may be nil for a default configuration.
114-
func NewEd25519Signer(keyID string, key *ed25519.PrivateKey, config *SignConfig, fields Fields) (*Signer, error) {
105+
func NewEd25519Signer(keyID string, key ed25519.PrivateKey, config *SignConfig, fields Fields) (*Signer, error) {
115106
if key == nil {
116107
return nil, fmt.Errorf("key must not be nil")
117108
}
@@ -133,18 +124,21 @@ func NewEd25519Signer(keyID string, key *ed25519.PrivateKey, config *SignConfig,
133124
// NewEd25519SignerFromSeed returns a new Signer structure. Key is an EdDSA Curve 25519 private key,
134125
// a 32 byte buffer according to RFC 8032.
135126
// Config may be nil for a default configuration.
136-
func NewEd25519SignerFromSeed(keyID string, seed *[]byte, config *SignConfig, fields Fields) (*Signer, error) {
137-
if seed == nil || len(*seed) != ed25519.SeedSize {
127+
func NewEd25519SignerFromSeed(keyID string, seed []byte, config *SignConfig, fields Fields) (*Signer, error) {
128+
if seed == nil || len(seed) != ed25519.SeedSize {
138129
return nil, fmt.Errorf("seed must not be nil, and must have length %d", ed25519.SeedSize)
139130
}
140-
key := ed25519.NewKeyFromSeed(*seed)
141-
return NewEd25519Signer(keyID, &key, config, fields)
131+
key := ed25519.NewKeyFromSeed(seed)
132+
return NewEd25519Signer(keyID, key, config, fields)
142133
}
143134

144135
// NewJWSSigner creates a generic signer for JWS algorithms, using the go-jwx package. The particular key type for each algorithm
145136
// is documented in that package.
146137
// Config may be nil for a default configuration.
147138
func NewJWSSigner(alg jwa.SignatureAlgorithm, keyID string, key interface{}, config *SignConfig, fields Fields) (*Signer, error) {
139+
if key == nil {
140+
return nil, fmt.Errorf("key must not be nil")
141+
}
148142
if alg == jwa.NoSignature {
149143
return nil, fmt.Errorf("the NONE signing algorithm is expressly disallowed")
150144
}
@@ -180,23 +174,27 @@ func (s Signer) sign(buff []byte) ([]byte, error) {
180174
return mac.Sum(nil), nil
181175
case "rsa-v1_5-sha256":
182176
hashed := sha256.Sum256(buff)
183-
sig, err := rsa.SignPKCS1v15(nil, s.key.(*rsa.PrivateKey), crypto.SHA256, hashed[:])
177+
key := s.key.(rsa.PrivateKey)
178+
sig, err := rsa.SignPKCS1v15(nil, &key, crypto.SHA256, hashed[:])
184179
if err != nil {
185180
return nil, fmt.Errorf("RSA signature failed")
186181
}
187182
return sig, nil
188183
case "rsa-pss-sha512":
189184
hashed := sha512.Sum512(buff)
190-
sig, err := rsa.SignPSS(rand.Reader, s.key.(*rsa.PrivateKey), crypto.SHA512, hashed[:], nil)
185+
key := s.key.(rsa.PrivateKey)
186+
sig, err := rsa.SignPSS(rand.Reader, &key, crypto.SHA512, hashed[:], nil)
191187
if err != nil {
192188
return nil, fmt.Errorf("RSA-PSS signature failed")
193189
}
194190
return sig, nil
195191
case "ecdsa-p256-sha256":
196192
hashed := sha256.Sum256(buff)
197-
return ecdsaSignRaw(rand.Reader, s.key.(*ecdsa.PrivateKey), hashed[:])
193+
key := s.key.(ecdsa.PrivateKey)
194+
return ecdsaSignRaw(rand.Reader, &key, hashed[:])
198195
case "ed25519":
199-
return ed25519.Sign(*s.key.(*ed25519.PrivateKey), buff), nil
196+
key := s.key.(ed25519.PrivateKey)
197+
return ed25519.Sign(key, buff), nil
200198
default:
201199
return nil, fmt.Errorf("sign: unknown algorithm \"%s\"", s.alg)
202200
}
@@ -235,10 +233,7 @@ func NewHMACSHA256Verifier(keyID string, key []byte, config *VerifyConfig, field
235233

236234
// NewRSAVerifier generates a new Verifier for RSA signatures. Set config to nil for a default configuration.
237235
// Fields is the list of required headers and fields, which may be empty (but this is typically insecure).
238-
func NewRSAVerifier(keyID string, key *rsa.PublicKey, config *VerifyConfig, fields Fields) (*Verifier, error) {
239-
if key == nil {
240-
return nil, fmt.Errorf("key must not be nil")
241-
}
236+
func NewRSAVerifier(keyID string, key rsa.PublicKey, config *VerifyConfig, fields Fields) (*Verifier, error) {
242237
if config == nil {
243238
config = NewVerifyConfig()
244239
}
@@ -253,10 +248,7 @@ func NewRSAVerifier(keyID string, key *rsa.PublicKey, config *VerifyConfig, fiel
253248

254249
// NewRSAPSSVerifier generates a new Verifier for RSA-PSS signatures. Set config to nil for a default configuration.
255250
// Fields is the list of required headers and fields, which may be empty (but this is typically insecure).
256-
func NewRSAPSSVerifier(keyID string, key *rsa.PublicKey, config *VerifyConfig, fields Fields) (*Verifier, error) {
257-
if key == nil {
258-
return nil, fmt.Errorf("key must not be nil")
259-
}
251+
func NewRSAPSSVerifier(keyID string, key rsa.PublicKey, config *VerifyConfig, fields Fields) (*Verifier, error) {
260252
if config == nil {
261253
config = NewVerifyConfig()
262254
}
@@ -271,10 +263,7 @@ func NewRSAPSSVerifier(keyID string, key *rsa.PublicKey, config *VerifyConfig, f
271263

272264
// NewP256Verifier generates a new Verifier for ECDSA (P-256) signatures. Set config to nil for a default configuration.
273265
// Fields is the list of required headers and fields, which may be empty (but this is typically insecure).
274-
func NewP256Verifier(keyID string, key *ecdsa.PublicKey, config *VerifyConfig, fields Fields) (*Verifier, error) {
275-
if key == nil {
276-
return nil, fmt.Errorf("key must not be nil")
277-
}
266+
func NewP256Verifier(keyID string, key ecdsa.PublicKey, config *VerifyConfig, fields Fields) (*Verifier, error) {
278267
if config == nil {
279268
config = NewVerifyConfig()
280269
}
@@ -289,7 +278,7 @@ func NewP256Verifier(keyID string, key *ecdsa.PublicKey, config *VerifyConfig, f
289278

290279
// NewEd25519Verifier generates a new Verifier for EdDSA Curve 25519 signatures. Set config to nil for a default configuration.
291280
// Fields is the list of required headers and fields, which may be empty (but this is typically insecure).
292-
func NewEd25519Verifier(keyID string, key *ed25519.PublicKey, config *VerifyConfig, fields Fields) (*Verifier, error) {
281+
func NewEd25519Verifier(keyID string, key ed25519.PublicKey, config *VerifyConfig, fields Fields) (*Verifier, error) {
293282
if key == nil {
294283
return nil, fmt.Errorf("key must not be nil")
295284
}
@@ -309,6 +298,9 @@ func NewEd25519Verifier(keyID string, key *ed25519.PublicKey, config *VerifyConf
309298
// is documented in that package. Set config to nil for a default configuration.
310299
// Fields is the list of required headers and fields, which may be empty (but this is typically insecure).
311300
func NewJWSVerifier(alg jwa.SignatureAlgorithm, key interface{}, keyID string, config *VerifyConfig, fields Fields) (*Verifier, error) {
301+
if key == nil {
302+
return nil, fmt.Errorf("key must not be nil")
303+
}
312304
if alg == jwa.NoSignature {
313305
return nil, fmt.Errorf("the NONE signing algorithm is expressly disallowed")
314306
}
@@ -347,23 +339,27 @@ func (v Verifier) verify(buff []byte, sig []byte) (bool, error) {
347339
return bytes.Equal(mac.Sum(nil), sig), nil
348340
case "rsa-v1_5-sha256":
349341
hashed := sha256.Sum256(buff)
350-
err := rsa.VerifyPKCS1v15(v.key.(*rsa.PublicKey), crypto.SHA256, hashed[:], sig)
342+
key := v.key.(rsa.PublicKey)
343+
err := rsa.VerifyPKCS1v15(&key, crypto.SHA256, hashed[:], sig)
351344
if err != nil {
352345
return false, fmt.Errorf("RSA verification failed: %w", err)
353346
}
354347
return true, nil
355348
case "rsa-pss-sha512":
356349
hashed := sha512.Sum512(buff)
357-
err := rsa.VerifyPSS(v.key.(*rsa.PublicKey), crypto.SHA512, hashed[:], sig, nil)
350+
key := v.key.(rsa.PublicKey)
351+
err := rsa.VerifyPSS(&key, crypto.SHA512, hashed[:], sig, nil)
358352
if err != nil {
359353
return false, fmt.Errorf("RSA-PSS verification failed: %w", err)
360354
}
361355
return true, nil
362356
case "ecdsa-p256-sha256":
363357
hashed := sha256.Sum256(buff)
364-
return ecdsaVerifyRaw(v.key.(*ecdsa.PublicKey), hashed[:], sig)
358+
key := v.key.(ecdsa.PublicKey)
359+
return ecdsaVerifyRaw(&key, hashed[:], sig)
365360
case "ed25519":
366-
verified := ed25519.Verify(*v.key.(*ed25519.PublicKey), buff, sig)
361+
key := v.key.(ed25519.PublicKey)
362+
verified := ed25519.Verify(key, buff, sig)
367363
if !verified {
368364
return false, fmt.Errorf("failed Ed25519 verification")
369365
}

‎crypto_test.go‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ func TestNewRSASigner(t *testing.T) {
135135

136136
type args struct {
137137
keyID string
138-
key *rsa.PrivateKey
138+
key rsa.PrivateKey
139139
config *SignConfig
140140
fields Fields
141141
}
@@ -149,24 +149,13 @@ func TestNewRSASigner(t *testing.T) {
149149
name: "empty key ID",
150150
args: args{
151151
keyID: "",
152-
key: privateKey,
152+
key: *privateKey,
153153
config: nil,
154154
fields: *NewFields(),
155155
},
156156
want: nil,
157157
wantErr: true,
158158
},
159-
{
160-
name: "nil key",
161-
args: args{
162-
keyID: "kk",
163-
key: nil,
164-
config: NewSignConfig(),
165-
fields: *NewFields(),
166-
},
167-
want: nil,
168-
wantErr: true,
169-
},
170159
}
171160
for _, tt := range tests {
172161
t.Run(tt.name, func(t *testing.T) {
@@ -240,7 +229,7 @@ func TestNewRSASigner1(t *testing.T) {
240229
},
241230
want: &Signer{
242231
keyID: "key100",
243-
key: key,
232+
key: *key,
244233
alg: "rsa-v1_5-sha256",
245234
config: NewSignConfig(),
246235
fields: Fields{},
@@ -251,7 +240,7 @@ func TestNewRSASigner1(t *testing.T) {
251240
}
252241
for _, tt := range tests {
253242
t.Run(tt.name, func(t *testing.T) {
254-
got, err := NewRSASigner(tt.args.keyID, tt.args.key, tt.args.config, tt.args.fields)
243+
got, err := NewRSASigner(tt.args.keyID, *tt.args.key, tt.args.config, tt.args.fields)
255244
if (err != nil) != tt.wantErr {
256245
t.Errorf("NewRSASigner() error = %v, wantErr %v", err, tt.wantErr)
257246
return

0 commit comments

Comments
 (0)