Skip to content

Commit e8f35e1

Browse files
authored
fix(quic): avoid panic on post-handshake cert validation
Pull-Request: #6525.
1 parent 93c5059 commit e8f35e1

5 files changed

Lines changed: 63 additions & 10 deletions

File tree

Cargo.lock

Lines changed: 20 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

transports/quic/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33
- Raise MSRV to 1.88.0.
44
See [PR 6273](https://github.com/libp2p/rust-libp2p/pull/6273).
55

6+
## 0.13.1
7+
- Handle certificate validation failures in post-handshake upgrade path instead of panicking.
8+
See [GHSA-5hq8-qhww-jm7q](https://github.com/libp2p/rust-libp2p/security/advisories/GHSA-5hq8-qhww-jm7q)
9+
610
## 0.13.0
711

812
- Remove `async-std` support.

transports/quic/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ libp2p-core = { workspace = true }
1616
libp2p-tls = { workspace = true }
1717
libp2p-identity = { workspace = true }
1818
quinn = { version = "0.11.9", default-features = false, features = ["rustls", "futures-io"] }
19+
quinn-proto = { version = "0.11" }
1920
rand = { workspace = true }
2021
rustls = { version = "0.23.40", default-features = false }
2122
thiserror = { workspace = true }

transports/quic/src/connection/connecting.rs

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ use futures::{
3333
use futures_timer::Delay;
3434
use libp2p_identity::PeerId;
3535
use quinn::rustls::pki_types::CertificateDer;
36+
use quinn_proto::TransportError;
3637

3738
use crate::{Connection, ConnectionError, Error};
3839

@@ -51,20 +52,29 @@ impl Connecting {
5152
}
5253

5354
impl Connecting {
54-
/// Returns the address of the node we're connected to.
55-
/// Panics if the connection is still handshaking.
56-
fn remote_peer_id(connection: &quinn::Connection) -> PeerId {
55+
fn remote_peer_id(connection: &quinn::Connection) -> Result<PeerId, Error> {
56+
fn transport_err(reason: &str) -> Error {
57+
Error::Connection(ConnectionError(quinn::ConnectionError::TransportError(
58+
TransportError {
59+
code: quinn::TransportErrorCode::PROTOCOL_VIOLATION,
60+
frame: None,
61+
reason: reason.to_string(),
62+
},
63+
)))
64+
}
65+
5766
let identity = connection
5867
.peer_identity()
59-
.expect("connection got identity because it passed TLS handshake; qed");
60-
let certificates: Box<Vec<CertificateDer>> =
61-
identity.downcast().expect("we rely on rustls feature; qed");
68+
.ok_or_else(|| transport_err("No crypto identity in quinn's Connection"))?;
69+
let certificates: Box<Vec<CertificateDer>> = identity
70+
.downcast()
71+
.map_err(|_| transport_err("Could not downcast identity"))?;
6272
let end_entity = certificates
6373
.first()
64-
.expect("there should be exactly one certificate; qed");
74+
.ok_or_else(|| transport_err("No certificate found"))?;
6575
let p2p_cert = libp2p_tls::certificate::parse(end_entity)
66-
.expect("the certificate was validated during TLS handshake; qed");
67-
p2p_cert.peer_id()
76+
.map_err(|_| transport_err("Could not parse certificate"))?;
77+
Ok(p2p_cert.peer_id())
6878
}
6979
}
7080

@@ -77,7 +87,7 @@ impl Future for Connecting {
7787
Either::Left((connection, _)) => connection.map_err(ConnectionError)?,
7888
};
7989

80-
let peer_id = Self::remote_peer_id(&connection);
90+
let peer_id = Self::remote_peer_id(&connection)?;
8191
let muxer = Connection::new(connection);
8292
Poll::Ready(Ok((peer_id, muxer)))
8393
}

transports/tls/src/certificate.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,24 @@ mod tests {
581581
assert!(cert.signature_scheme().is_err());
582582
}
583583

584+
#[test]
585+
fn parse_rejects_expired_certificate() {
586+
// Regression for GHSA-5hq8-qhww-jm7q: expired cert must error, not panic.
587+
let identity = identity::Keypair::generate_ed25519();
588+
let signing = rcgen::KeyPair::generate_for(P2P_SIGNATURE_ALGORITHM).unwrap();
589+
590+
let mut params = rcgen::CertificateParams::default();
591+
params.distinguished_name = rcgen::DistinguishedName::new();
592+
params.not_before = rcgen::date_time_ymd(1970, 1, 1);
593+
params.not_after = rcgen::date_time_ymd(1975, 1, 1);
594+
params
595+
.custom_extensions
596+
.push(make_libp2p_extension(&identity, &signing).unwrap());
597+
let cert: rustls::pki_types::CertificateDer = params.self_signed(&signing).unwrap().into();
598+
599+
assert!(parse(&cert).is_err());
600+
}
601+
584602
#[test]
585603
fn can_parse_certificate_with_ed25519_keypair() {
586604
let certificate = rustls::pki_types::CertificateDer::from(hex!("308201773082011ea003020102020900f5bd0debaa597f52300a06082a8648ce3d04030230003020170d3735303130313030303030305a180f34303936303130313030303030305a30003059301306072a8648ce3d020106082a8648ce3d030107034200046bf9871220d71dcb3483ecdfcbfcc7c103f8509d0974b3c18ab1f1be1302d643103a08f7a7722c1b247ba3876fe2c59e26526f479d7718a85202ddbe47562358a37f307d307b060a2b0601040183a25a01010101ff046a30680424080112207fda21856709c5ae12fd6e8450623f15f11955d384212b89f56e7e136d2e17280440aaa6bffabe91b6f30c35e3aa4f94b1188fed96b0ffdd393f4c58c1c047854120e674ce64c788406d1c2c4b116581fd7411b309881c3c7f20b46e54c7e6fe7f0f300a06082a8648ce3d040302034700304402207d1a1dbd2bda235ff2ec87daf006f9b04ba076a5a5530180cd9c2e8f6399e09d0220458527178c7e77024601dbb1b256593e9b96d961b96349d1f560114f61a87595").to_vec());

0 commit comments

Comments
 (0)