Skip to content

Commit

Permalink
Update TLS dependencies for legacy client and fix vulnerability
Browse files Browse the repository at this point in the history
  • Loading branch information
n1ghtmare committed Jun 25, 2023
1 parent 662cfb2 commit adec6c9
Show file tree
Hide file tree
Showing 10 changed files with 301 additions and 72 deletions.
1 change: 0 additions & 1 deletion async-nats/tests/jetstream_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1154,7 +1154,6 @@ mod jetstream {
.unwrap();
}

#[ignore]
#[tokio::test]
async fn pull_sequence() {
let server = nats_server::run_server("tests/configs/jetstream.conf");
Expand Down
8 changes: 4 additions & 4 deletions nats/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ nuid = "0.3.1"
once_cell = "1.8.0"
parking_lot = "0.12.0"
regex = { version = "1.5.5", default-features = false, features = ["std", "unicode-perl"] }
rustls = "0.19.1"
rustls-native-certs = "0.5.0"
rustls-pemfile = "0.2.1"
webpki = "0.21.0"
rustls = "0.21"
rustls-native-certs = "0.6"
rustls-pemfile = "1.0.2"
webpki = { package = "rustls-webpki", version = "0.100.0", features = ["alloc", "std"] }
serde = { version = "1.0.126", features = ["derive"] }
serde_json = "1.0.64"
serde_nanos = "0.1.1"
Expand Down
2 changes: 1 addition & 1 deletion nats/src/auth_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ pub(crate) fn load_key(path: &Path) -> io::Result<PrivateKey> {
return Ok(PrivateKey(key))
}
// if public key is found, don't error, just skip it and hope to find client key next.
Some(rustls_pemfile::Item::X509Certificate(_)) => {}
Some(rustls_pemfile::Item::X509Certificate(_)) | Some(_) => {}
None => break,
}
}
Expand Down
139 changes: 85 additions & 54 deletions nats/src/connector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use lazy_static::__Deref;
use parking_lot::{Mutex, MutexGuard};
use std::collections::HashMap;
use std::convert::TryFrom;
use std::io::prelude::*;
use std::io::{self, BufReader, Error, ErrorKind};
use std::net::{Shutdown, SocketAddr, TcpStream, ToSocketAddrs};
Expand All @@ -23,11 +24,9 @@ use std::thread;
use std::time::Duration;
use url::{Host, Url};

use webpki::DNSNameRef;

use crate::auth_utils;
use crate::proto::{self, ClientOp, ServerOp};
use crate::rustls::{ClientConfig, ClientSession, Session};
use crate::rustls::{ClientConfig, ClientConnection};
use crate::secure_wipe::SecureString;
use crate::{connect::ConnectInfo, inject_io_failure, AuthStyle, Options, ServerInfo};

Expand All @@ -47,53 +46,80 @@ pub(crate) struct Connector {
tls_config: Arc<ClientConfig>,
}

