Skip to content
This repository has been archived by the owner on Sep 6, 2022. It is now read-only.

Commit

Permalink
feat: harden encoding/decoding functions against panics
Browse files Browse the repository at this point in the history 
Part of libp2p/go-libp2p#1389

These kinds of functions:

1. Handle user input.
2. Often have out-of-bounds, null pointer, etc bugs.
3. Have completely isolated logic where local panics are unlikely to
   cause memory corruption elsewhere.
  • Loading branch information
@Stebalien
Stebalien committed Apr 18, 2022
1 parent 2a28e1d commit ef3e50c
Show file tree
Hide file tree
Showing 10 changed files with 126 additions and 31 deletions.
32 changes: 25 additions & 7 deletions crypto/ecdsa.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"math/big"

pb "github.com/libp2p/go-libp2p-core/crypto/pb"
"github.com/libp2p/go-libp2p-core/internal/catch"

"github.com/minio/sha256-simd"
)
Expand Down Expand Up @@ -73,17 +74,21 @@ func ECDSAPublicKeyFromPubKey(pub ecdsa.PublicKey) (PubKey, error) {
}

// MarshalECDSAPrivateKey returns x509 bytes from a private key
func MarshalECDSAPrivateKey(ePriv ECDSAPrivateKey) ([]byte, error) {
func MarshalECDSAPrivateKey(ePriv ECDSAPrivateKey) (res []byte, err error) {
defer func() { catch.HandlePanic(recover(), &err, "ECDSA private-key marshal") }()
return x509.MarshalECPrivateKey(ePriv.priv)
}

// MarshalECDSAPublicKey returns x509 bytes from a public key
func MarshalECDSAPublicKey(ePub ECDSAPublicKey) ([]byte, error) {
func MarshalECDSAPublicKey(ePub ECDSAPublicKey) (res []byte, err error) {
defer func() { catch.HandlePanic(recover(), &err, "ECDSA public-key marshal") }()
return x509.MarshalPKIXPublicKey(ePub.pub)
}

// UnmarshalECDSAPrivateKey returns a private key from x509 bytes
func UnmarshalECDSAPrivateKey(data []byte) (PrivKey, error) {
func UnmarshalECDSAPrivateKey(data []byte) (res PrivKey, err error) {
defer func() { catch.HandlePanic(recover(), &err, "ECDSA private-key unmarshal") }()

priv, err := x509.ParseECPrivateKey(data)
if err != nil {
return nil, err
Expand All @@ -93,7 +98,9 @@ func UnmarshalECDSAPrivateKey(data []byte) (PrivKey, error) {
}

// UnmarshalECDSAPublicKey returns the public key from x509 bytes
func UnmarshalECDSAPublicKey(data []byte) (PubKey, error) {
func UnmarshalECDSAPublicKey(data []byte) (key PubKey, err error) {
defer func() { catch.HandlePanic(recover(), &err, "ECDSA public-key unmarshal") }()

pubIfc, err := x509.ParsePKIXPublicKey(data)
if err != nil {
return nil, err
Expand All @@ -113,7 +120,8 @@ func (ePriv *ECDSAPrivateKey) Type() pb.KeyType {
}

// Raw returns x509 bytes from a private key
func (ePriv *ECDSAPrivateKey) Raw() ([]byte, error) {
func (ePriv *ECDSAPrivateKey) Raw() (res []byte, err error) {
defer func() { catch.HandlePanic(recover(), &err, "ECDSA private-key marshal") }()
return x509.MarshalECPrivateKey(ePriv.priv)
}

Expand All @@ -123,7 +131,8 @@ func (ePriv *ECDSAPrivateKey) Equals(o Key) bool {
}

// Sign returns the signature of the input data
func (ePriv *ECDSAPrivateKey) Sign(data []byte) ([]byte, error) {
func (ePriv *ECDSAPrivateKey) Sign(data []byte) (sig []byte, err error) {
defer func() { catch.HandlePanic(recover(), &err, "ECDSA signing") }()
hash := sha256.Sum256(data)
r, s, err := ecdsa.Sign(rand.Reader, ePriv.priv, hash[:])
if err != nil {
Expand Down Expand Up @@ -157,7 +166,16 @@ func (ePub *ECDSAPublicKey) Equals(o Key) bool {
}

// Verify compares data to a signature
func (ePub *ECDSAPublicKey) Verify(data, sigBytes []byte) (bool, error) {
func (ePub *ECDSAPublicKey) Verify(data, sigBytes []byte) (success bool, err error) {
defer func() {
catch.HandlePanic(recover(), &err, "ECDSA signature verification")

// Just to be extra paranoid.
if err != nil {
success = false
}
}()

sig := new(ECDSASig)
if _, err := asn1.Unmarshal(sigBytes, sig); err != nil {
return false, err
Expand Down
15 changes: 13 additions & 2 deletions crypto/ed25519.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"io"

pb "github.com/libp2p/go-libp2p-core/crypto/pb"
"github.com/libp2p/go-libp2p-core/internal/catch"
)

// Ed25519PrivateKey is an ed25519 private key.
Expand Down Expand Up @@ -74,7 +75,9 @@ func (k *Ed25519PrivateKey) GetPublic() PubKey {
}

// Sign returns a signature from an input message.
func (k *Ed25519PrivateKey) Sign(msg []byte) ([]byte, error) {
func (k *Ed25519PrivateKey) Sign(msg []byte) (res []byte, err error) {
defer func() { catch.HandlePanic(recover(), &err, "ed15519 signing") }()

return ed25519.Sign(k.k, msg), nil
}

Expand All @@ -99,7 +102,15 @@ func (k *Ed25519PublicKey) Equals(o Key) bool {
}

// Verify checks a signature agains the input data.
func (k *Ed25519PublicKey) Verify(data []byte, sig []byte) (bool, error) {
func (k *Ed25519PublicKey) Verify(data []byte, sig []byte) (success bool, err error) {
defer func() {
catch.HandlePanic(recover(), &err, "ed15519 signature verification")

// To be safe.
if err != nil {
success = false
}
}()
return ed25519.Verify(k.k, data, sig), nil
}

Expand Down
12 changes: 9 additions & 3 deletions crypto/key_openssl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,19 @@ import (

btcec "github.com/btcsuite/btcd/btcec"
openssl "github.com/libp2p/go-openssl"

"github.com/libp2p/go-libp2p-core/internal/catch"
)

// KeyPairFromStdKey wraps standard library (and secp256k1) private keys in libp2p/go-libp2p-core/crypto keys
func KeyPairFromStdKey(priv crypto.PrivateKey) (PrivKey, PubKey, error) {
func KeyPairFromStdKey(priv crypto.PrivateKey) (_priv PrivKey, _pub PubKey, err error) {
if priv == nil {
return nil, nil, ErrNilPrivateKey
}

switch p := priv.(type) {
case *rsa.PrivateKey:
defer func() { catch.HandlePanic(recover(), &err, "x509 private key marshaling") }()
pk, err := openssl.LoadPrivateKeyFromDER(x509.MarshalPKCS1PrivateKey(p))
if err != nil {
return nil, nil, err
Expand All @@ -48,12 +51,13 @@ func KeyPairFromStdKey(priv crypto.PrivateKey) (PrivKey, PubKey, error) {
}

// PrivKeyToStdKey converts libp2p/go-libp2p-core/crypto private keys to standard library (and secp256k1) private keys
func PrivKeyToStdKey(priv PrivKey) (crypto.PrivateKey, error) {
func PrivKeyToStdKey(priv PrivKey) (_priv crypto.PrivateKey, err error) {
if priv == nil {
return nil, ErrNilPrivateKey
}
switch p := priv.(type) {
case *opensslPrivateKey:
defer func() { catch.HandlePanic(recover(), &err, "x509 private key parsing") }()
raw, err := p.Raw()
if err != nil {
return nil, err
Expand All @@ -71,13 +75,15 @@ func PrivKeyToStdKey(priv PrivKey) (crypto.PrivateKey, error) {
}

// PubKeyToStdKey converts libp2p/go-libp2p-core/crypto private keys to standard library (and secp256k1) public keys
func PubKeyToStdKey(pub PubKey) (crypto.PublicKey, error) {
func PubKeyToStdKey(pub PubKey) (key crypto.PublicKey, err error) {
if pub == nil {
return nil, ErrNilPublicKey
}

switch p := pub.(type) {
case *opensslPublicKey:
defer func() { catch.HandlePanic(recover(), &err, "x509 public key parsing") }()

raw, err := p.Raw()
if err != nil {
return nil, err
Expand Down
28 changes: 21 additions & 7 deletions crypto/rsa_go.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"io"

pb "github.com/libp2p/go-libp2p-core/crypto/pb"
"github.com/libp2p/go-libp2p-core/internal/catch"

"github.com/minio/sha256-simd"
)
Expand Down Expand Up @@ -42,9 +43,17 @@ func GenerateRSAKeyPair(bits int, src io.Reader) (PrivKey, PubKey, error) {
}

// Verify compares a signature against input data
func (pk *RsaPublicKey) Verify(data, sig []byte) (bool, error) {
func (pk *RsaPublicKey) Verify(data, sig []byte) (success bool, err error) {
defer func() {
catch.HandlePanic(recover(), &err, "RSA signature verification")

// To be safe
if err != nil {
success = false
}
}()
hashed := sha256.Sum256(data)
err := rsa.VerifyPKCS1v15(&pk.k, crypto.SHA256, hashed[:], sig)
err = rsa.VerifyPKCS1v15(&pk.k, crypto.SHA256, hashed[:], sig)
if err != nil {
return false, err
}
Expand All @@ -55,7 +64,8 @@ func (pk *RsaPublicKey) Type() pb.KeyType {
return pb.KeyType_RSA
}

func (pk *RsaPublicKey) Raw() ([]byte, error) {
func (pk *RsaPublicKey) Raw() (res []byte, err error) {
defer func() { catch.HandlePanic(recover(), &err, "RSA public-key marshaling") }()
return x509.MarshalPKIXPublicKey(&pk.k)
}

Expand All @@ -71,7 +81,8 @@ func (pk *RsaPublicKey) Equals(k Key) bool {
}

// Sign returns a signature of the input data
func (sk *RsaPrivateKey) Sign(message []byte) ([]byte, error) {
func (sk *RsaPrivateKey) Sign(message []byte) (sig []byte, err error) {
defer func() { catch.HandlePanic(recover(), &err, "RSA signing") }()
hashed := sha256.Sum256(message)
return rsa.SignPKCS1v15(rand.Reader, &sk.sk, crypto.SHA256, hashed[:])
}
Expand All @@ -85,7 +96,8 @@ func (sk *RsaPrivateKey) Type() pb.KeyType {
return pb.KeyType_RSA
}

func (sk *RsaPrivateKey) Raw() ([]byte, error) {
func (sk *RsaPrivateKey) Raw() (res []byte, err error) {
defer func() { catch.HandlePanic(recover(), &err, "RSA private-key marshaling") }()
b := x509.MarshalPKCS1PrivateKey(&sk.sk)
return b, nil
}
Expand All @@ -106,7 +118,8 @@ func (sk *RsaPrivateKey) Equals(k Key) bool {
}

// UnmarshalRsaPrivateKey returns a private key from the input x509 bytes
func UnmarshalRsaPrivateKey(b []byte) (PrivKey, error) {
func UnmarshalRsaPrivateKey(b []byte) (key PrivKey, err error) {
defer func() { catch.HandlePanic(recover(), &err, "RSA private-key unmarshaling") }()
sk, err := x509.ParsePKCS1PrivateKey(b)
if err != nil {
return nil, err
Expand All @@ -118,7 +131,8 @@ func UnmarshalRsaPrivateKey(b []byte) (PrivKey, error) {
}

// UnmarshalRsaPublicKey returns a public key from the input x509 bytes
func UnmarshalRsaPublicKey(b []byte) (PubKey, error) {
func UnmarshalRsaPublicKey(b []byte) (key PubKey, err error) {
defer func() { catch.HandlePanic(recover(), &err, "RSA public-key unmarshaling") }()
pub, err := x509.ParsePKIXPublicKey(b)
if err != nil {
return nil, err
Expand Down
23 changes: 18 additions & 5 deletions crypto/secp256k1.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"io"

pb "github.com/libp2p/go-libp2p-core/crypto/pb"
"github.com/libp2p/go-libp2p-core/internal/catch"

"github.com/btcsuite/btcd/btcec"
"github.com/minio/sha256-simd"
Expand All @@ -28,17 +29,19 @@ func GenerateSecp256k1Key(src io.Reader) (PrivKey, PubKey, error) {
}

// UnmarshalSecp256k1PrivateKey returns a private key from bytes
func UnmarshalSecp256k1PrivateKey(data []byte) (PrivKey, error) {
func UnmarshalSecp256k1PrivateKey(data []byte) (k PrivKey, err error) {
if len(data) != btcec.PrivKeyBytesLen {
return nil, fmt.Errorf("expected secp256k1 data size to be %d", btcec.PrivKeyBytesLen)
}
defer func() { catch.HandlePanic(recover(), &err, "secp256k1 private-key unmarshal") }()

privk, _ := btcec.PrivKeyFromBytes(btcec.S256(), data)
return (*Secp256k1PrivateKey)(privk), nil
}

// UnmarshalSecp256k1PublicKey returns a public key from bytes
func UnmarshalSecp256k1PublicKey(data []byte) (PubKey, error) {
func UnmarshalSecp256k1PublicKey(data []byte) (_k PubKey, err error) {
defer func() { catch.HandlePanic(recover(), &err, "secp256k1 public-key unmarshal") }()
k, err := btcec.ParsePubKey(data, btcec.S256())
if err != nil {
return nil, err
Expand Down Expand Up @@ -68,7 +71,8 @@ func (k *Secp256k1PrivateKey) Equals(o Key) bool {
}

// Sign returns a signature from input data
func (k *Secp256k1PrivateKey) Sign(data []byte) ([]byte, error) {
func (k *Secp256k1PrivateKey) Sign(data []byte) (_sig []byte, err error) {
defer func() { catch.HandlePanic(recover(), &err, "secp256k1 signing") }()
hash := sha256.Sum256(data)
sig, err := (*btcec.PrivateKey)(k).Sign(hash[:])
if err != nil {
Expand All @@ -89,7 +93,8 @@ func (k *Secp256k1PublicKey) Type() pb.KeyType {
}

// Raw returns the bytes of the key
func (k *Secp256k1PublicKey) Raw() ([]byte, error) {
func (k *Secp256k1PublicKey) Raw() (res []byte, err error) {
defer func() { catch.HandlePanic(recover(), &err, "secp256k1 public key marshaling") }()
return (*btcec.PublicKey)(k).SerializeCompressed(), nil
}

Expand All @@ -104,7 +109,15 @@ func (k *Secp256k1PublicKey) Equals(o Key) bool {
}

// Verify compares a signature against the input data
func (k *Secp256k1PublicKey) Verify(data []byte, sigStr []byte) (bool, error) {
func (k *Secp256k1PublicKey) Verify(data []byte, sigStr []byte) (success bool, err error) {
defer func() {
catch.HandlePanic(recover(), &err, "secp256k1 signature verification")

// To be extra safe.
if err != nil {
success = false
}
}()
sig, err := btcec.ParseDERSignature(sigStr, btcec.S256())
if err != nil {
return false, err
Expand Down
18 changes: 18 additions & 0 deletions internal/catch/catch.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package catch

import (
"fmt"
"io"
"os"
"runtime/debug"
)

var panicWriter io.Writer = os.Stderr

// HandlePanic handles and logs panics.
func HandlePanic(rerr interface{}, err *error, where string) {
if rerr != nil {
fmt.Fprintf(panicWriter, "caught panic: %s\n%s\n", rerr, debug.Stack())
*err = fmt.Errorf("panic in %s: %s", where, rerr)
}
}
8 changes: 6 additions & 2 deletions peer/addrinfo_serde.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package peer
import (
"encoding/json"

"github.com/libp2p/go-libp2p-core/internal/catch"
ma "github.com/multiformats/go-multiaddr"
)

Expand All @@ -12,7 +13,9 @@ type addrInfoJson struct {
Addrs []string
}

func (pi AddrInfo) MarshalJSON() ([]byte, error) {
func (pi AddrInfo) MarshalJSON() (res []byte, err error) {
defer func() { catch.HandlePanic(recover(), &err, "libp2p addr info marshal") }()

addrs := make([]string, len(pi.Addrs))
for i, addr := range pi.Addrs {
addrs[i] = addr.String()
Expand All @@ -23,7 +26,8 @@ func (pi AddrInfo) MarshalJSON() ([]byte, error) {
})
}

func (pi *AddrInfo) UnmarshalJSON(b []byte) error {
func (pi *AddrInfo) UnmarshalJSON(b []byte) (err error) {
defer func() { catch.HandlePanic(recover(), &err, "libp2p addr info unmarshal") }()
var data addrInfoJson
if err := json.Unmarshal(b, &data); err != nil {
return err
Expand Down
Loading

0 comments on commit ef3e50c

Please sign in to comment.