From 471417cd6ed36f62f163249b8390c46048c89368 Mon Sep 17 00:00:00 2001
From: Leo <leo@certus.one>
Date: Fri, 20 Nov 2020 22:35:32 +0100
Subject: [PATCH] bridge: use GPG-armored binary protobufs for keys

---
 bridge/cmd/guardiand/bridgekey.go | 61 ++++++++++++++++++++++++-------
 proto/node/v1/node.proto          |  8 +---
 2 files changed, 49 insertions(+), 20 deletions(-)

diff --git a/bridge/cmd/guardiand/bridgekey.go b/bridge/cmd/guardiand/bridgekey.go
index 925cde3e..b4bd2138 100644
--- a/bridge/cmd/guardiand/bridgekey.go
+++ b/bridge/cmd/guardiand/bridgekey.go
@@ -11,7 +11,8 @@ import (
 
 	ethcrypto "github.com/ethereum/go-ethereum/crypto"
 	"github.com/spf13/cobra"
-	"google.golang.org/protobuf/encoding/prototext"
+	"golang.org/x/crypto/openpgp/armor"
+	"google.golang.org/protobuf/proto"
 
 	"github.com/certusone/wormhole/bridge/pkg/devnet"
 	nodev1 "github.com/certusone/wormhole/bridge/pkg/proto/node/v1"
@@ -19,6 +20,10 @@ import (
 
 var keyDescription *string
 
+const (
+	GuardianKeyArmoredBlock = "WORMHOLE GUARDIAN PRIVATE KEY"
+)
+
 func init() {
 	keyDescription = KeygenCmd.Flags().String("desc", "", "Human-readable key description (optional)")
 }
@@ -49,20 +54,34 @@ func runKeygen(cmd *cobra.Command, args []string) {
 
 // loadGuardianKey loads a serialized guardian key from disk.
 func loadGuardianKey(filename string) (*ecdsa.PrivateKey, error) {
-	b, err := ioutil.ReadFile(filename)
+	f, err := os.Open(filename)
 	if err != nil {
-		return nil, fmt.Errorf("failed to read guardian private key from disk: %w", err)
+		return nil, fmt.Errorf("failed to open file: %w", err)
+	}
+
+	p, err := armor.Decode(f)
+	if err != nil {
+		return nil, fmt.Errorf("failed to read armored file: %w", err)
+	}
+
+	if p.Type != GuardianKeyArmoredBlock {
+		return nil, fmt.Errorf("invalid block type: %s", p.Type)
+	}
+
+	b, err := ioutil.ReadAll(p.Body)
+	if err != nil {
+		return nil, fmt.Errorf("failed to read file: %w", err)
 	}
 
 	var m nodev1.GuardianKey
-	err = prototext.Unmarshal(b, &m)
+	err = proto.Unmarshal(b, &m)
 	if err != nil {
-		return nil, fmt.Errorf("failed to deserialize private key from disk: %w", err)
+		return nil, fmt.Errorf("failed to deserialize protobuf: %w", err)
 	}
 
 	gk, err := ethcrypto.ToECDSA(m.Data)
 	if err != nil {
-		return nil, fmt.Errorf("failed to deserialize key data: %w", err)
+		return nil, fmt.Errorf("failed to deserialize raw key data: %w", err)
 	}
 
 	return gk, nil
@@ -75,21 +94,37 @@ func writeGuardianKey(key *ecdsa.PrivateKey, description string, filename string
 	}
 
 	m := &nodev1.GuardianKey{
-		Description: description,
-		Data:        ethcrypto.FromECDSA(key),
-		Pubkey:      ethcrypto.PubkeyToAddress(key.PublicKey).String(),
+		Data: ethcrypto.FromECDSA(key),
 	}
 
-	b, err := prototext.MarshalOptions{Multiline: true, EmitASCII: true}.Marshal(m)
+	// The private key is a really long-lived piece of data, and we really want to use the stable binary
+	// protobuf encoding with field tags to make sure that we can safely evolve it in the future.
+	b, err := proto.Marshal(m)
 	if err != nil {
 		panic(err)
 	}
 
-	if err := ioutil.WriteFile(filename, b, 0600); err != nil {
-		return err
+	f, err := os.OpenFile(filename, os.O_RDWR|os.O_CREATE, 0600)
+	if err != nil {
+		return fmt.Errorf("failed to open file: %w", err)
 	}
 
-	return nil
+	a, err := armor.Encode(f, GuardianKeyArmoredBlock, map[string]string{
+		"Description": description,
+		"PublicKey":   ethcrypto.PubkeyToAddress(key.PublicKey).String(),
+	})
+	if err != nil {
+		panic(err)
+	}
+	_, err = a.Write(b)
+	if err != nil {
+		return fmt.Errorf("failed to write to file: %w", err)
+	}
+	err = a.Close()
+	if err != nil {
+		return err
+	}
+	return f.Close()
 }
 
 // generateDevnetGuardianKey returns a deterministic testnet key.
diff --git a/proto/node/v1/node.proto b/proto/node/v1/node.proto
index d62e40b0..a42a1752 100644
--- a/proto/node/v1/node.proto
+++ b/proto/node/v1/node.proto
@@ -59,12 +59,6 @@ message SubmitGuardianSetVAAResponse {
 
 // GuardianKey specifies the on-disk format for a node's guardian key.
 message GuardianKey {
-    // description is an optional, free-form description text set by the operator.
-    string description = 1;
-
     // data is the binary representation of the secp256k1 private key.
-    bytes data = 2;
-
-    // pubkey is a human-readable representation of the key, included for operator convenience.
-    string pubkey = 3;
+    bytes data = 1;
 }