impl Connector {
/// Creates a new connector with the URLs and options.
pub(crate) fn new(urls: Vec<ServerAddress>, options: Arc<Options>) -> io::Result<Connector> {
let mut tls_config = options.tls_client_config.clone();

// Include system root certificates.
//
// On Windows, some certificates cannot be loaded by rustls
// for whatever reason, so we simply skip them.
// See https://github.com/ctz/rustls-native-certs/issues/5
let roots = match rustls_native_certs::load_native_certs() {
Ok(store) | Err((Some(store), _)) => store.roots,
Err((None, _)) => Vec::new(),
};
for root in roots {
tls_config.root_store.roots.push(root);
}
fn configure_tls(options: &Arc<Options>) -> Result<ClientConfig, Error> {
let mut root_store = rustls::RootCertStore::empty();

// load native system certs only if user did not specify them
if options.tls_client_config.is_some() || options.certificates.is_empty() {
let native_certs = rustls_native_certs::load_native_certs()
.map_err(|err| {
io::Error::new(
ErrorKind::Other,
format!("could not load platform certs: {err}"),
)
})?
.into_iter()
.map(|cert| cert.0)
.collect::<Vec<Vec<u8>>>();

// Include user-provided certificates.
root_store.add_parsable_certificates(&native_certs);
}

if let Some(config) = &options.tls_client_config {
Ok(config.to_owned())
} else {
// Include user-provided certificates
for path in &options.certificates {
let contents = std::fs::read(path)?;
let mut cursor = std::io::Cursor::new(contents);

tls_config
.root_store
.add_pem_file(&mut cursor)
.map_err(|_| {
io::Error::new(io::ErrorKind::InvalidInput, "invalid certificate file")
})?;
let mut pem = BufReader::new(std::fs::File::open(path)?);
let certs = rustls_pemfile::certs(&mut pem)?;
let trust_anchors = certs.iter().map(|cert| {
let trust_anchor = webpki::TrustAnchor::try_from_cert_der(&cert[..])
.map_err(|err| {
io::Error::new(
ErrorKind::InvalidInput,
format!("could not load certs: {err}"),
)
})
.unwrap();
rustls::OwnedTrustAnchor::from_subject_spki_name_constraints(
trust_anchor.subject,
trust_anchor.spki,
trust_anchor.name_constraints,
)
});
root_store.add_server_trust_anchors(trust_anchors);
}

let builder = rustls::ClientConfig::builder()
.with_safe_defaults()
.with_root_certificates(root_store);

if let Some(cert) = &options.client_cert {
if let Some(key) = &options.client_key {
tls_config
.set_single_client_cert(
auth_utils::load_certs(cert)?,
auth_utils::load_key(key)?,
)
.map_err(|err| {
io::Error::new(
io::ErrorKind::InvalidInput,
format!("invalid client certificate and key pair: {err}"),
)
})?;
let cert = auth_utils::load_certs(cert)?;
let key = auth_utils::load_key(key)?;

return builder.with_single_cert(cert, key).map_err(|_| {
io::Error::new(ErrorKind::Other, "could not add certificate or key")
});
} else {
return Err(io::Error::new(
ErrorKind::Other,
"found certificate, but no key",
));
}
}

// if there are no client certs provided, connect with just TLS.
Ok(builder.with_no_client_auth())
}
}

impl Connector {
/// Creates a new connector with the URLs and options.
pub(crate) fn new(urls: Vec<ServerAddress>, options: Arc<Options>) -> io::Result<Connector> {
let tls_config = configure_tls(&options)?;

let connector = Connector {
attempts: urls.into_iter().map(|url| (url, 0)).collect(),
options,
Expand Down Expand Up @@ -252,15 +278,18 @@ impl Connector {
inject_io_failure()?;

// Connect using TLS.
let dns_name = DNSNameRef::try_from_ascii_str(&server_info.host)
.or_else(|_| DNSNameRef::try_from_ascii_str(server.host()))
.map_err(|_| {
let server_name =
rustls::client::ServerName::try_from(server.host()).map_err(|_| {
io::Error::new(
io::ErrorKind::InvalidInput,
"cannot determine hostname for TLS connection",
)
})?;
Some(ClientSession::new(&self.tls_config, dns_name))

Some(
ClientConnection::new(self.tls_config.clone(), server_name)
.map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?,
)
} else {
None
};
Expand Down Expand Up @@ -374,12 +403,12 @@ enum Flavor {

struct TlsStream {
tcp: TcpStream,
session: ClientSession,
session: ClientConnection,
}

impl NatsStream {
/// Creates a NATS stream from a TCP stream and an optional TLS session.
fn new(tcp: TcpStream, session: Option<ClientSession>) -> io::Result<NatsStream> {
fn new(tcp: TcpStream, session: Option<ClientConnection>) -> io::Result<NatsStream> {
let flavor = match session {
None => Flavor::Tcp(tcp),
Some(session) => {
Expand Down Expand Up @@ -418,7 +447,7 @@ impl Read for &NatsStream {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
match &*self.flavor {
Flavor::Tcp(tcp) => (tcp.deref()).read(buf),
Flavor::Tls(tls) => tls_op(tls, |session, eof| match session.read(buf) {
Flavor::Tls(tls) => tls_op(tls, |session, eof| match session.reader().read(buf) {
Ok(0) if !eof => Err(io::ErrorKind::WouldBlock.into()),
res => res,
}),
Expand All @@ -440,14 +469,14 @@ impl Write for &NatsStream {
fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
match &*self.flavor {
Flavor::Tcp(tcp) => (tcp.deref()).write(buf),
Flavor::Tls(tls) => tls_op(tls, |session, _| session.write(buf)),
Flavor::Tls(tls) => tls_op(tls, |session, _| session.writer().write(buf)),
}
}

fn flush(&mut self) -> io::Result<()> {
match &*self.flavor {
Flavor::Tcp(tcp) => (tcp.deref()).flush(),
Flavor::Tls(tls) => tls_op(tls, |session, _| session.flush()),
Flavor::Tls(tls) => tls_op(tls, |session, _| session.writer().flush()),
}
}
}
Expand All @@ -457,7 +486,7 @@ impl Write for &NatsStream {
/// However, note that the inner TCP stream is in non-blocking mode.
fn tls_op<T: std::fmt::Debug>(
tls: &Mutex<TlsStream>,
mut op: impl FnMut(&mut ClientSession, bool) -> io::Result<T>,
mut op: impl FnMut(&mut ClientConnection, bool) -> io::Result<T>,
) -> io::Result<T> {
loop {
let mut tls = tls.lock();
Expand All @@ -468,9 +497,11 @@ fn tls_op<T: std::fmt::Debug>(
if session.wants_read() {
match session.read_tls(tcp) {
Ok(0) => eof = true,
Ok(_) => session
.process_new_packets()
.map_err(|err| Error::new(ErrorKind::Other, format!("TLS error: {err}")))?,
Ok(_) => {
session
.process_new_packets()
.map_err(|err| Error::new(ErrorKind::Other, format!("TLS error: {err}")))?;
}
Err(err) if err.kind() == ErrorKind::WouldBlock => {}
Err(err) => return Err(err),
}
Expand Down
25 changes: 18 additions & 7 deletions nats/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ pub struct Options {
pub(crate) certificates: Vec<PathBuf>,
pub(crate) client_cert: Option<PathBuf>,
pub(crate) client_key: Option<PathBuf>,
pub(crate) tls_client_config: crate::rustls::ClientConfig,
pub(crate) tls_client_config: Option<crate::rustls::ClientConfig>,

pub(crate) error_callback: ErrorCallback,
pub(crate) disconnect_callback: Callback,
Expand Down Expand Up @@ -91,7 +91,7 @@ impl Default for Options {
reconnect_delay_callback: ReconnectDelayCallback(Box::new(backoff)),
close_callback: Callback(None),
lame_duck_callback: Callback(None),
tls_client_config: crate::rustls::ClientConfig::default(),
tls_client_config: None,
}
}
}
Expand Down Expand Up @@ -342,19 +342,30 @@ impl Options {
/// # Example
/// ```no_run
/// # fn main() -> std::io::Result<()> {
/// let mut tls_client_config = nats::rustls::ClientConfig::default();
/// tls_client_config.set_single_client_cert(
/// vec![nats::rustls::Certificate(b"MY_CERT".to_vec())],
/// nats::rustls::PrivateKey(b"MY_KEY".to_vec()),
/// let mut root_store = nats::rustls::RootCertStore::empty();
///
/// root_store.add_parsable_certificates(
/// rustls_native_certs::load_native_certs()?
/// .into_iter()
/// .map(|cert| cert.0)
/// .collect::<Vec<Vec<u8>>>()
/// .as_ref(),
/// );
///
/// let tls_client_config = nats::rustls::ClientConfig::builder()
/// .with_safe_defaults()
/// .with_root_certificates(root_store)
/// .with_no_client_auth();
///
/// let nc = nats::Options::new()
/// .tls_client_config(tls_client_config)
/// .connect("nats://localhost:4443")?;
///
/// # Ok(())
/// # }
/// ```
pub fn tls_client_config(mut self, tls_client_config: crate::rustls::ClientConfig) -> Options {
self.tls_client_config = tls_client_config;
self.tls_client_config = Some(tls_client_config);
self
}

Expand Down
27 changes: 22 additions & 5 deletions nats/tests/auth_tls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,28 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use std::io;
use std::path::PathBuf;

#[test]
fn basic_tls() -> io::Result<()> {
fn basic_tls() {
let server = nats_server::run_server("tests/configs/tls.conf");

// Should fail without certs.
assert!(nats::connect(server.client_url()).is_err());

let path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));

// Should fail with IP (cert doesn't have proper SAN entry)
assert!(nats::connect(format!("tls://127.0.0.1:{}", server.client_port())).is_err());

nats::Options::with_user_pass("derek", "porkchop")
.add_root_certificate(path.join("tests/configs/certs/rootCA.pem"))
.client_cert(
path.join("tests/configs/certs/client-cert.pem"),
path.join("tests/configs/certs/client-key.pem"),
)
.connect(server.client_url())?;
.connect(server.client_url())
.unwrap();

// test scenario where rootCA, client certificate and client key are all in one .pem file
nats::Options::with_user_pass("derek", "porkchop")
Expand All @@ -36,7 +41,19 @@ fn basic_tls() -> io::Result<()> {
path.join("tests/configs/certs/client-all.pem"),
path.join("tests/configs/certs/client-all.pem"),
)
.connect(server.client_url())?;
.connect(server.client_url())
.unwrap();
}

#[test]
fn ip_basic_tls() {
let server = nats_server::run_server("tests/configs/ip-tls.conf");

let path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));

Ok(())
nats::Options::with_user_pass("derek", "porkchop")
.add_root_certificate(path.join("tests/configs/certs/ip-ca.pem"))
.tls_required(true)
.connect(format!("tls://127.0.0.1:{}", server.client_port()))
.unwrap();
}
27 changes: 27 additions & 0 deletions nats/tests/configs/certs/ip-ca.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
-----BEGIN CERTIFICATE-----
MIIEkDCCA3igAwIBAgIUSZwW7btc9EUbrMWtjHpbM0C2bSEwDQYJKoZIhvcNAQEL
BQAwcTELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExEDAOBgNVBAoM
B1N5bmFkaWExEDAOBgNVBAsMB25hdHMuaW8xKTAnBgNVBAMMIENlcnRpZmljYXRl
IEF1dGhvcml0eSAyMDIyLTA4LTI3MB4XDTIyMDgyNzIwMjMwMloXDTMyMDgyNDIw
MjMwMlowcTELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExEDAOBgNV
BAoMB1N5bmFkaWExEDAOBgNVBAsMB25hdHMuaW8xKTAnBgNVBAMMIENlcnRpZmlj
YXRlIEF1dGhvcml0eSAyMDIyLTA4LTI3MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A
MIIBCgKCAQEAqilVqyY8rmCpTwAsLF7DEtWEq37KbljBWVjmlp2Wo6TgMd3b537t
6iO8+SbI8KH75i63RcxV3Uzt1/L9Yb6enDXF52A/U5ugmDhaa+Vsoo2HBTbCczmp
qndp7znllQqn7wNLv6aGSvaeIUeYS5Dmlh3kt7Vqbn4YRANkOUTDYGSpMv7jYKSu
1ee05Rco3H674zdwToYto8L8V7nVMrky42qZnGrJTaze+Cm9tmaIyHCwUq362CxS
dkmaEuWx11MOIFZvL80n7ci6pveDxe5MIfwMC3/oGn7mbsSqidPMcTtjw6ey5NEu
Z0UrC/2lL1FtF4gnVMKUSaEhU2oKjj0ZAQIDAQABo4IBHjCCARowHQYDVR0OBBYE
FP7Pfz4u7sSt6ltviEVsx4hIFIs6MIGuBgNVHSMEgaYwgaOAFP7Pfz4u7sSt6ltv
iEVsx4hIFIs6oXWkczBxMQswCQYDVQQGEwJVUzETMBEGA1UECAwKQ2FsaWZvcm5p
YTEQMA4GA1UECgwHU3luYWRpYTEQMA4GA1UECwwHbmF0cy5pbzEpMCcGA1UEAwwg
Q2VydGlmaWNhdGUgQXV0aG9yaXR5IDIwMjItMDgtMjeCFEmcFu27XPRFG6zFrYx6
WzNAtm0hMAwGA1UdEwQFMAMBAf8wOgYJYIZIAYb4QgENBC0WK25hdHMuaW8gbmF0
cy1zZXJ2ZXIgdGVzdC1zdWl0ZSB0cmFuc2llbnQgQ0EwDQYJKoZIhvcNAQELBQAD
ggEBAHDCHLQklYZlnzHDaSwxgGSiPUrCf2zhk2DNIYSDyBgdzrIapmaVYQRrCBtA
j/4jVFesgw5WDoe4TKsyha0QeVwJDIN8qg2pvpbmD8nOtLApfl0P966vcucxDwqO
dQWrIgNsaUdHdwdo0OfvAlTfG0v/y2X0kbL7h/el5W9kWpxM/rfbX4IHseZL2sLq
FH69SN3FhMbdIm1ldrcLBQVz8vJAGI+6B9hSSFQWljssE0JfAX+8VW/foJgMSx7A
vBTq58rLkAko56Jlzqh/4QT+ckayg9I73v1Q5/44jP1mHw35s5ZrzpDQt2sVv4l5
lwRPJFXMwe64flUs9sM+/vqJaIY=
-----END CERTIFICATE-----
Loading

0 comments on commit adec6c9

Please sign in to comment.