From d787921c357a25b1536d182d972ae944af880b2d Mon Sep 17 00:00:00 2001 From: Stanislau Hlebik Date: Fri, 29 Jun 2018 14:36:49 -0700 Subject: [PATCH] mononoke: tls for hgcli and Mononoke server Summary: Use tls for connection between hgcli and Mononoke server always, even for localhost connections[1] The setup is similar to tls setup of Eden server. [1] This is not necessary of course, but adding an option to bypass tls connection may result in accidental use of it in prod. However if it turns out to be too unusable, we can add such option in the future Reviewed By: jsgf Differential Revision: D8644299 fbshipit-source-id: 0898e30e33b718e13a766763479f3adf9323ffe7 --- hgcli/src/main.rs | 13 ++++++ hgcli/src/serve/mod.rs | 44 ++++++++++++++++-- server/src/main.rs | 56 +++++++++++++++++++---- server/src/ssl.rs | 46 +++++++++++++++++++ tests/integration/library.sh | 18 ++++++-- tests/integration/third_party/dummyssh.py | 19 ++++++-- 6 files changed, 176 insertions(+), 20 deletions(-) create mode 100644 server/src/ssl.rs diff --git a/hgcli/src/main.rs b/hgcli/src/main.rs index 927ee07753..4beed33a7c 100644 --- a/hgcli/src/main.rs +++ b/hgcli/src/main.rs @@ -15,11 +15,14 @@ extern crate tokio_uds; extern crate bytes; extern crate futures; +extern crate native_tls; +extern crate secure_utils; extern crate tokio; extern crate tokio_core; extern crate tokio_io; extern crate tokio_proto; extern crate tokio_service; +extern crate tokio_tls; extern crate mio; extern crate nix; @@ -56,6 +59,16 @@ fn main() { .arg(Arg::from_usage( "-a, --address [ADDR] 'address to listen on'", )) + .arg(Arg::from_usage( + "--cert [CERT] 'path to the certificate file'", + )) + .arg(Arg::from_usage("--ca-pem [PEM] 'path to the pem file'")) + .arg(Arg::from_usage( + "--private-key [KEY] 'path to the private key'", + )) + .arg(Arg::from_usage( + "--common-name [CN] 'expected SSL common name of the server see https://www.ssl.com/faqs/common-name/'", + )) .arg(Arg::from_usage("--stdio 'for remote clients'")) .arg( Arg::from_usage("--cmdserver [MODE] 'for remote clients'") diff --git a/hgcli/src/serve/mod.rs b/hgcli/src/serve/mod.rs index b4f2487214..88ac39cc1c 100644 --- a/hgcli/src/serve/mod.rs +++ b/hgcli/src/serve/mod.rs @@ -9,9 +9,12 @@ use std::net::SocketAddr; use bytes::Bytes; use futures::{future, stream, Future, Sink, Stream}; +use native_tls::TlsConnector; +use native_tls::backend::openssl::TlsConnectorBuilderExt; use tokio_core::reactor::Core; use tokio_io::AsyncRead; use tokio_io::codec::{FramedRead, FramedWrite}; +use tokio_tls::TlsConnectorExt; use tokio::net::TcpStream; @@ -20,6 +23,7 @@ use clap::ArgMatches; use errors::*; use futures_ext::StreamExt; +use secure_utils::build_pkcs12; use sshrelay::{Preamble, SshDecoder, SshEncoder, SshMsg, SshStream}; mod fdio; @@ -28,14 +32,35 @@ pub fn cmd(main: &ArgMatches, sub: &ArgMatches) -> Result<()> { if sub.is_present("stdio") { if let Some(repo) = main.value_of("repository") { let mononoke_path = sub.value_of("mononoke-path").unwrap(); - return stdio_relay(mononoke_path, repo); + + let cert = sub.value_of("cert") + .expect("certificate file is not specified") + .to_string(); + let private_key = sub.value_of("private-key") + .expect("private key file is not specified") + .to_string(); + let ca_pem = sub.value_of("ca-pem") + .expect("Cental authority pem file is not specified") + .to_string(); + let common_name = sub.value_of("common-name") + .expect("expected SSL common name of the Mononoke server") + .to_string(); + + return stdio_relay(mononoke_path, repo, cert, private_key, ca_pem, common_name); } bail_msg!("Missing repository"); } bail_msg!("Only stdio server is supported"); } -fn stdio_relay>(path: P, repo: &str) -> Result<()> { +fn stdio_relay>( + path: P, + repo: &str, + cert: String, + private_key: String, + ca_pem: String, + ssl_common_name: String, +) -> Result<()> { let path = path.as_ref(); let mut reactor = Core::new()?; @@ -50,7 +75,20 @@ fn stdio_relay>(path: P, repo: &str) -> Result<()> { let socket = TcpStream::connect(&addr) .map_err(|err| format_err!("connecting to Mononoke {} socket '{}' failed", path, err)); - let socket = reactor.run(socket)?; + let pkcs12 = build_pkcs12(cert, private_key)?; + let mut connector_builder = TlsConnector::builder()?; + connector_builder.identity(pkcs12)?; + { + let sslcontextbuilder = connector_builder.builder_mut(); + + sslcontextbuilder.set_ca_file(ca_pem)?; + } + let connector = connector_builder.build()?; + + let socket = reactor.run(socket.and_then(move |socket| { + let async_connector = connector.connect_async(&ssl_common_name, socket); + async_connector.map_err(|err| format_err!("async connect error {}", err)) + }))?; // Wrap the socket with the ssh codec let (socket_read, socket_write) = socket.split(); diff --git a/server/src/main.rs b/server/src/main.rs index 1f0c9cf9aa..76c1865633 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -22,6 +22,7 @@ extern crate tokio; extern crate tokio_codec; extern crate tokio_core; extern crate tokio_io; +extern crate tokio_tls; extern crate tokio_uds; extern crate rand; @@ -58,10 +59,13 @@ extern crate mercurial_types; #[cfg(test)] extern crate mercurial_types_mocks; extern crate metaconfig; +extern crate native_tls; +extern crate openssl; extern crate pylz4; extern crate repoinfo; extern crate revset; extern crate scuba_ext; +extern crate secure_utils; extern crate services; extern crate sshrelay; extern crate stats; @@ -76,6 +80,7 @@ mod listener; mod monitoring; mod remotefilelog; mod repo; +mod ssl; use std::collections::HashMap; use std::io; @@ -96,6 +101,7 @@ use futures::sync::mpsc; use futures_ext::{asynchronize, FutureExt}; use futures_stats::Timed; use tokio::util::FutureExt as TokioFutureExt; +use tokio_tls::TlsAcceptorExt; use bytes::Bytes; use clap::{App, ArgMatches}; @@ -166,6 +172,10 @@ fn setup_app<'a, 'b>() -> App<'a, 'b> { -p, --thrift_port [PORT] 'if provided the thrift server will start on this port' + --cert [PATH] 'path to a file with certificate' + --private-key [PATH] 'path to a file with private key' + --ca-pem [PATH] 'path to a file with CA certificate' + -d, --debug 'print debug level output' "#, ) @@ -244,6 +254,7 @@ fn start_repo_listeners( repos: I, root_log: &Logger, sockname: &str, + ssl: ssl::SslConfig, ) -> Result<(Vec>, ReadyState)> where I: IntoIterator, @@ -283,7 +294,7 @@ where .name(format!("connection_acceptor")) .spawn({ let root_log = root_log.clone(); - move || connection_acceptor(&sockname, root_log, repo_senders) + move || connection_acceptor(&sockname, root_log, repo_senders, ssl) }) .map_err(Error::from); @@ -309,7 +320,10 @@ fn connection_acceptor( sockname: &str, root_log: Logger, repo_senders: HashMap>, + ssl: ssl::SslConfig, ) -> ! { + let tls_acceptor = ssl::build_tls_acceptor(ssl).expect("failed to build tls acceptor"); + let mut core = tokio_core::reactor::Core::new().expect("failed to create tokio core"); let remote = core.remote(); let connection_acceptor = listener::listener(sockname) @@ -322,19 +336,32 @@ fn connection_acceptor( Ok(addr) => addr, Err(err) => { crit!(root_log, "Failed to get peer addr"; SlogKVError(Error::from(err))); - return Ok(None).into_future().boxify(); + return Ok(None).into_future().left_future(); } }; - ssh_server_mux(sock, remote.clone()) - .map(move |stdio| Some((stdio, addr))) - .or_else({ + tls_acceptor + .accept_async(sock) + .then({ + let remote = remote.clone(); let root_log = root_log.clone(); - move |err| { - error!(root_log, "Error while reading preamble: {}", err); - Ok(None) + move |sock| match sock { + Ok(sock) => ssh_server_mux(sock, remote.clone()) + .map(move |stdio| Some((stdio, addr))) + .or_else({ + let root_log = root_log.clone(); + move |err| { + error!(root_log, "Error while reading preamble: {}", err); + Ok(None) + } + }) + .left_future(), + Err(err) => { + error!(root_log, "Error while reading preamble: {}", err); + Ok(None).into_future().right_future() + } } }) - .boxify() + .right_future() } }) .for_each(move |maybe_stdio| { @@ -549,12 +576,23 @@ fn main() { let stats_aggregation = monitoring::start_stats()?; let config = get_config(root_log, &matches)?; + let cert = matches.value_of("cert").unwrap().to_string(); + let private_key = matches.value_of("private_key").unwrap().to_string(); + let ca_pem = matches.value_of("ca_pem").unwrap().to_string(); + + let ssl = ssl::SslConfig { + cert, + private_key, + ca_pem, + }; + let (repo_listeners, ready) = start_repo_listeners( config.repos.into_iter(), root_log, matches .value_of("listening-host-port") .expect("listening path must be specified"), + ssl, )?; tracing_fb303::register(); diff --git a/server/src/ssl.rs b/server/src/ssl.rs new file mode 100644 index 0000000000..5ae91fae62 --- /dev/null +++ b/server/src/ssl.rs @@ -0,0 +1,46 @@ +// Copyright (c) 2004-present, Facebook, Inc. +// All Rights Reserved. +// +// This software may be used and distributed according to the terms of the +// GNU General Public License version 2 or any later version. + +#![deny(warnings)] + +use native_tls::TlsAcceptor; +use native_tls::backend::openssl::TlsAcceptorBuilderExt; +use openssl::ssl::{SSL_VERIFY_FAIL_IF_NO_PEER_CERT, SSL_VERIFY_PEER}; + +use secure_utils; + +use errors::*; + +pub struct SslConfig { + pub cert: String, + pub private_key: String, + pub ca_pem: String, +} + +// Builds an acceptor that has `accept_async()` method that handles tls handshake +// and returns decrypted stream. +pub fn build_tls_acceptor(ssl: SslConfig) -> Result { + let pkcs12 = + secure_utils::build_pkcs12(ssl.cert, ssl.private_key).context("failed to build pkcs12")?; + let mut tlsacceptor_builder = TlsAcceptor::builder(pkcs12)?; + + // Set up client authentication + { + let sslcontextbuilder = tlsacceptor_builder.builder_mut(); + + sslcontextbuilder + .set_ca_file(ssl.ca_pem) + .context("cannot set CA file")?; + + // SSL_VERIFY_PEER checks client certificate if it was supplied. + // Connection is terminated if certificate verification fails. + // SSL_VERIFY_FAIL_IF_NO_PEER_CERT terminates the connection if client did not return + // certificate. + // More about it - https://wiki.openssl.org/index.php/Manual:SSL_CTX_set_verify(3) + sslcontextbuilder.set_verify(SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT); + } + tlsacceptor_builder.build().map_err(Error::from) +} diff --git a/tests/integration/library.sh b/tests/integration/library.sh index b62c0c8b81..aacc3b7033 100644 --- a/tests/integration/library.sh +++ b/tests/integration/library.sh @@ -19,19 +19,31 @@ EOF function mononoke { export MONONOKE_SOCKET MONONOKE_SOCKET=$(get_free_socket) - "$MONONOKE_SERVER" "$@" --debug --listening-host-port 127.0.0.1:"$MONONOKE_SOCKET" -P "$TESTTMP/mononoke-config-rocks" --configrepo_book local_master >> "$TESTTMP/mononoke.out" 2>&1 & + "$MONONOKE_SERVER" "$@" --ca-pem "$TESTDIR/testcert.crt" \ + --private-key "$TESTDIR/testcert.key" \ + --cert "$TESTDIR/testcert.crt" \ + --debug \ + --listening-host-port 127.0.0.1:"$MONONOKE_SOCKET" \ + -P "$TESTTMP/mononoke-config-rocks" \ + --configrepo_book local_master >> "$TESTTMP/mononoke.out" 2>&1 & echo $! >> "$DAEMON_PIDS" } # Wait until a Mononoke server is available for this repo. function wait_for_mononoke { local attempts=150 + + SSLCURL="curl --cert $TESTDIR/testcert.crt \ + --cacert $TESTDIR/testcert.crt \ + --key $TESTDIR/testcert.key \ + https://localhost:$MONONOKE_SOCKET" + for _ in $(seq 1 $attempts); do - curl 127.0.0.1:"$MONONOKE_SOCKET" 2>&1 | grep -q 'Empty reply' && break + $SSLCURL 2>&1 | grep -q 'Empty reply' && break sleep 0.1 done - if ! curl 127.0.0.1:"$MONONOKE_SOCKET" 2>&1 | grep -q 'Empty reply'; then + if ! $SSLCURL 2>&1 | grep -q 'Empty reply'; then echo "Mononoke did not start" >&2 cat "$TESTTMP/mononoke.out" exit 1 diff --git a/tests/integration/third_party/dummyssh.py b/tests/integration/third_party/dummyssh.py index f29126a244..fa8a9fc04e 100755 --- a/tests/integration/third_party/dummyssh.py +++ b/tests/integration/third_party/dummyssh.py @@ -5,12 +5,12 @@ from __future__ import absolute_import import os import sys -os.chdir(os.getenv('TESTTMP')) +os.chdir(os.getenv("TESTTMP")) if sys.argv[1] != "user@dummy": sys.exit(-1) -os.environ["SSH_CLIENT"] = "%s 1 2" % os.environ.get('LOCALIP', '127.0.0.1') +os.environ["SSH_CLIENT"] = "%s 1 2" % os.environ.get("LOCALIP", "127.0.0.1") log = open("dummylog", "ab") log.write("Got arguments") @@ -19,13 +19,22 @@ for i, arg in enumerate(sys.argv[1:]): log.write("\n") log.close() hgcmd = sys.argv[2] -if os.name == 'nt': +if os.name == "nt": # hack to make simple unix single quote quoting work on windows hgcmd = hgcmd.replace("'", '"') log = open("dummylog", "a+b") -if 'hgcli' in hgcmd: - hgcmd += ' --mononoke-path 127.0.0.1:' + os.getenv('MONONOKE_SOCKET') + +cert = os.path.join(os.getenv("TESTDIR"), "testcert.crt") +capem = os.path.join(os.getenv("TESTDIR"), "testcert.crt") +privatekey = os.path.join(os.getenv("TESTDIR"), "testcert.key") + +if "hgcli" in hgcmd: + hgcmd += ( + " --mononoke-path 127.0.0.1:" + + os.getenv("MONONOKE_SOCKET") + + (" --cert %s --ca-pem %s --private-key %s --common-name localhost" % (cert, capem, privatekey)) + ) r = os.system(hgcmd) sys.exit(bool(r))