From 798962f48ad3a39ed86df424f9ce0ddefdcf862e Mon Sep 17 00:00:00 2001 From: Grzegorz Prusak Date: Fri, 20 Sep 2024 22:59:31 +0200 Subject: [PATCH 1/3] slight refactor --- node/actors/executor/src/lib.rs | 9 +- .../network/src/{http => debug_page}/mod.rs | 194 +++++++++++------- .../src/{http => debug_page}/style.css | 0 node/actors/network/src/lib.rs | 2 +- node/libs/protobuf/src/lib.rs | 2 +- node/libs/protobuf/src/repr.rs | 5 + node/libs/utils/src/debug_page.rs | 40 ---- node/libs/utils/src/encode.rs | 10 - node/libs/utils/src/lib.rs | 1 - node/tools/src/bin/deployer.rs | 9 +- node/tools/src/bin/localnet_config.rs | 6 +- node/tools/src/config.rs | 185 ++++++++++------- node/tools/src/k8s.rs | 8 +- node/tools/src/lib.rs | 5 +- node/tools/src/main.rs | 13 +- node/tools/src/proto/mod.proto | 34 ++- node/tools/src/tests.rs | 34 ++- 17 files changed, 315 insertions(+), 242 deletions(-) rename node/actors/network/src/{http => debug_page}/mod.rs (64%) rename node/actors/network/src/{http => debug_page}/style.css (100%) delete mode 100644 node/libs/utils/src/debug_page.rs diff --git a/node/actors/executor/src/lib.rs b/node/actors/executor/src/lib.rs index 3cbed6e6..51f3a335 100644 --- a/node/actors/executor/src/lib.rs +++ b/node/actors/executor/src/lib.rs @@ -1,7 +1,6 @@ //! Library files for the executor. We have it separate from the binary so that we can use these files in the tools crate. use crate::io::Dispatcher; use anyhow::Context as _; -use network::http; pub use network::{gossip::attestation, RpcConfig}; use std::{ collections::{HashMap, HashSet}, @@ -63,7 +62,7 @@ pub struct Config { /// Http debug page configuration. /// If None, debug page is disabled - pub debug_page: Option, + pub debug_page: Option, /// How often to poll the database looking for the batch commitment. pub batch_poll_interval: time::Duration, @@ -144,12 +143,12 @@ impl Executor { net.register_metrics(); s.spawn(async { runner.run(ctx).await.context("Network stopped") }); - if let Some(debug_config) = self.config.debug_page { + if let Some(cfg) = self.config.debug_page { s.spawn(async { - http::DebugPageServer::new(debug_config, net) + network::debug_page::Server::new(cfg, net) .run(ctx) .await - .context("Http Server stopped") + .context("Debug page server stopped") }); } diff --git a/node/actors/network/src/http/mod.rs b/node/actors/network/src/debug_page/mod.rs similarity index 64% rename from node/actors/network/src/http/mod.rs rename to node/actors/network/src/debug_page/mod.rs index 748e027f..5f20b754 100644 --- a/node/actors/network/src/http/mod.rs +++ b/node/actors/network/src/debug_page/mod.rs @@ -24,56 +24,131 @@ use tokio_rustls::{ pki_types::{CertificateDer, PrivateKeyDer}, ServerConfig, }, + server::TlsStream, TlsAcceptor, }; use zksync_concurrency::{ctx, scope}; use zksync_consensus_crypto::TextFmt as _; -use zksync_consensus_utils::debug_page; const STYLE: &str = include_str!("style.css"); +/// TLS certificate chain with a private key. +#[derive(Debug, PartialEq)] +pub struct TlsConfig { + /// TLS certificate chain. + pub cert_chain: Vec>, + /// Private key for the leaf cert. + pub private_key: PrivateKeyDer<'static>, +} + +/// Credentials. +#[derive(PartialEq, Clone)] +pub struct Credentials { + /// User for debug page + pub user: String, + /// Password for debug page + /// TODO: it should be treated as a secret: zeroize, etc. + pub password: String, +} + +impl Credentials { + fn parse(value: String) -> anyhow::Result { + let [user, password] = value + .split(':') + .collect::>() + .try_into() + .ok() + .context("bad format")?; + Ok(Self { + user: user.to_string(), + password: password.to_string(), + }) + } +} + +impl std::fmt::Debug for Credentials { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Credentials").finish_non_exhaustive() + } +} + /// Http debug page configuration. #[derive(Debug, PartialEq)] -pub struct DebugPageConfig { +pub struct Config { /// Public Http address to listen incoming http requests. pub addr: SocketAddr, /// Debug page credentials. - pub credentials: Option, - /// Cert file path - pub certs: Vec>, - /// Key file path - pub private_key: PrivateKeyDer<'static>, + pub credentials: Option, + /// TLS certificate to terminate the connections with. + pub tls: Option, } /// Http Server for debug page. -pub struct DebugPageServer { - config: DebugPageConfig, +pub struct Server { + config: Config, network: Arc, } -impl DebugPageServer { +#[async_trait::async_trait] +trait Listener: 'static + Send { + type Stream: tokio::io::AsyncRead + tokio::io::AsyncWrite + Send + Unpin; + async fn accept(&mut self) -> anyhow::Result; +} + +#[async_trait::async_trait] +impl Listener for TcpListener { + type Stream = tokio::net::TcpStream; + async fn accept(&mut self) -> anyhow::Result { + Ok(TcpListener::accept(self).await?.0) + } +} + +#[async_trait::async_trait] +impl Listener for TlsListener { + type Stream = TlsStream; + async fn accept(&mut self) -> anyhow::Result { + Ok(TlsListener::accept(self).await?.0) + } +} + +impl Server { /// Creates a new Server - pub fn new(config: DebugPageConfig, network: Arc) -> DebugPageServer { - DebugPageServer { config, network } + pub fn new(config: Config, network: Arc) -> Self { + Self { config, network } } /// Runs the Server. pub async fn run(&self, ctx: &ctx::Ctx) -> anyhow::Result<()> { + let listener = TcpListener::bind(self.config.addr) + .await + .context("TcpListener::bind()")?; + if let Some(tls) = &self.config.tls { + let cfg = ServerConfig::builder() + .with_no_client_auth() + .with_single_cert(tls.cert_chain.clone(), tls.private_key.clone_key()) + .context("with_signle_cert()")?; + self.run_with_listener(ctx, TlsListener::new(Arc::new(cfg).into(), listener)) + .await + } else { + self.run_with_listener(ctx, listener).await + } + } + + async fn run_with_listener( + &self, + ctx: &ctx::Ctx, + mut listener: L, + ) -> anyhow::Result<()> { // Start a watcher to shut down the server whenever ctx gets cancelled let graceful = hyper_util::server::graceful::GracefulShutdown::new(); scope::run!(ctx, |ctx, s| async { - let mut listener = TlsListener::new( - self.tls_acceptor(), - TcpListener::bind(self.config.addr).await?, - ); - let http = http1::Builder::new(); // Start a loop to accept incoming connections while let Ok(res) = ctx.wait(listener.accept()).await { match res { - Ok((stream, _)) => { + Ok(stream) => { let io = TokioIo::new(stream); let conn = http.serve_connection(io, service_fn(|req| self.handle(req))); // watch this connection @@ -86,10 +161,6 @@ impl DebugPageServer { }); } Err(err) => { - if let Some(remote_addr) = err.peer_addr() { - tracing::error!("[client {remote_addr}] "); - } - tracing::error!("Error accepting connection: {}", err); continue; } @@ -106,46 +177,41 @@ impl DebugPageServer { request: Request, ) -> anyhow::Result>> { let mut response = Response::new(Full::default()); - match self.basic_authentication(request.headers()) { - Ok(_) => *response.body_mut() = self.serve(request), - Err(e) => { - *response.status_mut() = StatusCode::UNAUTHORIZED; - *response.body_mut() = Full::new(Bytes::from(e.to_string())); - let header_value = HeaderValue::from_str(r#"Basic realm="debug""#).unwrap(); - response - .headers_mut() - .insert(header::WWW_AUTHENTICATE, header_value); - } + if let Err(err) = self.authenticate(request.headers()) { + *response.status_mut() = StatusCode::UNAUTHORIZED; + *response.body_mut() = Full::new(Bytes::from(err.to_string())); + let header_value = HeaderValue::from_str(r#"Basic realm="debug""#).unwrap(); + response + .headers_mut() + .insert(header::WWW_AUTHENTICATE, header_value); } + *response.body_mut() = self.serve(request); Ok(response) } - fn basic_authentication(&self, headers: &HeaderMap) -> anyhow::Result<()> { - self.config - .credentials - .clone() - .map_or(Ok(()), |credentials| { - // The header value, if present, must be a valid UTF8 string - let header_value = headers - .get("Authorization") - .context("The 'Authorization' header was missing")? - .to_str() - .context("The 'Authorization' header was not a valid UTF8 string.")?; - let base64encoded_segment = header_value - .strip_prefix("Basic ") - .context("The authorization scheme was not 'Basic'.")?; - let decoded_bytes = base64::engine::general_purpose::STANDARD - .decode(base64encoded_segment) - .context("Failed to base64-decode 'Basic' credentials.")?; - let incoming_credentials = debug_page::Credentials::try_from( - String::from_utf8(decoded_bytes) - .context("The decoded credential string is not valid UTF8.")?, - )?; - if credentials != incoming_credentials { - anyhow::bail!("Invalid password.") - } - Ok(()) - }) + fn authenticate(&self, headers: &HeaderMap) -> anyhow::Result<()> { + let Some(want) = self.config.credentials.as_ref() else { + return Ok(()); + }; + + // The header value, if present, must be a valid UTF8 string + let header_value = headers + .get("Authorization") + .context("The 'Authorization' header was missing")? + .to_str() + .context("The 'Authorization' header was not a valid UTF8 string.")?; + let base64encoded_segment = header_value + .strip_prefix("Basic ") + .context("Unsupported authorization scheme.")?; + let decoded_bytes = base64::engine::general_purpose::STANDARD + .decode(base64encoded_segment) + .context("Failed to base64-decode 'Basic' credentials.")?; + let got = Credentials::parse( + String::from_utf8(decoded_bytes) + .context("The decoded credential string is not valid UTF8.")?, + )?; + anyhow::ensure!(want == &got, "Invalid credentials."); + Ok(()) } fn serve(&self, _request: Request) -> Full { @@ -262,16 +328,4 @@ impl DebugPageServer { format!("{}...{}", &key[..10], &key[len - 11..len]) }) } - - fn tls_acceptor(&self) -> TlsAcceptor { - let cert_der = self.config.certs.clone(); - let key_der = self.config.private_key.clone_key(); - Arc::new( - ServerConfig::builder() - .with_no_client_auth() - .with_single_cert(cert_der, key_der) - .unwrap(), - ) - .into() - } } diff --git a/node/actors/network/src/http/style.css b/node/actors/network/src/debug_page/style.css similarity index 100% rename from node/actors/network/src/http/style.css rename to node/actors/network/src/debug_page/style.css diff --git a/node/actors/network/src/lib.rs b/node/actors/network/src/lib.rs index 41e953b5..adbb156f 100644 --- a/node/actors/network/src/lib.rs +++ b/node/actors/network/src/lib.rs @@ -13,9 +13,9 @@ use zksync_consensus_utils::pipe::ActorPipe; mod config; pub mod consensus; +pub mod debug_page; mod frame; pub mod gossip; -pub mod http; pub mod io; mod metrics; mod mux; diff --git a/node/libs/protobuf/src/lib.rs b/node/libs/protobuf/src/lib.rs index 2ad12b50..7a15d363 100644 --- a/node/libs/protobuf/src/lib.rs +++ b/node/libs/protobuf/src/lib.rs @@ -6,7 +6,7 @@ pub mod build; pub mod proto; mod proto_fmt; pub mod repr; -pub use repr::{read_required_repr, ProtoRepr}; +pub use repr::{read_optional_repr, read_required_repr, ProtoRepr}; pub mod serde; mod std_conv; pub mod testonly; diff --git a/node/libs/protobuf/src/repr.rs b/node/libs/protobuf/src/repr.rs index e8659715..0d54e94f 100644 --- a/node/libs/protobuf/src/repr.rs +++ b/node/libs/protobuf/src/repr.rs @@ -19,6 +19,11 @@ pub fn read_required_repr(field: &Option

) -> anyhow::Result(field: &Option

) -> anyhow::Result> { + field.as_ref().map(ProtoRepr::read).transpose() +} + /// Encodes a proto message. /// Currently it outputs a canonical encoding, but `decode` accepts /// non-canonical encoding as well. diff --git a/node/libs/utils/src/debug_page.rs b/node/libs/utils/src/debug_page.rs deleted file mode 100644 index ac40f771..00000000 --- a/node/libs/utils/src/debug_page.rs +++ /dev/null @@ -1,40 +0,0 @@ -//! Http Server configuration structs -use anyhow::Context as _; -/// Debug Page credentials (user:password) -#[derive(PartialEq, Clone)] -pub struct Credentials { - /// User for debug page - pub user: String, - /// Password for debug page - pub password: String, -} - -impl TryFrom for Credentials { - type Error = anyhow::Error; - fn try_from(value: String) -> anyhow::Result { - let mut credentials = value.split(':'); - let user = credentials.next().context("Empty debug page credentials")?; - let password = credentials - .next() - .context("Invalid debug page credentials: expected '{user:password}'")?; - Ok(Self { - user: user.to_string(), - password: password.to_string(), - }) - } -} - -impl From for String { - fn from(val: Credentials) -> Self { - format!("{}:{}", val.user, val.password) - } -} - -impl std::fmt::Debug for Credentials { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("DebugCredentials") - .field("user", &"****") - .field("password", &"****") - .finish() - } -} diff --git a/node/libs/utils/src/encode.rs b/node/libs/utils/src/encode.rs index 395ccaea..e368559c 100644 --- a/node/libs/utils/src/encode.rs +++ b/node/libs/utils/src/encode.rs @@ -1,5 +1,4 @@ //! Utilities for testing encodings. -use crate::debug_page; use rand::{ distributions::{Alphanumeric, DistString, Distribution}, Rng, @@ -150,15 +149,6 @@ impl Distribution for EncodeDist { } } -impl Distribution for EncodeDist { - fn sample(&self, rng: &mut R) -> debug_page::Credentials { - debug_page::Credentials { - user: self.sample(rng), - password: self.sample(rng), - } - } -} - impl Distribution for EncodeDist { fn sample(&self, rng: &mut R) -> time::Duration { time::Duration::new(self.sample(rng), self.sample(rng)) diff --git a/node/libs/utils/src/lib.rs b/node/libs/utils/src/lib.rs index 664d37a1..71fc6af1 100644 --- a/node/libs/utils/src/lib.rs +++ b/node/libs/utils/src/lib.rs @@ -1,6 +1,5 @@ //! Crate that holds several small utilities and primitives. -pub mod debug_page; mod encode; pub mod enum_util; pub mod pipe; diff --git a/node/tools/src/bin/deployer.rs b/node/tools/src/bin/deployer.rs index 2b5333b8..b67c3fc8 100644 --- a/node/tools/src/bin/deployer.rs +++ b/node/tools/src/bin/deployer.rs @@ -5,7 +5,7 @@ use std::{ net::{Ipv4Addr, SocketAddr}, }; use zksync_consensus_roles::{node::SecretKey, validator}; -use zksync_consensus_tools::{k8s, k8s::ConsensusNode, AppConfig, NODES_PORT}; +use zksync_consensus_tools::{config, k8s, k8s::ConsensusNode}; /// Command line arguments. #[derive(Debug, Parser)] @@ -40,9 +40,10 @@ fn generate_consensus_nodes(nodes: usize, seed_nodes_amount: Option) -> V let mut cfgs: Vec = (0..nodes) .map(|i| ConsensusNode { id: format!("consensus-node-{i:0>2}"), - config: AppConfig { - server_addr: SocketAddr::new(Ipv4Addr::UNSPECIFIED.into(), NODES_PORT), - public_addr: SocketAddr::new(Ipv4Addr::UNSPECIFIED.into(), NODES_PORT).into(), + config: config::App { + server_addr: SocketAddr::new(Ipv4Addr::UNSPECIFIED.into(), config::NODES_PORT), + public_addr: SocketAddr::new(Ipv4Addr::UNSPECIFIED.into(), config::NODES_PORT) + .into(), rpc_addr: None, metrics_server_addr: None, genesis: setup.genesis.clone(), diff --git a/node/tools/src/bin/localnet_config.rs b/node/tools/src/bin/localnet_config.rs index ad5b19b9..197e90a1 100644 --- a/node/tools/src/bin/localnet_config.rs +++ b/node/tools/src/bin/localnet_config.rs @@ -10,7 +10,7 @@ use std::{ path::PathBuf, }; use zksync_consensus_roles::{node, validator}; -use zksync_consensus_tools::{encode_json, AppConfig}; +use zksync_consensus_tools::config; use zksync_protobuf::serde::Serde; /// Command line arguments. @@ -67,7 +67,7 @@ fn main() -> anyhow::Result<()> { let node_keys: Vec = (0..nodes).map(|_| rng.gen()).collect(); let mut cfgs: Vec<_> = (0..nodes) - .map(|i| AppConfig { + .map(|i| config::App { server_addr: SocketAddr::new(Ipv4Addr::UNSPECIFIED.into(), addrs[i].port()), public_addr: addrs[i].into(), rpc_addr: None, @@ -111,7 +111,7 @@ fn main() -> anyhow::Result<()> { .context("fs::set_permissions()")?; let config_path = root.join("config.json"); - fs::write(&config_path, encode_json(&Serde(cfg))).context("fs::write()")?; + fs::write(&config_path, config::encode_json(&Serde(cfg))).context("fs::write()")?; fs::set_permissions(&config_path, Permissions::from_mode(0o600)) .context("fs::set_permissions()")?; } diff --git a/node/tools/src/config.rs b/node/tools/src/config.rs index 75fa8243..26ffcb1d 100644 --- a/node/tools/src/config.rs +++ b/node/tools/src/config.rs @@ -14,11 +14,12 @@ use zksync_concurrency::{ctx, net, time}; use zksync_consensus_bft as bft; use zksync_consensus_crypto::{read_optional_text, read_required_text, Text, TextFmt}; use zksync_consensus_executor::{self as executor, attestation}; -use zksync_consensus_network::http; +use zksync_consensus_network as network; use zksync_consensus_roles::{attester, node, validator}; use zksync_consensus_storage::testonly::{TestMemoryStorage, TestMemoryStorageRunner}; -use zksync_consensus_utils::debug_page; -use zksync_protobuf::{kB, read_required, required, ProtoFmt}; +use zksync_protobuf::{ + kB, read_optional, read_optional_repr, read_required, required, ProtoFmt, ProtoRepr, +}; const CRATE_VERSION: &str = env!("CARGO_PKG_VERSION"); @@ -91,7 +92,7 @@ impl ProtoFmt for NodeAddr { /// Node configuration including executor configuration, optional validator configuration, /// and application-specific settings (e.g. metrics scraping). #[derive(Debug, PartialEq, Clone)] -pub struct AppConfig { +pub struct App { pub server_addr: SocketAddr, pub public_addr: net::Host, pub rpc_addr: Option, @@ -108,10 +109,66 @@ pub struct AppConfig { pub gossip_static_inbound: HashSet, pub gossip_static_outbound: HashMap, - pub debug_page: Option, + pub debug_page: Option, +} + +impl ProtoRepr for proto::Credentials { + type Type = network::debug_page::Credentials; + + fn read(&self) -> anyhow::Result { + Ok(Self::Type { + user: required(&self.user).context("user")?.clone(), + password: required(&self.password).context("password")?.clone(), + }) + } + + fn build(this: &Self::Type) -> Self { + Self { + user: Some(this.user.clone()), + password: Some(this.password.clone()), + } + } +} + +impl ProtoFmt for Tls { + type Proto = proto::TlsConfig; + + fn read(r: &Self::Proto) -> anyhow::Result { + Ok(Self { + cert_path: read_required_text(&r.cert_path).context("cert_path")?, + key_path: read_required_text(&r.key_path).context("key_path")?, + }) + } + + fn build(&self) -> Self::Proto { + Self::Proto { + cert_path: Some(self.cert_path.to_string_lossy().into()), + key_path: Some(self.key_path.to_string_lossy().into()), + } + } +} + +impl ProtoFmt for DebugPage { + type Proto = proto::DebugPageConfig; + + fn read(r: &Self::Proto) -> anyhow::Result { + Ok(Self { + addr: read_required_text(&r.addr).context("addr")?, + credentials: read_optional_repr(&r.credentials).context("credentials")?, + tls: read_optional(&r.tls).context("tls")?, + }) + } + + fn build(&self) -> Self::Proto { + Self::Proto { + addr: Some(self.addr.encode()), + credentials: self.credentials.as_ref().map(ProtoRepr::build), + tls: self.tls.as_ref().map(|x| x.build()), + } + } } -impl ProtoFmt for AppConfig { +impl ProtoFmt for App { type Proto = proto::AppConfig; fn read(r: &Self::Proto) -> anyhow::Result { @@ -165,25 +222,7 @@ impl ProtoFmt for AppConfig { .context("gossip_dynamic_inbound_limit")?, gossip_static_inbound, gossip_static_outbound, - debug_page: match read_optional_text(&r.debug_addr).context("debug_addr")? { - Some(addr) => Some(BasicDebugPageConfig { - addr, - credentials: r - .debug_credentials - .clone() - .map(debug_page::Credentials::try_from) - .transpose()?, - cert_path: read_optional_text(&r.debug_cert_path) - .context("debug_cert_path")? - // default to 'cert.pem' if cert_path is missing - .unwrap_or(PathBuf::from("cert.pem")), - key_path: read_optional_text(&r.debug_key_path) - .context("debug_key_path")? - // default to 'key.pem' if key_path is missing - .unwrap_or(PathBuf::from("key.pem")), - }), - _ => None, - }, + debug_page: read_optional(&r.debug_page).context("debug_page")?, }) } @@ -217,44 +256,33 @@ impl ProtoFmt for AppConfig { addr: Some(addr.0.clone()), }) .collect(), - debug_addr: self.debug_page.as_ref().map(|config| config.addr.encode()), - debug_credentials: self.debug_page.as_ref().map(|config| { - config - .credentials - .clone() - .map(debug_page::Credentials::into) - .unwrap() - }), - debug_cert_path: self - .debug_page - .as_ref() - .map(|config| config.cert_path.encode()), - debug_key_path: self - .debug_page - .as_ref() - .map(|config| config.key_path.encode()), + debug_page: self.debug_page.as_ref().map(|x| x.build()), } } } -/// Basic http debug page configuration. -/// Paths will be converted to actual cert and private key -/// on zksync_consensus_network::http::DebugPageConfig struct #[derive(Debug, PartialEq, Clone)] -pub struct BasicDebugPageConfig { - /// Public Http address to listen incoming http requests. - pub addr: SocketAddr, - /// Debug page credentials. - pub credentials: Option, +pub struct Tls { /// Cert file path pub cert_path: PathBuf, /// Key file path pub key_path: PathBuf, } +/// Debug page configuration. +#[derive(Debug, PartialEq, Clone)] +pub struct DebugPage { + /// Public Http address to listen incoming http requests. + pub addr: SocketAddr, + /// Debug page credentials. + pub credentials: Option, + /// TLS config. + pub tls: Option, +} + #[derive(Debug)] pub struct Configs { - pub app: AppConfig, + pub app: App, pub database: PathBuf, } @@ -281,16 +309,31 @@ impl Configs { max_payload_size: self.app.max_payload_size, max_batch_size: self.app.max_batch_size, rpc: executor::RpcConfig::default(), - debug_page: self.app.debug_page.as_ref().map(|debug_page_config| { - http::DebugPageConfig { - addr: debug_page_config.addr, - credentials: debug_page_config.credentials.clone(), - certs: load_certs(&debug_page_config.cert_path) - .expect("Could not obtain certs for debug page"), - private_key: load_private_key(&debug_page_config.key_path) - .expect("Could not obtain private key for debug page"), - } - }), + debug_page: self + .app + .debug_page + .as_ref() + .map(|debug_page_config| { + anyhow::Ok(network::debug_page::Config { + addr: debug_page_config.addr, + credentials: debug_page_config.credentials.clone(), + tls: debug_page_config + .tls + .as_ref() + .map(|tls| { + anyhow::Ok(network::debug_page::TlsConfig { + cert_chain: load_cert_chain(&tls.cert_path) + .context("load_cert_chain()")?, + private_key: load_private_key(&tls.key_path) + .context("load_private_key()")?, + }) + }) + .transpose() + .context("tls")?, + }) + }) + .transpose() + .context("debug_page")?, batch_poll_interval: time::Duration::seconds(1), }, block_store: store.blocks, @@ -313,23 +356,19 @@ impl Configs { } /// Load public certificate from file. -fn load_certs(path: &PathBuf) -> anyhow::Result>> { - // Open certificate file. - let certfile = fs::File::open(path).with_context(|| anyhow!("failed to open {:?}", path))?; - let mut reader = io::BufReader::new(certfile); - - // Load and return certificate. - Ok(rustls_pemfile::certs(&mut reader) - .map(|r| r.expect("Invalid certificate")) - .collect()) +fn load_cert_chain(path: &PathBuf) -> anyhow::Result>> { + let file = fs::File::open(path).with_context(|| anyhow!("failed to open {:?}", path))?; + let mut reader = io::BufReader::new(file); + rustls_pemfile::certs(&mut reader) + .collect::>() + .context("invalid certificate chain") } /// Load private key from file. fn load_private_key(path: &PathBuf) -> anyhow::Result> { - // Open keyfile. let keyfile = fs::File::open(path).with_context(|| anyhow!("failed to open {:?}", path))?; let mut reader = io::BufReader::new(keyfile); - - // Load and return a single private key. - Ok(rustls_pemfile::private_key(&mut reader).map(|key| key.expect("Private key not found"))?) + rustls_pemfile::private_key(&mut reader) + .context("invalid key")? + .context("no key in file") } diff --git a/node/tools/src/k8s.rs b/node/tools/src/k8s.rs index 6ad0a6d7..56b00b8e 100644 --- a/node/tools/src/k8s.rs +++ b/node/tools/src/k8s.rs @@ -1,4 +1,4 @@ -use crate::{config, AppConfig, NodeAddr}; +use crate::config; use anyhow::{ensure, Context}; use k8s_openapi::{ api::{ @@ -32,9 +32,9 @@ pub struct ConsensusNode { /// Node identifier pub id: String, /// Node configuration - pub config: AppConfig, + pub config: config::App, /// Full NodeAddr - pub node_addr: Option, + pub node_addr: Option, /// Is seed node (meaning it has no gossipStaticOutbound configuration) pub is_seed: bool, } @@ -67,7 +67,7 @@ impl ConsensusNode { .context("Status not present")? .pod_ip .context("Pod IP address not present")?; - self.node_addr = Some(NodeAddr { + self.node_addr = Some(config::NodeAddr { key: self.config.node_key.public(), addr: SocketAddr::new(ip.parse()?, config::NODES_PORT).into(), }); diff --git a/node/tools/src/lib.rs b/node/tools/src/lib.rs index f1b97867..e03e34fd 100644 --- a/node/tools/src/lib.rs +++ b/node/tools/src/lib.rs @@ -1,6 +1,6 @@ //! CLI tools for the consensus node. #![allow(missing_docs)] -mod config; +pub mod config; pub mod k8s; mod proto; pub mod rpc; @@ -9,7 +9,4 @@ mod store; #[cfg(test)] mod tests; -pub use config::{ - decode_json, encode_json, AppConfig, BasicDebugPageConfig, Configs, NodeAddr, NODES_PORT, -}; pub use rpc::server::RPCServer; diff --git a/node/tools/src/main.rs b/node/tools/src/main.rs index c927c129..73e3a39b 100644 --- a/node/tools/src/main.rs +++ b/node/tools/src/main.rs @@ -7,7 +7,7 @@ use tracing::metadata::LevelFilter; use tracing_subscriber::{prelude::*, Registry}; use vise_exporter::MetricsExporter; use zksync_concurrency::{ctx, scope}; -use zksync_consensus_tools::{decode_json, AppConfig, Configs, RPCServer, NODES_PORT}; +use zksync_consensus_tools::{config, RPCServer}; use zksync_protobuf::serde::Serde; /// Command-line application launching a node executor. @@ -26,22 +26,23 @@ struct Cli { impl Cli { /// Extracts configuration from the cli args. - fn load(&self) -> anyhow::Result { + fn load(&self) -> anyhow::Result { let raw = match &self.config { Some(raw) => raw.clone(), None => fs::read_to_string(&self.config_path)?, }; - Ok(Configs { - app: decode_json::>(&raw)?.0, + Ok(config::Configs { + app: config::decode_json::>(&raw)?.0, database: self.database.clone(), }) } } /// Overrides `cfg.public_addr`, based on the `PUBLIC_ADDR` env variable. -fn check_public_addr(cfg: &mut AppConfig) -> anyhow::Result<()> { +fn check_public_addr(cfg: &mut config::App) -> anyhow::Result<()> { if let Ok(public_addr) = std::env::var("PUBLIC_ADDR") { - cfg.public_addr = std::net::SocketAddr::new(public_addr.parse()?, NODES_PORT).into(); + cfg.public_addr = + std::net::SocketAddr::new(public_addr.parse()?, config::NODES_PORT).into(); } Ok(()) } diff --git a/node/tools/src/proto/mod.proto b/node/tools/src/proto/mod.proto index 4fd799c5..2120a9ee 100644 --- a/node/tools/src/proto/mod.proto +++ b/node/tools/src/proto/mod.proto @@ -48,8 +48,30 @@ message NodeAddr { optional string addr = 2; // required; IpAddr } +message TlsConfig { + optional string cert_path = 1; // required + optional string key_path = 2; // required +} + +message Credentials { + optional string user = 1; // required + optional string password = 2; // required +} + +message DebugPageConfig { + // IP:port to open a debug http endpoint on. + optional string addr = 1; // required; IpAddr + // Http debug page access credentials + optional Credentials credentials = 2; // optional + // TLS configuration. + optional TlsConfig tls = 3; // optional +} + // Application configuration. message AppConfig { + reserved 9,13,14,15; + reserved "debug_addr", "debug_credentials", "debug_cert_path", "debug_key_path"; + // Ports // IP:port to listen on, for incoming TCP connections. @@ -63,10 +85,6 @@ message AppConfig { // If not set, testing rpc endpoint won't be opened. optional string rpc_addr = 12; // optional; IpAddr - // IP:port to open a debug http endpoint on. - // If not set, debug endpoint won't be opened. - optional string debug_addr = 9; // optional; IpAddr - // IP:port to serve metrics data for scraping. // Use `0.0.0.0:` to listen on all network interfaces. // If not set, metrics data won't be served. @@ -103,10 +121,6 @@ message AppConfig { // establish and maintain. repeated NodeAddr gossip_static_outbound = 8; - // Http debug page access credentials - optional string debug_credentials = 13; // optional - // Path to the http debug page tls certificate - optional string debug_cert_path = 14; // optional - // Path to the http debug page private key - optional string debug_key_path = 15; // optional + // Debug page configuration. + optional DebugPageConfig debug_page = 18; // optional } diff --git a/node/tools/src/tests.rs b/node/tools/src/tests.rs index fb323e6f..50e82f66 100644 --- a/node/tools/src/tests.rs +++ b/node/tools/src/tests.rs @@ -1,15 +1,16 @@ -use crate::{store, AppConfig, BasicDebugPageConfig}; +use crate::{config, store}; use rand::{distributions::Distribution, Rng}; use tempfile::TempDir; use zksync_concurrency::{ctx, sync}; +use zksync_consensus_network as network; use zksync_consensus_roles::validator::testonly::Setup; use zksync_consensus_storage::{testonly, PersistentBlockStore}; use zksync_consensus_utils::EncodeDist; use zksync_protobuf::testonly::{test_encode_all_formats, FmtConv}; -impl Distribution for EncodeDist { - fn sample(&self, rng: &mut R) -> AppConfig { - AppConfig { +impl Distribution for EncodeDist { + fn sample(&self, rng: &mut R) -> config::App { + config::App { server_addr: self.sample(rng), public_addr: self.sample(rng), rpc_addr: self.sample(rng), @@ -33,22 +34,35 @@ impl Distribution for EncodeDist { } } -impl Distribution for EncodeDist { - fn sample(&self, rng: &mut R) -> BasicDebugPageConfig { - BasicDebugPageConfig { - addr: self.sample(rng), - credentials: self.sample(rng), +impl Distribution for EncodeDist { + fn sample(&self, rng: &mut R) -> config::Tls { + config::Tls { cert_path: self.sample(rng), key_path: self.sample(rng), } } } +impl Distribution for EncodeDist { + fn sample(&self, rng: &mut R) -> config::DebugPage { + config::DebugPage { + addr: self.sample(rng), + credentials: self.sample_opt(|| network::debug_page::Credentials { + user: self.sample(rng), + password: self.sample(rng), + }), + tls: self.sample(rng), + } + } +} + #[test] fn test_schema_encoding() { let ctx = ctx::test_root(&ctx::RealClock); let rng = &mut ctx.rng(); - test_encode_all_formats::>(rng); + test_encode_all_formats::>(rng); + test_encode_all_formats::>(rng); + test_encode_all_formats::>(rng); } #[tokio::test] From a219e82d6586ad80628a6adea9440d4a1a6d1d27 Mon Sep 17 00:00:00 2001 From: Grzegorz Prusak Date: Mon, 23 Sep 2024 13:39:29 +0200 Subject: [PATCH 2/3] typo --- node/actors/network/src/debug_page/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/actors/network/src/debug_page/mod.rs b/node/actors/network/src/debug_page/mod.rs index 5f20b754..05472854 100644 --- a/node/actors/network/src/debug_page/mod.rs +++ b/node/actors/network/src/debug_page/mod.rs @@ -126,7 +126,7 @@ impl Server { let cfg = ServerConfig::builder() .with_no_client_auth() .with_single_cert(tls.cert_chain.clone(), tls.private_key.clone_key()) - .context("with_signle_cert()")?; + .context("with_single_cert()")?; self.run_with_listener(ctx, TlsListener::new(Arc::new(cfg).into(), listener)) .await } else { From 88015be0ead464706fc2c71b561496e4ee3dc358 Mon Sep 17 00:00:00 2001 From: Grzegorz Prusak Date: Mon, 23 Sep 2024 13:53:55 +0200 Subject: [PATCH 3/3] clippy & removed some assumptions --- node/libs/concurrency/src/net/tcp/mod.rs | 29 ++++++++++-------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/node/libs/concurrency/src/net/tcp/mod.rs b/node/libs/concurrency/src/net/tcp/mod.rs index 097b4e3f..2fc6f7e4 100644 --- a/node/libs/concurrency/src/net/tcp/mod.rs +++ b/node/libs/concurrency/src/net/tcp/mod.rs @@ -17,13 +17,12 @@ pub type Listener = tokio::net::TcpListener; /// Accepts an INBOUND listener connection. pub async fn accept(ctx: &ctx::Ctx, this: &mut Listener) -> ctx::OrCanceled> { - Ok(ctx.wait(this.accept()).await?.map(|(stream, _)| { - // We are the only owner of the correctly opened - // socket at this point so `set_nodelay` should - // always succeed. - stream.set_nodelay(true).unwrap(); - stream - })) + ctx.wait(async { + let stream = this.accept().await?.0; + stream.set_nodelay(true)?; + Ok(stream) + }) + .await } /// Opens a TCP connection to a remote host. @@ -31,14 +30,10 @@ pub async fn connect( ctx: &ctx::Ctx, addr: std::net::SocketAddr, ) -> ctx::OrCanceled> { - Ok(ctx - .wait(tokio::net::TcpStream::connect(addr)) - .await? - .map(|stream| { - // We are the only owner of the correctly opened - // socket at this point so `set_nodelay` should - // always succeed. - stream.set_nodelay(true).unwrap(); - stream - })) + ctx.wait(async { + let stream = tokio::net::TcpStream::connect(addr).await?; + stream.set_nodelay(true)?; + Ok(stream) + }) + .await }