fix: Refuse to use private keys that are group or world readable

- Added common.ValidatePrivateKeyFilePermissions.
- LoadArmoredKey now rejects regular private-key files with any group/other permissions: mode.Perm() & 0077 != 0.
- NewFileSigner now reuses common.LoadArmoredKey, removing duplicated key decoding.
- LoadWormchainPrivKey now validates permissions before reading.
- Added tests for unsafe 0644 key files in:
- node/pkg/common/armoredKey_test.go
- node/pkg/guardiansigner/guardiansigner_test.go
- node/pkg/wormconn/wormchain_key_test.go
- Updated query tests to copy the checked-in dev key into a temp 0600 file before loading.
- Add unit test to maintain code coverage that would've dropped due to
  refactoring

Author: johnsaigle

.coverage-baseline

@@ -6,8 +6,8 @@ github.com/certusone/wormhole/node 0.0
 github.com/certusone/wormhole/node/cmd 0.0
 github.com/certusone/wormhole/node/cmd/ccq 15.5
 github.com/certusone/wormhole/node/cmd/debug 0.0
-github.com/certusone/wormhole/node/cmd/guardiand 21.6
-github.com/certusone/wormhole/node/cmd/spy 31.1
+github.com/certusone/wormhole/node/cmd/guardiand 21.5
+github.com/certusone/wormhole/node/cmd/spy 31.2
 github.com/certusone/wormhole/node/cmd/txverifier 0.0
 github.com/certusone/wormhole/node/hack/accountant 0.0
 github.com/certusone/wormhole/node/hack/encrypt 0.0
@@ -23,21 +23,22 @@ github.com/certusone/wormhole/node/hack/repair_eth 0.0
 github.com/certusone/wormhole/node/hack/repair_solana 0.0
 github.com/certusone/wormhole/node/hack/solana 0.0
 github.com/certusone/wormhole/node/pkg/accountant 49.8
-github.com/certusone/wormhole/node/pkg/adminrpc 49.2
+github.com/certusone/wormhole/node/pkg/adminrpc 50.5
 github.com/certusone/wormhole/node/pkg/altpub 87.4
-github.com/certusone/wormhole/node/pkg/common 62.5
+github.com/certusone/wormhole/node/pkg/common 69.1
 github.com/certusone/wormhole/node/pkg/db 76.6
 github.com/certusone/wormhole/node/pkg/devnet 41.2
-github.com/certusone/wormhole/node/pkg/governor 49.7
-github.com/certusone/wormhole/node/pkg/guardiansigner 42.3
+github.com/certusone/wormhole/node/pkg/governor 49.6
+github.com/certusone/wormhole/node/pkg/guardiansigner 39.7
 github.com/certusone/wormhole/node/pkg/gwrelayer 31.9
-github.com/certusone/wormhole/node/pkg/manager 23.3
+github.com/certusone/wormhole/node/pkg/manager 29.3
 github.com/certusone/wormhole/node/pkg/manager/delegatedmanagersetabi 0.0
-github.com/certusone/wormhole/node/pkg/manager/dogecoin 93.8
-github.com/certusone/wormhole/node/pkg/node 65.2
+github.com/certusone/wormhole/node/pkg/manager/dogecoin 94.0
+github.com/certusone/wormhole/node/pkg/manager/xrpl 91.9
+github.com/certusone/wormhole/node/pkg/node 67.4
 github.com/certusone/wormhole/node/pkg/notary 36.9
 github.com/certusone/wormhole/node/pkg/p2p 40.3
-github.com/certusone/wormhole/node/pkg/processor 10.2
+github.com/certusone/wormhole/node/pkg/processor 25.0
 github.com/certusone/wormhole/node/pkg/proto/gossip/v1 0.0
 github.com/certusone/wormhole/node/pkg/proto/node/v1 0.0
 github.com/certusone/wormhole/node/pkg/proto/prometheus/v1 0.0
@@ -47,28 +48,31 @@ github.com/certusone/wormhole/node/pkg/publicrpc 12.4
 github.com/certusone/wormhole/node/pkg/query 70.2
 github.com/certusone/wormhole/node/pkg/random 0.0
 github.com/certusone/wormhole/node/pkg/readiness 0.0
+github.com/certusone/wormhole/node/pkg/suiclient 71.7
 github.com/certusone/wormhole/node/pkg/supervisor 0.0
 github.com/certusone/wormhole/node/pkg/telemetry 16.0
 github.com/certusone/wormhole/node/pkg/telemetry/prom_remote_write 59.5
 github.com/certusone/wormhole/node/pkg/txverifier 54.7
 github.com/certusone/wormhole/node/pkg/version 0.0
 github.com/certusone/wormhole/node/pkg/watchers/algorand 30.4
 github.com/certusone/wormhole/node/pkg/watchers/aptos 29.8
