veyron2/security,veyron/security: Fix bug in signature generation.
The ECDSA signature algorithm signs only the leftmost N bits of a message
where N is the size of the underlying field of the curve. For example,
a P256 curve signs the leftmost 32 bits.
Prior to this commit, NewClearSigner.Sign would effectively only
sign the leftmost bits of the message, thereby allowing signatures
of one message >N bits to be used for distinct messages that share
the first N bits.
This commit fixes this problem by using a cryptographic hash function
on the message (if necessary) to "reduce" the size of the data
being signed to N bits.
The test added to signature_test.go is more comprehensive than
the tests it is replacing in this commit and would fail without
the changes to apply the hash function.
This commit also moves NewClearSigner from the veyron/security/signing
package to veyron2/security.
Change-Id: I4b9adcd467165e3eddb18a116fe0165377e7b198
diff --git a/runtimes/google/security/identity_chain.go b/runtimes/google/security/identity_chain.go
index 0987af3..abeb085 100644
--- a/runtimes/google/security/identity_chain.go
+++ b/runtimes/google/security/identity_chain.go
@@ -14,7 +14,6 @@
"veyron/runtimes/google/security/keys"
vsecurity "veyron/security"
"veyron/security/caveat"
- "veyron/security/signing"
"veyron2/security"
"veyron2/security/wire"
@@ -158,7 +157,7 @@
PublicKey: *id.publicID.publicKey.DO_NOT_USE(),
D: new(big.Int).SetBytes(w.Secret),
}
- id.Signer = signing.NewClearSigner(id.privateKey)
+ id.Signer = security.NewInMemoryECDSASigner(id.privateKey)
return nil
}
@@ -255,7 +254,7 @@
if err != nil {
return nil, err
}
- signer = signing.NewClearSigner(privKey)
+ signer = security.NewInMemoryECDSASigner(privKey)
}
id := &chainPrivateID{
diff --git a/security/serialization/serialization_test.go b/security/serialization/serialization_test.go
index ce9b2f8..430a021 100644
--- a/security/serialization/serialization_test.go
+++ b/security/serialization/serialization_test.go
@@ -14,7 +14,6 @@
"testing"
"veyron/lib/testutil"
- "veyron/security/signing"
"veyron2/security"
)
@@ -56,7 +55,7 @@
if err != nil {
panic(err)
}
- return signing.NewClearSigner(key)
+ return security.NewInMemoryECDSASigner(key)
}
func matchesErrorPattern(err error, pattern string) bool {
diff --git a/security/signing/signer.go b/security/signing/signer.go
deleted file mode 100644
index 5f15434..0000000
--- a/security/signing/signer.go
+++ /dev/null
@@ -1,34 +0,0 @@
-package signing
-
-import (
- "crypto/ecdsa"
- "crypto/rand"
-
- "veyron2/security"
-)
-
-// NewClearSigner creates a Signer that uses the provided private key to sign messages.
-// This private key is kept in the clear in the memory of the running process.
-// No hashing is applied prior to signing.
-func NewClearSigner(key *ecdsa.PrivateKey) security.Signer {
- return &clearSigner{key, security.NewECDSAPublicKey(&key.PublicKey)}
-}
-
-type clearSigner struct {
- key *ecdsa.PrivateKey
- pubkey security.PublicKey
-}
-
-func (c *clearSigner) Sign(message []byte) (sig security.Signature, err error) {
- r, s, err := ecdsa.Sign(rand.Reader, c.key, message)
- if err != nil {
- return
- }
- sig.R, sig.S = r.Bytes(), s.Bytes()
- sig.Hash = security.NoHash
- return
-}
-
-func (c *clearSigner) PublicKey() security.PublicKey {
- return c.pubkey
-}
diff --git a/security/signing/signer_test.go b/security/signing/signer_test.go
deleted file mode 100644
index 98a3352..0000000
--- a/security/signing/signer_test.go
+++ /dev/null
@@ -1,35 +0,0 @@
-package signing
-
-import (
- "crypto/ecdsa"
- "crypto/elliptic"
- "crypto/rand"
- "crypto/sha256"
- "testing"
-)
-
-func TestSigner(t *testing.T) {
- key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
- if err != nil {
- t.Fatalf("Couldn't generate ECDSA private key: %v", err)
- }
- signer := NewClearSigner(key)
- testdata := [][]byte{
- nil,
- []byte{},
- []byte(""),
- []byte("withorwithoutyou"),
- []byte("wherethestreetshavenoname"),
- }
- for _, d := range testdata {
- hash := sha256.Sum256(d)
- sig, err := signer.Sign(hash[:])
- if err != nil {
- t.Errorf("Sign(%q) returned err: %q, expected success.", d, err)
- continue
- }
- if !sig.Verify(signer.PublicKey(), hash[:]) {
- t.Errorf("Sign(%q) signature couldn't be verified.", d)
- }
- }
-}
