-
Notifications
You must be signed in to change notification settings - Fork 267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: quic integration #1466
Draft
rach-id
wants to merge
56
commits into
v0.34.x-celestia
Choose a base branch
from
quic-initial-changes
base: v0.34.x-celestia
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
WIP: quic integration #1466
Changes from 23 commits
Commits
Show all changes
56 commits
Select commit
Hold shift + click to select a range
8aa840d
initial quic prototype
rach-id 89de565
working connection but not receiving data
rach-id 6851c78
connected but not sending/receiving
rach-id 74930f0
working connection and some messages pass but still slow and panicing
rach-id 78e6326
remove private key from node address
rach-id 5ea01bc
rename to start receiving
rach-id 07e9a09
synchronous send
rach-id 7e7098c
cleanup
rach-id fd0577f
use some kind of status
rach-id bde075b
fix send
rach-id c3351aa
min tls version and 0rtt comment
rach-id 6f401e9
reduce number of streams per connection
rach-id b14191e
tls using rsa and freshly generated keys
rach-id ee6ff45
add peer reconnection todo
rach-id d93bef9
close connection after 5 sec
rach-id fcc4a13
add TLS support
rach-id e53f0f9
todos
rach-id c8f39ed
starting to fix e2e
rach-id 196919f
running e2e
rach-id 00de160
Merge branch 'v0.34.x-celestia' into quic-initial-changes
rach-id 0a5aebf
chore: merge v0.34.x
rach-id 4ace628
chore: increase quic params
rach-id 21d8e38
fix: synchronisation when adding a new stream
rach-id 50a74f5
to be reverted: remove peers filter
rach-id 1307d92
to be reverted: more synchronised access to streams
rach-id 060595a
to be reverted: more logs
rach-id c8e964c
to be reverted: more logs
rach-id 30d09ac
fix: use the p2p packet and proto reader/writer instead of raw bytes
rach-id fa7a8d6
chore: correct logs
rach-id 574cbb8
to be reverted: remove filtering of peers
rach-id 0b5ff4e
to be reverted: just some logging
rach-id e976795
chore: update the idle timeout and keep alive period
rach-id 2a7de97
chore: revert previously commented code
rach-id f57f4bd
chore: trying for better peer management
rach-id 7bcf6d6
chore: not print congestion status
rach-id abe9bb0
chore: remove unnecessaary todos
rach-id 68f7321
chore: cleanup
rach-id 5ff281f
chore: use fmt instead of logger
rach-id 257f9f8
chore: increase cert validity period
rach-id 267a6c7
chore: change the peer listen address to be the remote address
rach-id 4da09a0
chore: pex logs
rach-id f8b5204
chore: ;use multiple channels to send
rach-id 5570d4a
chore: ;use multiple channels to send
rach-id 71eadd1
chore: unnecessary fmt
rach-id 0a06082
chore: use rand to choose stream
rach-id 46c8386
chore: use rand to choose stream
rach-id dd2de41
chore: rmove fmt
rach-id 9d8adf6
chore: increase number of streams to 50
rach-id 5e234c0
chore: create streams for other reactors too
rach-id c53eff2
chore: inmitlaize correctly satreams
rach-id b25b789
chore: create streams sync
rach-id 2fa96bc
crazy: send in a go routine and return immediatly
rach-id 33d1db4
chore: revert crazy and uses deterministic way of choosing from multi…
rach-id ffe88ef
chore: increase max block size
rach-id 7c39591
chore(starting to degug): ignore peer has no state panic
rach-id 7635b86
chore: crank up some params
rach-id File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,209 @@ | ||
package p2p | ||
|
||
import ( | ||
"crypto/ecdsa" | ||
"crypto/elliptic" | ||
"crypto/rand" | ||
"crypto/tls" | ||
"crypto/x509" | ||
"crypto/x509/pkix" | ||
"encoding/asn1" | ||
"errors" | ||
"fmt" | ||
"github.com/gogo/protobuf/proto" | ||
"github.com/tendermint/tendermint/crypto" | ||
"github.com/tendermint/tendermint/crypto/encoding" | ||
crypto2 "github.com/tendermint/tendermint/proto/tendermint/crypto" | ||
"math/big" | ||
"time" | ||
) | ||
|
||
// TODO(rach-id): mention this code is adapted from libp2p p2p/security/tls/crypto.go | ||
|
||
const certValidityPeriod = 24 * time.Hour | ||
const certificatePrefix = "tendermint-tls:" | ||
|
||
// TODO(rach-id): update the OID prefix to reflect Celestia/Tendermint | ||
var extensionPrefix = []int{1, 3, 6, 1, 4, 1, 53594} | ||
|
||
// getPrefixedExtensionID returns an Object Identifier | ||
// that can be used in x509 Certificates. | ||
func getPrefixedExtensionID(suffix []int) []int { | ||
return append(extensionPrefix, suffix...) | ||
} | ||
|
||
var extensionID = getPrefixedExtensionID([]int{1, 1}) | ||
var extensionCritical bool // so we can mark the extension critical in tests | ||
|
||
// extensionIDEqual compares two extension IDs. | ||
func extensionIDEqual(a, b []int) bool { | ||
if len(a) != len(b) { | ||
return false | ||
} | ||
for i := range a { | ||
if a[i] != b[i] { | ||
return false | ||
} | ||
} | ||
return true | ||
} | ||
|
||
type signedKey struct { | ||
PubKey []byte | ||
Signature []byte | ||
} | ||
|
||
// NewTLSConfig creates a new TLS configuration | ||
func NewTLSConfig(privKey crypto.PrivKey) (*tls.Config, error) { | ||
template, err := certTemplate() | ||
if err != nil { | ||
return nil, err | ||
} | ||
cert, err := keyToCertificate(privKey, template) | ||
if err != nil { | ||
return nil, err | ||
} | ||
return &tls.Config{ | ||
MinVersion: tls.VersionTLS13, | ||
InsecureSkipVerify: true, // This is not insecure here. We will verify the cert chain ourselves. | ||
ClientAuth: tls.RequireAnyClientCert, | ||
Certificates: []tls.Certificate{*cert}, | ||
VerifyPeerCertificate: func(_ [][]byte, _ [][]*x509.Certificate) error { | ||
panic("tls config not specialized for peer") | ||
}, | ||
}, nil | ||
} | ||
|
||
// VerifyCertificate verifies the certificate chain and extract the remote's public key. | ||
func VerifyCertificate(cert *x509.Certificate) (crypto.PubKey, error) { | ||
pool := x509.NewCertPool() | ||
pool.AddCert(cert) | ||
var found bool | ||
var keyExt pkix.Extension | ||
// find the tendermint key extension, skipping all unknown extensions | ||
for _, ext := range cert.Extensions { | ||
if extensionIDEqual(ext.Id, extensionID) { | ||
keyExt = ext | ||
found = true | ||
for i, oident := range cert.UnhandledCriticalExtensions { | ||
if oident.Equal(ext.Id) { | ||
// delete the extension from UnhandledCriticalExtensions | ||
cert.UnhandledCriticalExtensions = append(cert.UnhandledCriticalExtensions[:i], cert.UnhandledCriticalExtensions[i+1:]...) | ||
break | ||
} | ||
} | ||
break | ||
} | ||
} | ||
if !found { | ||
return nil, errors.New("expected certificate to contain the key extension") | ||
} | ||
if _, err := cert.Verify(x509.VerifyOptions{Roots: pool}); err != nil { | ||
// If we return an x509 error here, it will be sent on the wire. | ||
// Wrap the error to avoid that. | ||
return nil, fmt.Errorf("certificate verification failed: %s", err) | ||
} | ||
|
||
var sk signedKey | ||
if _, err := asn1.Unmarshal(keyExt.Value, &sk); err != nil { | ||
return nil, fmt.Errorf("unmarshalling signed certificate failed: %s", err) | ||
} | ||
protoPubKey := crypto2.PublicKey{} | ||
err := proto.Unmarshal(sk.PubKey, &protoPubKey) | ||
if err != nil { | ||
return nil, fmt.Errorf("unmarshalling public key failed: %s", err) | ||
} | ||
certKeyPub, err := x509.MarshalPKIXPublicKey(cert.PublicKey) | ||
if err != nil { | ||
return nil, err | ||
} | ||
pubKey, err := encoding.PubKeyFromProto(protoPubKey) | ||
valid := pubKey.VerifySignature(append([]byte(certificatePrefix), certKeyPub...), sk.Signature) | ||
if err != nil { | ||
return nil, fmt.Errorf("signature verification failed: %s", err) | ||
} | ||
if !valid { | ||
return nil, errors.New("signature invalid") | ||
} | ||
return pubKey, nil | ||
} | ||
|
||
// GenerateSignedExtension uses the provided private key to sign the public key, and returns the | ||
// signature within a pkix.Extension. | ||
// This extension is included in a certificate to cryptographically tie it to the libp2p private key. | ||
func GenerateSignedExtension(nodePrivateKey crypto.PrivKey, certificatePublicKey *ecdsa.PublicKey) (pkix.Extension, error) { | ||
protoPubKey, err := encoding.PubKeyToProto(nodePrivateKey.PubKey()) | ||
if err != nil { | ||
return pkix.Extension{}, err | ||
} | ||
keyBytes, err := proto.Marshal(&protoPubKey) | ||
if err != nil { | ||
return pkix.Extension{}, err | ||
} | ||
certKeyPub, err := x509.MarshalPKIXPublicKey(certificatePublicKey) | ||
if err != nil { | ||
return pkix.Extension{}, err | ||
} | ||
signature, err := nodePrivateKey.Sign(append([]byte(certificatePrefix), certKeyPub...)) | ||
if err != nil { | ||
return pkix.Extension{}, err | ||
} | ||
value, err := asn1.Marshal(signedKey{ | ||
PubKey: keyBytes, | ||
Signature: signature, | ||
}) | ||
if err != nil { | ||
return pkix.Extension{}, err | ||
} | ||
|
||
return pkix.Extension{Id: extensionID, Critical: extensionCritical, Value: value}, nil | ||
} | ||
|
||
// keyToCertificate generates a new ECDSA private key and corresponding x509 certificate. | ||
// The certificate includes an extension that cryptographically ties it to the provided libp2p | ||
// private key to authenticate TLS connections. | ||
func keyToCertificate(sk crypto.PrivKey, certTmpl *x509.Certificate) (*tls.Certificate, error) { | ||
certKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
// after calling CreateCertificate, these will end up in Certificate.Extensions | ||
extension, err := GenerateSignedExtension(sk, &certKey.PublicKey) | ||
if err != nil { | ||
return nil, err | ||
} | ||
certTmpl.ExtraExtensions = append(certTmpl.ExtraExtensions, extension) | ||
|
||
certDER, err := x509.CreateCertificate(rand.Reader, certTmpl, certTmpl, certKey.Public(), certKey) | ||
if err != nil { | ||
return nil, err | ||
} | ||
return &tls.Certificate{ | ||
Certificate: [][]byte{certDER}, | ||
PrivateKey: certKey, | ||
}, nil | ||
} | ||
|
||
// certTemplate returns the template for generating an Identity's TLS certificates. | ||
func certTemplate() (*x509.Certificate, error) { | ||
bigNum := big.NewInt(1 << 62) | ||
sn, err := rand.Int(rand.Reader, bigNum) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
subjectSN, err := rand.Int(rand.Reader, bigNum) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
return &x509.Certificate{ | ||
SerialNumber: sn, | ||
NotBefore: time.Now().Add(-time.Hour), | ||
NotAfter: time.Now().Add(certValidityPeriod), | ||
// According to RFC 3280, the issuer field must be set, | ||
// see https://datatracker.ietf.org/doc/html/rfc3280#section-4.1.2.4. | ||
Subject: pkix.Name{SerialNumber: subjectSN.String()}, | ||
}, nil | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Check failure
Code scanning / CodeQL
Disabled TLS certificate check High
Copilot Autofix AI 19 days ago
To fix the problem, we need to ensure that TLS certificate verification is enabled and properly implemented. This involves:
InsecureSkipVerify: true
setting.VerifyPeerCertificate
function to perform the necessary certificate chain verification.The changes will be made in the
NewTLSConfig
function in thep2p/certificate.go
file. We will:InsecureSkipVerify: true
line.VerifyPeerCertificate
function to verify the certificate chain.