-github.com/certusone/wormhole/node/pkg/watchers/cosmwasm 0.0
-github.com/certusone/wormhole/node/pkg/watchers/evm 23.6
-github.com/certusone/wormhole/node/pkg/watchers/evm/connectors 43.6
+github.com/certusone/wormhole/node/pkg/watchers/cosmwasm 2.3
+github.com/certusone/wormhole/node/pkg/watchers/evm 23.5
+github.com/certusone/wormhole/node/pkg/watchers/evm/connectors 59.3
 github.com/certusone/wormhole/node/pkg/watchers/evm/connectors/delegated_guardians 0.0
 github.com/certusone/wormhole/node/pkg/watchers/evm/connectors/ethabi 0.0
 github.com/certusone/wormhole/node/pkg/watchers/evm/custom_consistency_level_abi 0.0
 github.com/certusone/wormhole/node/pkg/watchers/evm/finalizers/optimismctcabi 0.0
 github.com/certusone/wormhole/node/pkg/watchers/evm/verify_chain_config 0.0
 github.com/certusone/wormhole/node/pkg/watchers/ibc 22.0
 github.com/certusone/wormhole/node/pkg/watchers/mock 0.0
-github.com/certusone/wormhole/node/pkg/watchers/near 75.7
+github.com/certusone/wormhole/node/pkg/watchers/near 76.0
 github.com/certusone/wormhole/node/pkg/watchers/near/nearapi 61.8
 github.com/certusone/wormhole/node/pkg/watchers/near/nearapi/mock 0.0
-github.com/certusone/wormhole/node/pkg/watchers/solana 39.9
-github.com/certusone/wormhole/node/pkg/watchers/sui 7.9
-github.com/certusone/wormhole/node/pkg/wormconn 0.0
+github.com/certusone/wormhole/node/pkg/watchers/solana 41.0
+github.com/certusone/wormhole/node/pkg/watchers/sui 26.9
+github.com/certusone/wormhole/node/pkg/watchers/xrpl 72.8
+github.com/certusone/wormhole/node/pkg/watchers/xrpl/currencycodec 100.0
+github.com/certusone/wormhole/node/pkg/wormconn 2.3
 github.com/wormhole-foundation/wormhole/sdk 70.2
-github.com/wormhole-foundation/wormhole/sdk/vaa 61.4
+github.com/wormhole-foundation/wormhole/sdk/vaa 66.8

node/pkg/common/armoredKey.go

