Skip to content

Commit

Permalink
don't sign X.509 certs
Browse files Browse the repository at this point in the history
Nodes currently don't verify X.509 self-signed certificates
because peer authentication is done via TLS 1.3 CertificateVerify.
Thus, encodes an invalid signature in the X.509 certificate instead.
  • Loading branch information
riptl committed Nov 22, 2023
1 parent ef4dea3 commit 468aea6
Show file tree
Hide file tree
Showing 19 changed files with 99 additions and 195 deletions.
26 changes: 0 additions & 26 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,6 @@ rand = "0.8.5"
rand_chacha = "0.3.1"
raptorq = "1.7.0"
rayon = "1.8.0"
rcgen = "0.10.0"
reed-solomon-erasure = "6.0.0"
regex = "1.10.2"
reqwest = { version = "0.11.22", default-features = false }
Expand Down
7 changes: 2 additions & 5 deletions client/src/connection_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,7 @@ impl ConnectionCache {
config.update_client_endpoint(client_endpoint);
}
if let Some(cert_info) = cert_info {
config
.update_client_certificate(cert_info.0, cert_info.1)
.unwrap();
config.update_client_certificate(cert_info.0, cert_info.1);
}
if let Some(stake_info) = stake_info {
config.set_staked_nodes(stake_info.0, stake_info.1);
Expand Down Expand Up @@ -240,7 +238,7 @@ mod tests {
#[test]
fn test_connection_with_specified_client_endpoint() {
// Start a response receiver:
let (response_recv_socket, response_recv_exit, keypair2, response_recv_ip) = server_args();
let (response_recv_socket, response_recv_exit, keypair2, _) = server_args();
let (sender2, _receiver2) = unbounded();

let staked_nodes = Arc::new(RwLock::new(StakedNodes::default()));
Expand All @@ -249,7 +247,6 @@ mod tests {
"quic_streamer_test",
response_recv_socket,
&keypair2,
response_recv_ip,
sender2,
response_recv_exit.clone(),
1,
Expand Down
1 change: 0 additions & 1 deletion connection-cache/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ indicatif = { workspace = true, optional = true }
log = { workspace = true }
rand = { workspace = true }
rayon = { workspace = true }
rcgen = { workspace = true }
solana-measure = { workspace = true }
solana-metrics = { workspace = true }
solana-sdk = { workspace = true }
Expand Down
3 changes: 0 additions & 3 deletions connection-cache/src/connection_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,9 +406,6 @@ pub enum ConnectionPoolError {

#[derive(Error, Debug)]
pub enum ClientError {
#[error("Certificate error: {0}")]
CertificateError(#[from] rcgen::RcgenError),

#[error("IO error: {0:?}")]
IoError(#[from] std::io::Error),
}
Expand Down
1 change: 0 additions & 1 deletion core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ quinn = { workspace = true }
rand = { workspace = true }
rand_chacha = { workspace = true }
rayon = { workspace = true }
rcgen = { workspace = true }
rolling-file = { workspace = true }
rustls = { workspace = true }
serde = { workspace = true }
Expand Down
8 changes: 2 additions & 6 deletions core/src/repair/quic_endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use {
EndpointConfig, IdleTimeout, ReadError, ReadToEndError, RecvStream, SendStream,
ServerConfig, TokioRuntime, TransportConfig, VarInt, WriteError,
},
rcgen::RcgenError,
rustls::{Certificate, PrivateKey},
serde_bytes::ByteBuf,
solana_quic_client::nonblocking::quic_client::SkipServerVerification,
Expand Down Expand Up @@ -88,8 +87,6 @@ pub struct RemoteRequest {
#[derive(Error, Debug)]
#[allow(clippy::enum_variant_names)]
pub(crate) enum Error {
#[error(transparent)]
CertificateError(#[from] RcgenError),
#[error("Channel Send Error")]
ChannelSendError,
#[error(transparent)]
Expand Down Expand Up @@ -123,11 +120,11 @@ pub(crate) fn new_quic_endpoint(
runtime: &tokio::runtime::Handle,
keypair: &Keypair,
socket: UdpSocket,
address: IpAddr,
_address: IpAddr,
remote_request_sender: Sender<RemoteRequest>,
bank_forks: Arc<RwLock<BankForks>>,
) -> Result<(Endpoint, AsyncSender<LocalRequest>, AsyncTryJoinHandle), Error> {
let (cert, key) = new_self_signed_tls_certificate(keypair, address)?;
let (cert, key) = new_self_signed_tls_certificate(keypair);
let server_config = new_server_config(cert.clone(), key.clone())?;
let client_config = new_client_config(cert, key)?;
let mut endpoint = {
Expand Down Expand Up @@ -792,7 +789,6 @@ async fn report_metrics_task(name: &'static str, stats: Arc<RepairQuicStats>) {

fn record_error(err: &Error, stats: &RepairQuicStats) {
match err {
Error::CertificateError(_) => (),
Error::ChannelSendError => (),
Error::ConnectError(ConnectError::EndpointStopping) => {
add_metric!(stats.connect_error_other)
Expand Down
12 changes: 1 addition & 11 deletions core/src/tpu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use {
},
bytes::Bytes,
crossbeam_channel::{unbounded, Receiver},
solana_client::connection_cache::{ConnectionCache, Protocol},
solana_client::connection_cache::ConnectionCache,
solana_gossip::cluster_info::ClusterInfo,
solana_ledger::{
blockstore::Blockstore, blockstore_processor::TransactionStatusSender,
Expand Down Expand Up @@ -152,11 +152,6 @@ impl Tpu {
"quic_streamer_tpu",
transactions_quic_sockets,
keypair,
cluster_info
.my_contact_info()
.tpu(Protocol::QUIC)
.expect("Operator must spin up node with valid (QUIC) TPU address")
.ip(),
packet_sender,
exit.clone(),
MAX_QUIC_CONNECTIONS_PER_PEER,
Expand All @@ -172,11 +167,6 @@ impl Tpu {
"quic_streamer_tpu_forwards",
transactions_forwards_quic_sockets,
keypair,
cluster_info
.my_contact_info()
.tpu_forwards(Protocol::QUIC)
.expect("Operator must spin up node with valid (QUIC) TPU-forwards address")
.ip(),
forwarded_packet_sender,
exit.clone(),
MAX_QUIC_CONNECTIONS_PER_PEER,
Expand Down
26 changes: 0 additions & 26 deletions programs/sbf/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion quic-client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ lazy_static = { workspace = true }
log = { workspace = true }
quinn = { workspace = true }
quinn-proto = { workspace = true }
rcgen = { workspace = true }
rustls = { workspace = true, features = ["dangerous_configuration"] }
solana-connection-cache = { workspace = true }
solana-measure = { workspace = true }
Expand Down
22 changes: 6 additions & 16 deletions quic-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ use {
quic_client::QuicClientConnection as BlockingQuicClientConnection,
},
quinn::Endpoint,
rcgen::RcgenError,
solana_connection_cache::{
connection_cache::{
BaseClientConnection, ClientError, ConnectionCache, ConnectionManager, ConnectionPool,
Expand All @@ -33,17 +32,14 @@ use {
tls_certificates::new_self_signed_tls_certificate,
},
std::{
net::{IpAddr, Ipv4Addr, SocketAddr},
net::{IpAddr, SocketAddr},
sync::{Arc, RwLock},
},
thiserror::Error,
};

#[derive(Error, Debug)]
pub enum QuicClientError {
#[error("Certificate error: {0}")]
CertificateError(#[from] RcgenError),
}
pub enum QuicClientError {}

pub struct QuicPool {
connections: Vec<Arc<Quic>>,
Expand Down Expand Up @@ -97,8 +93,7 @@ pub struct QuicConfig {

impl NewConnectionConfig for QuicConfig {
fn new() -> Result<Self, ClientError> {
let (cert, priv_key) =
new_self_signed_tls_certificate(&Keypair::new(), IpAddr::V4(Ipv4Addr::UNSPECIFIED))?;
let (cert, priv_key) = new_self_signed_tls_certificate(&Keypair::new());
Ok(Self {
client_certificate: Arc::new(QuicClientCertificate {
certificate: cert,
Expand Down Expand Up @@ -137,17 +132,12 @@ impl QuicConfig {
compute_max_allowed_uni_streams(client_type, stake, total_stake)
}

pub fn update_client_certificate(
&mut self,
keypair: &Keypair,
ipaddr: IpAddr,
) -> Result<(), RcgenError> {
let (cert, priv_key) = new_self_signed_tls_certificate(keypair, ipaddr)?;
pub fn update_client_certificate(&mut self, keypair: &Keypair, _ipaddr: IpAddr) {
let (cert, priv_key) = new_self_signed_tls_certificate(keypair);
self.client_certificate = Arc::new(QuicClientCertificate {
certificate: cert,
key: priv_key,
});
Ok(())
}

pub fn set_staked_nodes(
Expand Down Expand Up @@ -230,7 +220,7 @@ pub fn new_quic_connection_cache(
connection_pool_size: usize,
) -> Result<QuicConnectionCache, ClientError> {
let mut config = QuicConfig::new()?;
config.update_client_certificate(keypair, ipaddr)?;
config.update_client_certificate(keypair, ipaddr);
config.set_staked_nodes(staked_nodes, &keypair.pubkey());
let connection_manager = QuicConnectionManager::new_with_connection_config(config);
ConnectionCache::new(name, connection_manager, connection_pool_size)
Expand Down
4 changes: 1 addition & 3 deletions quic-client/src/nonblocking/quic_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,7 @@ impl QuicLazyInitializedEndpoint {

impl Default for QuicLazyInitializedEndpoint {
fn default() -> Self {
let (cert, priv_key) =
new_self_signed_tls_certificate(&Keypair::new(), IpAddr::V4(Ipv4Addr::UNSPECIFIED))
.expect("Failed to create QUIC client certificate");
let (cert, priv_key) = new_self_signed_tls_certificate(&Keypair::new());
Self::new(
Arc::new(QuicClientCertificate {
certificate: cert,
Expand Down

0 comments on commit 468aea6

Please sign in to comment.