Skip to content

Commit

Permalink
Fix fileks and keys for ed25519
Browse files Browse the repository at this point in the history 
Changes regarding dealing with pointers to
ed25519 keys versus the values. This is a
concern because golang generally deals
with ed25519 keys by value.

This causes a difference between the default
way to deal with ecdsa keys (pointers) and
ed25519 ones (values).

As I implemented, the ed25519 keys storing and parsing
follow the general rules:

1) Ed25519 keys are parsed from files as values (ed25519.PrivateKey or
ed25519.PublicKey) but are eventually converted to pointers
to fit the structs ed25519PrivateKey and ed25519PublicKey.

2) To store ed25519 keys in the fileks, they are dealt with as pointers
until the marshalling method, from the x509 golang library. This fits
test cases for storing "nil" keys, that would cause panic if the key
were treated as value.

Fileks test cases for ed25519 were added.

Signed-off-by: Johann Westphall <johannwestphall@gmail.com>
  • Loading branch information
@johannww
johannww committed Aug 30, 2022
1 parent 0b56955 commit 889907a
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 47 deletions.
4 changes: 2 additions & 2 deletions bccsp/sw/fileks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,13 @@ func (ks *fileBasedKeyStore) StoreKey(k bccsp.Key) (err error) {
}

case *ed25519PrivateKey:
err = ks.storePrivateKey(hex.EncodeToString(k.SKI()), *kk.privKey)
err = ks.storePrivateKey(hex.EncodeToString(k.SKI()), kk.privKey)
if err != nil {
return fmt.Errorf("failed storing ED25519 private key [%s]", err)
}

case *ed25519PublicKey:
err = ks.storePublicKey(hex.EncodeToString(k.SKI()), *kk.pubKey)
err = ks.storePublicKey(hex.EncodeToString(k.SKI()), kk.pubKey)
if err != nil {
return fmt.Errorf("failed storing ED25519 public key [%s]", err)
}
Expand Down
36 changes: 36 additions & 0 deletions bccsp/sw/fileks_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package sw

import (
"crypto/ecdsa"
"crypto/ed25519"
"crypto/elliptic"
"crypto/rand"
"fmt"
Expand Down Expand Up @@ -46,6 +47,16 @@ func TestInvalidStoreKey(t *testing.T) {
t.Fatal("Error should be different from nil in this case")
}

err = ks.StoreKey(&ed25519PrivateKey{nil})
if err == nil {
t.Fatal("Error should be different from nil in this case")
}

err = ks.StoreKey(&ed25519PublicKey{nil})
if err == nil {
t.Fatal("Error should be different from nil in this case")
}

err = ks.StoreKey(&aesPrivateKey{nil, false})
if err == nil {
t.Fatal("Error should be different from nil in this case")
Expand Down Expand Up @@ -139,3 +150,28 @@ func TestDirEmpty(t *testing.T) {
require.NoError(t, err)
require.Equal(t, false, r)
}

func TestStoreAndGetEd25519Keys(t *testing.T) {
ksPath, err := ioutil.TempDir("", "bccspks")
require.NoError(t, err)
defer os.RemoveAll(ksPath)

ks, err := NewFileBasedKeyStore(nil, filepath.Join(tempDir, "bccspks"), false)
require.NoError(t, err)

pub, priv, err := ed25519.GenerateKey(rand.Reader)
require.NoError(t, err)

ed25519FabricPriv := &ed25519PrivateKey{privKey: &priv}
ed25519FabricPub := &ed25519PublicKey{pubKey: &pub}

err = ks.StoreKey(ed25519FabricPriv)
require.NoError(t, err)
_, err = ks.GetKey(ed25519FabricPriv.SKI())
require.NoError(t, err)

err = ks.StoreKey(ed25519FabricPub)
require.NoError(t, err)
_, err = ks.GetKey(ed25519FabricPub.SKI())
require.NoError(t, err)
}
28 changes: 14 additions & 14 deletions bccsp/sw/keys.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ func privateKeyToDER(privateKey crypto.PrivateKey) ([]byte, error) {
// Fabric supports ECDSA and ED25519 at the moment.
case *ecdsa.PrivateKey:
return x509.MarshalECPrivateKey(privateKey.(*ecdsa.PrivateKey))
case ed25519.PrivateKey:
return x509.MarshalPKCS8PrivateKey(privateKey)
case *ed25519.PrivateKey:
return x509.MarshalPKCS8PrivateKey(*privateKey.(*ed25519.PrivateKey))
default:
return nil, fmt.Errorf("found unknown private key type (%T) in marshaling", key)
}
Expand Down Expand Up @@ -123,12 +123,12 @@ func privateKeyToPEM(privateKey interface{}, pwd []byte) ([]byte, error) {
Bytes: pkcs8Bytes,
},
), nil
case ed25519.PrivateKey:
case *ed25519.PrivateKey:
if k == nil {
return nil, errors.New("invalid ed25519 private key. It must be different from nil")
}

pkcs8Bytes, err := x509.MarshalPKCS8PrivateKey(k)
pkcs8Bytes, err := x509.MarshalPKCS8PrivateKey(*k)
if err != nil {
return nil, fmt.Errorf("error marshaling ED key to asn1: [%s]", err)
}
Expand All @@ -140,7 +140,7 @@ func privateKeyToPEM(privateKey interface{}, pwd []byte) ([]byte, error) {
), nil

default:
return nil, errors.New("invalid key type. It must be *ecdsa.PrivateKey or ed25519.PrivateKey")
return nil, errors.New("invalid key type. It must be *ecdsa.PrivateKey or *ed25519.PrivateKey")
}
}