@@ -24,10 +24,15 @@ func LoadGuardianKey(filename string, unsafeDevMode bool) (*ecdsa.PrivateKey, er
 
 // LoadArmoredKey loads a serialized key from disk.
 func LoadArmoredKey(filename string, blockType string, unsafeDevMode bool) (*ecdsa.PrivateKey, error) {
+	if err := ValidatePrivateKeyFilePermissions(filename); err != nil {
+		return nil, err
+	}
+
 	f, err := os.Open(filename)
 	if err != nil {
 		return nil, fmt.Errorf("failed to open file: %w", err)
 	}
+	defer f.Close()
 
 	p, err := armor.Decode(f)
 	if err != nil {
@@ -61,6 +66,20 @@ func LoadArmoredKey(filename string, blockType string, unsafeDevMode bool) (*ecd
 	return gk, nil
 }
 
+// ValidatePrivateKeyFilePermissions rejects regular private-key files that are readable by group or other users.
+func ValidatePrivateKeyFilePermissions(filename string) error {
+	info, err := os.Stat(filename)
+	if err != nil {
+		return fmt.Errorf("failed to stat private key file: %w", err)
+	}
+
+	if info.Mode().IsRegular() && info.Mode().Perm()&0077 != 0 {
+		return fmt.Errorf("private key file %q has insecure permissions %o; expected no group or other permissions", filename, info.Mode().Perm())
+	}
+
+	return nil
+}
+
 // WriteArmoredKey serializes a key and writes it to disk.
 func WriteArmoredKey(key *ecdsa.PrivateKey, description string, filename string, blockType string, unsafe bool) error {
 	if _, err := os.Stat(filename); !os.IsNotExist(err) {

node/pkg/common/armoredKey_test.go

@@ -0,0 +1,36 @@
+package common
+
+import (
+	"os"
+	"testing"
+
+	"github.com/ethereum/go-ethereum/crypto"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestLoadArmoredKeyRejectsUnsafePermissions(t *testing.T) {
+	key, err := crypto.GenerateKey()
+	require.NoError(t, err)
+	keyPath := t.TempDir() + "/guardian.key"
+	require.NoError(t, WriteArmoredKey(key, "test key", keyPath, GuardianKeyArmoredBlock, false))
+	require.NoError(t, os.Chmod(keyPath, 0644))
+
+	loadedKey, err := LoadArmoredKey(keyPath, GuardianKeyArmoredBlock, false)
+
+	require.Error(t, err)
+	assert.Nil(t, loadedKey)
+	assert.Contains(t, err.Error(), "insecure permissions")
+}
+
+func TestLoadArmoredKeyAllowsOwnerOnlyPermissions(t *testing.T) {
+	key, err := crypto.GenerateKey()
+	require.NoError(t, err)
+	keyPath := t.TempDir() + "/guardian.key"
+	require.NoError(t, WriteArmoredKey(key, "test key", keyPath, GuardianKeyArmoredBlock, false))
+
+	loadedKey, err := LoadArmoredKey(keyPath, GuardianKeyArmoredBlock, false)
+
+	require.NoError(t, err)
+	assert.Equal(t, key, loadedKey)
+}

node/pkg/common/scissors.go

@@ -65,12 +65,12 @@ func StartRunnable(ctx context.Context, errC chan error, catchPanics bool, name
 					default:
 						err = fmt.Errorf("%s: %v", name, x)
 					}
+					ScissorsPanicsCaught.WithLabelValues(name).Inc()
 					// We don't want this to hang if the listener has already gone away.
 					select {
 					case errC <- err:
 					default:
 					}
-					ScissorsPanicsCaught.WithLabelValues(name).Inc()
 
 				}
 			}()
@@ -83,11 +83,11 @@ func StartRunnable(ctx context.Context, errC chan error, catchPanics bool, name
 func startRunnable(ctx context.Context, errC chan error, name string, runnable supervisor.Runnable) {
 	err := runnable(ctx)
 	if err != nil {
+		ScissorsErrorsCaught.WithLabelValues(name).Inc()
 		// We don't want this to hang if the listener has already gone away.
 		select {
 		case errC <- err:
 		default:
 		}
-		ScissorsErrorsCaught.WithLabelValues(name).Inc()
 	}
 }

node/pkg/guardiansigner/filesigner.go

@@ -3,17 +3,12 @@ package guardiansigner
 import (
 	"context"
 	"crypto/ecdsa"
-	"errors"
 	"fmt"
-	"os"
 
 	"github.com/ethereum/go-ethereum/crypto"
 	ethcrypto "github.com/ethereum/go-ethereum/crypto"
-	"google.golang.org/protobuf/proto"
 
 	"github.com/certusone/wormhole/node/pkg/common"
-	nodev1 "github.com/certusone/wormhole/node/pkg/proto/node/v1"
-	"golang.org/x/crypto/openpgp/armor" //nolint:staticcheck // Package is deprecated but we need it in the codebase still.
 )
 
 // FileSigner is a signer that loads a guardian key from a file. The URI is expected to be
@@ -35,38 +30,9 @@ func NewFileSigner(_ context.Context, unsafeDevMode bool, signerKeyPath string)
 		keyPath: signerKeyPath,
 	}
 
-	f, err := os.Open(signerKeyPath)
+	gk, err := common.LoadArmoredKey(signerKeyPath, GuardianKeyArmoredBlock, unsafeDevMode)
 	if err != nil {
-		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 := common.SafeRead(p.Body)
-	if err != nil {
-		return nil, fmt.Errorf("failed to read file: %w", err)
-	}
-
-	var m nodev1.GuardianKey
-	err = proto.Unmarshal(b, &m)
-	if err != nil {
-		return nil, fmt.Errorf("failed to deserialize protobuf: %w", err)
-	}
-
-	if !unsafeDevMode && m.UnsafeDeterministicKey {
-		return nil, errors.New("refusing to use deterministic key in production")
-	}
-
-	gk, err := ethcrypto.ToECDSA(m.Data)
-	if err != nil {
-		return nil, fmt.Errorf("failed to deserialize raw key data: %w", err)
+		return nil, err
 	}
 
 	fileSigner.privateKey = gk

node/pkg/guardiansigner/guardiansigner_test.go

@@ -3,8 +3,10 @@ package guardiansigner
 import (
 	"context"
 	"encoding/hex"
+	"os"
 	"testing"
 
+	"github.com/certusone/wormhole/node/pkg/common"
 	"github.com/ethereum/go-ethereum/crypto"
 	ethcrypto "github.com/ethereum/go-ethereum/crypto"
 	"github.com/stretchr/testify/assert"
@@ -61,8 +63,12 @@ func TestFileSignerNonExistentFile(t *testing.T) {
 
 func TestFileSigner(t *testing.T) {
 	ctx := context.Background()
-	fileUri := "file://../query/dev.guardian.key"
-	expectedEthAddress := "0xbeFA429d57cD18b7F8A4d91A2da9AB4AF05d0FBe"
+	key, err := crypto.GenerateKey()
+	require.NoError(t, err)
+	keyPath := t.TempDir() + "/guardian.key"
+	require.NoError(t, common.WriteArmoredKey(key, "test key", keyPath, GuardianKeyArmoredBlock, false))
+	fileUri := "file://" + keyPath
+	expectedEthAddress := ethcrypto.PubkeyToAddress(key.PublicKey).Hex()
 
 	// For each file signer generation attempt, check:
 	//	That the signer returned is not nil
@@ -102,6 +108,43 @@ func TestFileSigner(t *testing.T) {
 
 }
 
+func TestFileSignerRejectsUnsafePermissions(t *testing.T) {
+	key, err := crypto.GenerateKey()
+	require.NoError(t, err)
+	keyPath := t.TempDir() + "/guardian.key"
+	require.NoError(t, common.WriteArmoredKey(key, "test key", keyPath, GuardianKeyArmoredBlock, false))
+	require.NoError(t, os.Chmod(keyPath, 0644))
+
+	fileSigner, err := NewFileSigner(context.Background(), true, keyPath)
+
+	require.Error(t, err)
+	assert.Nil(t, fileSigner)
+	assert.Contains(t, err.Error(), "insecure permissions")
+}
+
+func TestGeneratedSigner(t *testing.T) {
+	ctx := context.Background()
+	key, err := crypto.GenerateKey()
+	require.NoError(t, err)
+
+	signer, err := GenerateSignerWithPrivatekeyUnsafe(key)
+	require.NoError(t, err)
+	assert.Equal(t, "generated", signer.TypeAsString())
+	assert.Equal(t, key.PublicKey, signer.PublicKey(ctx))
+
+	hash := crypto.Keccak256Hash([]byte("data"))
+	sig, err := signer.Sign(ctx, hash.Bytes())
+	require.NoError(t, err)
+
+	valid, err := signer.Verify(ctx, sig, hash.Bytes())
+	require.NoError(t, err)
+	assert.True(t, valid)
+
+	valid, err = signer.Verify(ctx, sig, crypto.Keccak256Hash([]byte("other data")).Bytes())
+	require.NoError(t, err)
+	assert.False(t, valid)
+}
+
 func TestAmazonKmsAdjustBufferSize(t *testing.T) {
 
 	bytes_30_null_0102, _ := hex.DecodeString("0000000000000000000000000000000000000000000000000000000000000102")

node/pkg/query/query_test.go

@@ -7,6 +7,7 @@ import (
 	"encoding/hex"
 	"fmt"
 	"math"
+	"os"
 	"strconv"
 	"strings"
 	"sync"
@@ -417,7 +418,16 @@ func createQueryHandlerForTestWithoutPublisher(t *testing.T, ctx context.Context
 	md := mockData{}
 	var err error
 
-	md.sk, err = common.LoadGuardianKey("dev.guardian.key", true)
+	devKey, err := os.ReadFile("dev.guardian.key")
+	require.NoError(t, err)
+	devKeyFile, err := os.CreateTemp(t.TempDir(), "dev.guardian.key")
+	require.NoError(t, err)
+	_, err = devKeyFile.Write(devKey)
+	require.NoError(t, err)
+	require.NoError(t, devKeyFile.Close())
+	devKeyPath := devKeyFile.Name()
+
+	md.sk, err = common.LoadGuardianKey(devKeyPath, true)
 	require.NoError(t, err)
 	require.NotNil(t, md.sk)
 

node/pkg/wormconn/wormchain_key.go

@@ -3,11 +3,16 @@ package wormconn
 import (
 	"os"
 
+	"github.com/certusone/wormhole/node/pkg/common"
 	"github.com/cosmos/cosmos-sdk/crypto"
 	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
 )
 
 func LoadWormchainPrivKey(path string, passPhrase string) (cryptotypes.PrivKey, error) {
+	if err := common.ValidatePrivateKeyFilePermissions(path); err != nil {
+		return nil, err
+	}
+
 	armor, err := os.ReadFile(path)
 	if err != nil {
 		return nil, err

node/pkg/wormconn/wormchain_key_test.go

@@ -0,0 +1,20 @@
+package wormconn
+
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestLoadWormchainPrivKeyRejectsUnsafePermissions(t *testing.T) {
+	keyPath := t.TempDir() + "/wormchain.key"
+	require.NoError(t, os.WriteFile(keyPath, []byte("not a valid encrypted key"), 0644)) // #nosec G306 -- This test verifies unsafe key-file permissions are rejected.
+
+	key, err := LoadWormchainPrivKey(keyPath, "passphrase")
+
+	require.Error(t, err)
+	assert.Nil(t, key)
+	assert.Contains(t, err.Error(), "insecure permissions")
+}