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

feat: catch panics in TLS negotiation #111

Merged
merged 1 commit into from
Apr 19, 2022
Merged

feat: catch panics in TLS negotiation #111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion crypto.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"errors"
"fmt"
"math/big"
"os"
"runtime/debug"
"time"

"golang.org/x/sys/cpu"
Expand Down Expand Up @@ -72,7 +74,15 @@ func (i *Identity) ConfigForPeer(remote peer.ID) (*tls.Config, <-chan ic.PubKey)
conf := i.config.Clone()
// We're using InsecureSkipVerify, so the verifiedChains parameter will always be empty.
// We need to parse the certificates ourselves from the raw certs.
conf.VerifyPeerCertificate = func(rawCerts [][]byte, _ [][]*x509.Certificate) error {
conf.VerifyPeerCertificate = func(rawCerts [][]byte, _ [][]*x509.Certificate) (err error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed, since we have the panic handler in handshake, is it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • This way we don't panic through TLS (not sure what that might do).
  • E.g., I don't trust that TLS won't run this on another goroutine.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

crypto/tls doesn't, but we can leave it here to be extra paranoid.

defer func() {
if rerr := recover(); rerr != nil {
fmt.Fprintf(os.Stderr, "panic when processing peer certificate in TLS handshake: %s\n%s\n", rerr, debug.Stack())
err = fmt.Errorf("panic when processing peer certificate in TLS handshake: %s", rerr)

}
}()

defer close(keyCh)

chain := make([]*x509.Certificate, len(rawCerts))
Expand Down
13 changes: 12 additions & 1 deletion transport.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import (
"context"
"crypto/tls"
"errors"
"fmt"
"net"
"os"
"runtime/debug"

ci "github.com/libp2p/go-libp2p-core/crypto"
"github.com/libp2p/go-libp2p-core/peer"
Expand Down Expand Up @@ -70,7 +73,15 @@ func (t *Transport) SecureOutbound(ctx context.Context, insecure net.Conn, p pee
return cs, err
}

func (t *Transport) handshake(ctx context.Context, tlsConn *tls.Conn, keyCh <-chan ci.PubKey) (sec.SecureConn, error) {
func (t *Transport) handshake(ctx context.Context, tlsConn *tls.Conn, keyCh <-chan ci.PubKey) (_sconn sec.SecureConn, err error) {
defer func() {
if rerr := recover(); rerr != nil {
fmt.Fprintf(os.Stderr, "panic in TLS handshake: %s\n%s\n", rerr, debug.Stack())
err = fmt.Errorf("panic in TLS handshake: %s", rerr)

}
}()

if err := tlsConn.HandshakeContext(ctx); err != nil {
return nil, err
}
Expand Down