Expand Down Expand Up @@ -170,11 +170,11 @@ func privateKeyToEncryptedPEM(privateKey interface{}, pwd []byte) ([]byte, error
}

return pem.EncodeToMemory(block), nil
case ed25519.PrivateKey:
case *ed25519.PrivateKey:
if k == nil {
return nil, errors.New("invalid ed25519 private key. It must be different from nil")
}
raw, err := x509.MarshalPKCS8PrivateKey(k)
raw, err := x509.MarshalPKCS8PrivateKey(*k)
if err != nil {
return nil, err
}
Expand All @@ -192,7 +192,7 @@ func privateKeyToEncryptedPEM(privateKey interface{}, pwd []byte) ([]byte, error
return pem.EncodeToMemory(block), nil

default:
return nil, errors.New("invalid key type. It must be *ecdsa.PrivateKey or ed25519.PrivateKey")
return nil, errors.New("invalid key type. It must be *ecdsa.PrivateKey or *ed25519.PrivateKey")
}
}

Expand Down Expand Up @@ -325,11 +325,11 @@ func publicKeyToPEM(publicKey interface{}, pwd []byte) ([]byte, error) {
Bytes: PubASN1,
},
), nil
case ed25519.PublicKey:
case *ed25519.PublicKey:
if k == nil {
return nil, errors.New("invalid ed25519 public key. It must be different from nil")
}
PubASN1, err := x509.MarshalPKIXPublicKey(k)
PubASN1, err := x509.MarshalPKIXPublicKey(*k)
if err != nil {
return nil, err
}
Expand All @@ -342,7 +342,7 @@ func publicKeyToPEM(publicKey interface{}, pwd []byte) ([]byte, error) {
), nil

default:
return nil, errors.New("invalid key type. It must be *ecdsa.PublicKey or ed25519.PublicKey")
return nil, errors.New("invalid key type. It must be *ecdsa.PublicKey or *ed25519.PublicKey")
}
}

Expand All @@ -368,11 +368,11 @@ func publicKeyToEncryptedPEM(publicKey interface{}, pwd []byte) ([]byte, error)
}

return pem.EncodeToMemory(block), nil
case ed25519.PublicKey:
case *ed25519.PublicKey:
if k == nil {
return nil, errors.New("invalid ed25519 public key. It must be different from nil")
}
raw, err := x509.MarshalPKIXPublicKey(k)
raw, err := x509.MarshalPKIXPublicKey(*k)
if err != nil {
return nil, err
}
Expand All @@ -389,7 +389,7 @@ func publicKeyToEncryptedPEM(publicKey interface{}, pwd []byte) ([]byte, error)

return pem.EncodeToMemory(block), nil
default:
return nil, errors.New("invalid key type. It must be *ecdsa.PublicKey or ed25519.PublicKey")
return nil, errors.New("invalid key type. It must be *ecdsa.PublicKey or *ed25519.PublicKey")
}
}

Expand Down
36 changes: 5 additions & 31 deletions bccsp/sw/keys_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ func TestED25519Keys(t *testing.T) {
}

// Private Key DER format
der, err := privateKeyToDER(key)
der, err := privateKeyToDER(&key)
if err != nil {
t.Fatalf("Failed converting private key to DER [%s]", err)
}
Expand All @@ -317,8 +317,7 @@ func TestED25519Keys(t *testing.T) {
t.Fatalf("Failed converting DER to private key [%s]", err)
}

// TODO: Private Key PEM format
rawPEM, err := privateKeyToPEM(key, nil)
rawPEM, err := privateKeyToPEM(&key, nil)
if err != nil {
t.Fatalf("Failed converting private key to PEM [%s]", err)
}
Expand All @@ -340,32 +339,7 @@ func TestED25519Keys(t *testing.T) {
t.Fatal("PrivateKeyToPEM should fail on nil")
}

_, err = pemToPrivateKey(nil, nil)
if err == nil {
t.Fatal("PEMtoPublicKey should fail on nil")
}

_, err = pemToPrivateKey([]byte{0, 1, 3, 4}, nil)
if err == nil {
t.Fatal("PEMtoPublicKey should fail invalid PEM")
}

_, err = derToPrivateKey(nil)
if err == nil {
t.Fatal("DERToPrivateKey should fail on nil")
}

_, err = derToPrivateKey([]byte{0, 1, 3, 4})
if err == nil {
t.Fatal("DERToPrivateKey should fail on invalid DER")
}

_, err = privateKeyToDER(nil)
if err == nil {
t.Fatal("DERToPrivateKey should fail on nil")
}

encPEM, err := privateKeyToPEM(key, []byte("passwd"))
encPEM, err := privateKeyToPEM(&key, []byte("passwd"))
if err != nil {
t.Fatalf("Failed converting private key to encrypted PEM [%s]", err)
}
Expand All @@ -377,7 +351,7 @@ func TestED25519Keys(t *testing.T) {
}

// Public Key PEM format
rawPEM, err = publicKeyToPEM(pub, nil)
rawPEM, err = publicKeyToPEM(&pub, nil)
if err != nil {
t.Fatalf("Failed converting public key to PEM [%s]", err)
}
Expand All @@ -391,7 +365,7 @@ func TestED25519Keys(t *testing.T) {
}

// Public Key Encrypted PEM format
encPEM, err = publicKeyToPEM(pub, []byte("passwd"))
encPEM, err = publicKeyToPEM(&pub, []byte("passwd"))
if err != nil {
t.Fatalf("Failed converting private key to encrypted PEM [%s]", err)
}
Expand Down

0 comments on commit 889907a

Please sign in to comment.