whisper: identity management refactored

cmd/wnode/main.go

@@ -64,6 +64,7 @@ var (
 	asymKey    *ecdsa.PrivateKey
 	nodeid     *ecdsa.PrivateKey
 	topic      whisper.TopicType
+	asymKeyID  string
 	filterID   string
 	symPass    string
 	msPassword string
@@ -198,9 +199,26 @@ func initialize() {
 		shh = whisper.New()
 	}
 
-	asymKey = shh.NewIdentity()
+	asymKeyID, err = shh.NewIdentity()
+	if err != nil {
+		utils.Fatalf("Failed to generate a new key pair: %s", err)
+	}
+
+	asymKey, err = shh.GetIdentity(asymKeyID)
+	if err != nil {
+		utils.Fatalf("Failed to retrieve a new key pair: %s", err)
+	}
+
 	if nodeid == nil {
-		nodeid = shh.NewIdentity()
+		tmpID, err := shh.NewIdentity()
+		if err != nil {
+			utils.Fatalf("Failed to generate a new key pair: %s", err)
+		}
+
+		nodeid, err = shh.GetIdentity(tmpID)
+		if err != nil {
+			utils.Fatalf("Failed to retrieve a new key pair: %s", err)
+		}
 	}
 
 	maxPeers := 80

whisper/mailserver/server_test.go

@@ -102,7 +102,14 @@ func TestMailServer(t *testing.T) {
 }
 
 func deliverTest(t *testing.T, server *WMailServer, env *whisper.Envelope) {
-	testPeerID := shh.NewIdentity()
+	id, err := shh.NewIdentity()
+	if err != nil {
+		t.Fatalf("failed to generate new key pair with seed %d: %s.", seed, err)
+	}
+	testPeerID, err := shh.GetIdentity(id)
+	if err != nil {
+		t.Fatalf("failed to retireve new key pair with seed %d: %s.", seed, err)
+	}
 	birth := env.Expiry - env.TTL
 	p := &ServerTestParams{
 		topic: env.Topic,

whisper/whisperv5/api.go

@@ -108,19 +108,19 @@ func (api *PublicWhisperAPI) MarkPeerTrusted(peerID hexutil.Bytes) error {
 
 // HasIdentity checks if the whisper node is configured with the private key
 // of the specified public pair.
-func (api *PublicWhisperAPI) HasIdentity(identity string) (bool, error) {
+func (api *PublicWhisperAPI) HasIdentity(id string) (bool, error) {
 	if api.whisper == nil {
 		return false, whisperOffLineErr
 	}
-	return api.whisper.HasIdentity(identity), nil
+	return api.whisper.HasIdentity(id), nil
 }
 
 // DeleteIdentity deletes the specifies key if it exists.
-func (api *PublicWhisperAPI) DeleteIdentity(identity string) (bool, error) {
+func (api *PublicWhisperAPI) DeleteIdentity(id string) (bool, error) {
 	if api.whisper == nil {
 		return false, whisperOffLineErr
 	}
-	success := api.whisper.DeleteIdentity(identity)
+	success := api.whisper.DeleteIdentity(id)
 	return success, nil
 }
 
@@ -130,17 +130,32 @@ func (api *PublicWhisperAPI) NewIdentity() (string, error) {
 	if api.whisper == nil {
 		return "", whisperOffLineErr
 	}
-	identity := api.whisper.NewIdentity()
+	return api.whisper.NewIdentity()
-	return common.ToHex(crypto.FromECDSAPub(&identity.PublicKey)), nil
 }
 
-// todo: implement
+// GetPublicKey returns the public key for identity id
-//func (api *PublicWhisperAPI) GetPublicKey(id string) (bool, error) {
+func (api *PublicWhisperAPI) GetPublicKey(id string) (string, error) {
-//	if api.whisper == nil {
+	if api.whisper == nil {
-//		return false, whisperOffLineErr
+		return "", whisperOffLineErr
-//	}
+	}
-//	return api.whisper.HasIdentity(identity), nil
+	key, err := api.whisper.GetIdentity(id)
-//}
+	if err != nil {
+		return "", err
+	}
+	return common.ToHex(crypto.FromECDSAPub(&key.PublicKey)), nil
+}
+
+// GetPrivateKey returns the private key for identity id
+func (api *PublicWhisperAPI) GetPrivateKey(id string) (string, error) {
+	if api.whisper == nil {
+		return "", whisperOffLineErr
+	}
+	key, err := api.whisper.GetIdentity(id)
+	if err != nil {
+		return "", err
+	}
+	return common.ToHex(crypto.FromECDSA(key)), nil
+}
 
 // GenerateSymKey generates a random symmetric key and stores it under id,
 // which is then returned. Will be used in the future for session key exchange.
@@ -249,7 +264,10 @@ func (api *PublicWhisperAPI) NewFilter(args WhisperFilterArgs) (string, error) {
 			log.Error(fmt.Sprintf(info))
 			return "", errors.New(info)
 		}
-		filter.KeyAsym = api.whisper.GetIdentity(string(args.To))
+		filter.KeyAsym, err = api.whisper.GetIdentity(string(args.To))
+		if err != nil {
+			return "", err
+		}
 		if filter.KeyAsym == nil {
 			info := "NewFilter: non-existent identity provided"
 			log.Error(fmt.Sprintf(info))
@@ -334,7 +352,10 @@ func (api *PublicWhisperAPI) Post(args PostArgs) error {
 			log.Error(fmt.Sprintf(info))
 			return errors.New(info)
 		}
-		params.Src = api.whisper.GetIdentity(string(args.From))
+		params.Src, err = api.whisper.GetIdentity(string(args.From))
+		if err != nil {
+			return err
+		}
 		if params.Src == nil {
 			info := "Post: non-existent identity provided"
 			log.Error(fmt.Sprintf(info))

whisper/whisperv5/whisper.go

@@ -188,22 +188,32 @@ func (w *Whisper) SendP2PDirect(peer *Peer, envelope *Envelope) error {
 }
 
 // NewIdentity generates a new cryptographic identity for the client, and injects
-// it into the known identities for message decryption.
+// it into the known identities for message decryption. Returns ID of the new key pair.
-func (w *Whisper) NewIdentity() *ecdsa.PrivateKey {
+func (w *Whisper) NewIdentity() (string, error) {
 	key, err := crypto.GenerateKey()
 	if err != nil || !validatePrivateKey(key) {
 		key, err = crypto.GenerateKey() // retry once
 	}
 	if err != nil {
-		panic(err)
+		return "", err
 	}
 	if !validatePrivateKey(key) {
-		panic("Failed to generate valid key")
+		return "", fmt.Errorf("Failed to generate valid key")
 	}
+
+	id, err := GenerateRandomID()
+	if err != nil {
+		return "", fmt.Errorf("Failed to generate ID: %s", err)
+	}
+
 	w.keyMu.Lock()
 	defer w.keyMu.Unlock()
-	w.privateKeys[common.ToHex(crypto.FromECDSAPub(&key.PublicKey))] = key
+
-	return key
+	if w.privateKeys[id] != nil {
+		return "", fmt.Errorf("Failed to generate unique ID")
+	}
+	w.privateKeys[id] = key
+	return id, nil
 }
 
 // DeleteIdentity deletes the specified key if it exists.
@@ -220,17 +230,21 @@ func (w *Whisper) DeleteIdentity(key string) bool {
 
 // HasIdentity checks if the the whisper node is configured with the private key
 // of the specified public pair.
-func (w *Whisper) HasIdentity(pubKey string) bool {
+func (w *Whisper) HasIdentity(id string) bool {
 	w.keyMu.RLock()
 	defer w.keyMu.RUnlock()
-	return w.privateKeys[pubKey] != nil
+	return w.privateKeys[id] != nil
 }
 
 // GetIdentity retrieves the private key of the specified public identity.
-func (w *Whisper) GetIdentity(pubKey string) *ecdsa.PrivateKey {
+func (w *Whisper) GetIdentity(pubKey string) (*ecdsa.PrivateKey, error) {
 	w.keyMu.RLock()
 	defer w.keyMu.RUnlock()
-	return w.privateKeys[pubKey]
+	key := w.privateKeys[pubKey]
+	if key == nil {
+		return nil, fmt.Errorf("invalid id")
+	}
+	return key, nil
 }
 
 func (w *Whisper) GenerateSymKey() (string, error) {

whisper/whisperv5/whisper_test.go

@@ -20,9 +20,6 @@ import (
 	"bytes"
 	"testing"
 	"time"
-
-	"github.com/ethereum/go-ethereum/common"
-	"github.com/ethereum/go-ethereum/crypto"
 )
 
 func TestWhisperBasic(t *testing.T) {
@@ -103,7 +100,14 @@ func TestWhisperBasic(t *testing.T) {
 		t.Fatalf("failed BytesToIntBigEndian: %d.", be)
 	}
 
-	pk := w.NewIdentity()
+	id, err := w.NewIdentity()
+	if err != nil {
+		t.Fatalf("failed to generate new key pair: %s.", err)
+	}
+	pk, err := w.GetIdentity(id)
+	if err != nil {
+		t.Fatalf("failed to retrieve new key pair: %s.", err)
+	}
 	if !validatePrivateKey(pk) {
 		t.Fatalf("failed validatePrivateKey: %v.", pk)
 	}
@@ -114,67 +118,110 @@ func TestWhisperBasic(t *testing.T) {
 
 func TestWhisperIdentityManagement(t *testing.T) {
 	w := New()
-	id1 := w.NewIdentity()
+	id1, err := w.NewIdentity()
-	id2 := w.NewIdentity()
+	if err != nil {
-	pub1 := common.ToHex(crypto.FromECDSAPub(&id1.PublicKey))
+		t.Fatalf("failed to generate new key pair: %s.", err)
-	pub2 := common.ToHex(crypto.FromECDSAPub(&id2.PublicKey))
+	}
-	pk1 := w.GetIdentity(pub1)
+	id2, err := w.NewIdentity()
-	pk2 := w.GetIdentity(pub2)
+	if err != nil {
-	if !w.HasIdentity(pub1) {
+		t.Fatalf("failed to generate new key pair: %s.", err)
+	}
+	pk1, err := w.GetIdentity(id1)
+	if err != nil {
+		t.Fatalf("failed to retrieve the key pair: %s.", err)
+	}
+	pk2, err := w.GetIdentity(id2)
+	if err != nil {
+		t.Fatalf("failed to retrieve the key pair: %s.", err)
+	}
+	// todo: delete
+	//pub1 := common.ToHex(crypto.FromECDSAPub(&id1.PublicKey))
+	//pub2 := common.ToHex(crypto.FromECDSAPub(&id2.PublicKey))
+	//pub1 := pk1.PublicKey
+	//pub2 := pk2.PublicKey
+
+	if !w.HasIdentity(id1) {
 		t.Fatalf("failed HasIdentity(pub1).")
 	}
-	if !w.HasIdentity(pub2) {
+	if !w.HasIdentity(id2) {
 		t.Fatalf("failed HasIdentity(pub2).")
 	}
-	if pk1 != id1 {
+	if pk1 == nil {
 		t.Fatalf("failed GetIdentity(pub1).")
 	}
-	if pk2 != id2 {
+	if pk2 == nil {
 		t.Fatalf("failed GetIdentity(pub2).")
 	}
 
 	// Delete one identity
-	w.DeleteIdentity(pub1)
+	done := w.DeleteIdentity(id1)
-	pk1 = w.GetIdentity(pub1)
+	if !done {
-	pk2 = w.GetIdentity(pub2)
+		t.Fatalf("failed to delete id1.")
-	if w.HasIdentity(pub1) {
+	}
+	pk1, err = w.GetIdentity(id1)
+	if err == nil {
+		t.Fatalf("retrieve the key pair: false positive.")
+	}
+	pk2, err = w.GetIdentity(id2)
+	if err != nil {
+		t.Fatalf("failed to retrieve the key pair: %s.", err)
+	}
+	if w.HasIdentity(id1) {
 		t.Fatalf("failed DeleteIdentity(pub1): still exist.")
 	}
-	if !w.HasIdentity(pub2) {
+	if !w.HasIdentity(id2) {
 		t.Fatalf("failed DeleteIdentity(pub1): pub2 does not exist.")
 	}
 	if pk1 != nil {
 		t.Fatalf("failed DeleteIdentity(pub1): first key still exist.")
 	}
-	if pk2 != id2 {
+	if pk2 == nil {
 		t.Fatalf("failed DeleteIdentity(pub1): second key does not exist.")
 	}
 
 	// Delete again non-existing identity
-	w.DeleteIdentity(pub1)
+	done = w.DeleteIdentity(id1)
-	pk1 = w.GetIdentity(pub1)
+	if done {
-	pk2 = w.GetIdentity(pub2)
+		t.Fatalf("delete id1: false positive.")
-	if w.HasIdentity(pub1) {
+	}
+	pk1, err = w.GetIdentity(id1)
+	if err == nil {
+		t.Fatalf("retrieve the key pair: false positive.")
+	}
+	pk2, err = w.GetIdentity(id2)
+	if err != nil {
+		t.Fatalf("failed to retrieve the key pair: %s.", err)
+	}
+	if w.HasIdentity(id1) {
 		t.Fatalf("failed delete non-existing identity: exist.")
 	}
-	if !w.HasIdentity(pub2) {
+	if !w.HasIdentity(id2) {
 		t.Fatalf("failed delete non-existing identity: pub2 does not exist.")
 	}
 	if pk1 != nil {
 		t.Fatalf("failed delete non-existing identity: first key exist.")
 	}
-	if pk2 != id2 {
+	if pk2 == nil {
 		t.Fatalf("failed delete non-existing identity: second key does not exist.")
 	}
 
 	// Delete second identity
-	w.DeleteIdentity(pub2)
+	done = w.DeleteIdentity(id2)
-	pk1 = w.GetIdentity(pub1)
+	if !done {
-	pk2 = w.GetIdentity(pub2)
+		t.Fatalf("failed to delete id2.")
-	if w.HasIdentity(pub1) {
+	}
+	pk1, err = w.GetIdentity(id1)
+	if err == nil {
+		t.Fatalf("retrieve the key pair: false positive.")
+	}
+	pk2, err = w.GetIdentity(id2)
+	if err == nil {
+		t.Fatalf("retrieve the key pair: false positive.")
+	}
+	if w.HasIdentity(id1) {
 		t.Fatalf("failed delete second identity: first identity exist.")
 	}
-	if w.HasIdentity(pub2) {
+	if w.HasIdentity(id2) {
 		t.Fatalf("failed delete second identity: still exist.")
 	}
 	if pk1 != nil {