Skip to content

Commit

Permalink
Merge pull request #245 from umccr/refactor/swap-hyper
Browse files Browse the repository at this point in the history
refactor: swap the hyper client for reqwest
  • Loading branch information
mmalenic authored May 10, 2024
2 parents c84be9a + d0ab9a4 commit 4cff66c
Show file tree
Hide file tree
Showing 16 changed files with 731 additions and 632 deletions.
829 changes: 439 additions & 390 deletions Cargo.lock

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion htsget-actix/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]
name = "htsget-actix"
version = "0.5.8"
rust-version = "1.70"
rust-version = "1.74"
authors = ["Daniel del Castillo de la Rosa <delcastillodelarosadaniel@gmail.com>", "Marko Malenic <mmalenic1@gmail.com>"]
edition = "2021"
license = "MIT"
Expand Down
3 changes: 0 additions & 3 deletions htsget-actix/benches/request_benchmarks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ const REFSERVER_DOCKER_IMAGE: &str = "ga4gh/htsget-refserver:1.5.0";
const BENCHMARK_DURATION_SECONDS: u64 = 30;
const NUMBER_OF_SAMPLES: usize = 50;

#[derive(Serialize)]
struct Empty;

#[derive(Deserialize)]
struct RefserverConfig {
#[serde(rename = "htsgetConfig")]
Expand Down
9 changes: 4 additions & 5 deletions htsget-config/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]
name = "htsget-config"
version = "0.8.1"
rust-version = "1.70"
rust-version = "1.74"
authors = ["Daniel del Castillo de la Rosa <delcastillodelarosadaniel@gmail.com>", "Marko Malenic <mmalenic1@gmail.com>"]
edition = "2021"
description = "Used to configure htsget-rs by using a config file or reading environment variables."
Expand All @@ -12,7 +12,7 @@ repository = "https://github.com/umccr/htsget-rs"

[features]
s3-storage = []
url-storage = ["hyper"]
url-storage = ["reqwest"]
default = []

[dependencies]
Expand All @@ -32,10 +32,9 @@ http = "0.2"
http-serde = "1.1"
rustls-pemfile = "1.0"
rustls = "0.21"
rustls-native-certs = "0.6"
hyper-rustls = { version = "0.24", features = ["rustls-native-certs", "http2", "http1"] }

hyper = { version = "0.14", features = ["http1", "http2", "client"], optional = true }
# url-storage
reqwest = { version = "0.11", features = ["rustls-tls"], default-features = false, optional = true }

[dev-dependencies]
serde_json = "1.0"
Expand Down
42 changes: 37 additions & 5 deletions htsget-config/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -275,11 +275,11 @@ TLS can be configured for the ticket server, data server, or the url storage cli
certificates from PEM-formatted files. Certificates must be in X.509 format and private keys can be RSA, PKCS8, or SEC1 (EC) encoded.
The following options are available:

| Option | Description | Type | Default |
|------------------------|-------------------------------------------------------------------------------------------------------------------------------------------|-------------------|---------|
| `key` | The path to the PEM formatted X.509 certificate. Specifies TLS for servers or client authentication for clients. | Filesystem path | Not Set |
| `cert` | The path to the PEM formatted RSA, PKCS8, or SEC1 encoded EC private key. Specifies TLS for servers or client authentication for clients. | Filesystem path | Not Set |
| `root_store` | The path to the PEM formatted root certificate store. Only used to specify non-native root certificates for client TLS. | Filesystem path | Not Set |
| Option | Description | Type | Default |
|------------------------|----------------------------------------------------------------------------------------------------------------------------------------------|-------------------|---------|
| `key` | The path to the PEM formatted X.509 certificate. Specifies TLS for servers or client authentication for clients. | Filesystem path | Not Set |
| `cert` | The path to the PEM formatted RSA, PKCS8, or SEC1 encoded EC private key. Specifies TLS for servers or client authentication for clients. | Filesystem path | Not Set |
| `root_store` | The path to the PEM formatted root certificate store. Only used to specify non-native root certificates for the HTTP client in `UrlStorage`. | Filesystem path | Not Set |

When used by the ticket and data servers, `key` and `cert` enable TLS, and when used with the url storage client, they enable client authentication.
The root store is only used by the url storage client. Note, the url storage client always allows TLS, however the default configuration performs no client authentication
Expand All @@ -291,9 +291,41 @@ ticket_server_tls.cert = "cert.pem"
ticket_server_tls.key = "key.pem"
```

This project uses [rustls] for all TLS logic, and it does not depend on OpenSSL. The rustls library can be more
strict when accepting certificates and keys. For example, it does not accept self-signed certificates that have
a CA used as an end-entity. If generating certificates for `root_store` using OpenSSL, the correct extensions,
such as `subjectAltName` should be included.

An example of generating a custom root CA and certificates for a `UrlStorage` backend:

```sh
# Create a root CA
openssl req -x509 -noenc -subj '/CN=localhost' -newkey rsa -keyout root.key -out root.crt

# Create a certificate signing request
openssl req -noenc -newkey rsa -keyout server.key -out server.csr -subj '/CN=localhost' -addext subjectAltName=DNS:localhost

# Create the `UrlStorage` server's certificate
openssl x509 -req -in server.csr -CA root.crt -CAkey root.key -days 365 -out server.crt -copy_extensions copy

# An additional client certificate signing request and certificate can be created in the same way as the server
# certificate if using client authentication.
```

The `root.crt` can then be used in htsget-rs to allow authenticating to a `UrlStorage` backend using `server.crt`:

```toml
# Trust the root CA that signed the server's certificate.
tls.root_store = "root.crt"
```

Alternatively, projects such as [mkcert] can be used to simplify this process.

Further TLS examples are available under [`examples/config-files`][examples-config-files].

[examples-config-files]: examples/config-files
[rustls]: https://github.com/rustls/rustls
[mkcert]: https://github.com/FiloSottile/mkcert

#### Config file location

Expand Down
2 changes: 1 addition & 1 deletion htsget-config/examples/config-files/url_storage.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ substitution_string = "$0"

[resolvers.storage]
url = "http://127.0.0.1:8081"
response_scheme = "https"
response_url = "https://127.0.0.1:8081"
forward_headers = true

# Set client authentication
Expand Down
13 changes: 3 additions & 10 deletions htsget-config/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,8 +440,8 @@ mod tests {

#[cfg(feature = "url-storage")]
use {
crate::storage::url, crate::storage::url::ValidatedUrl, http::Uri as InnerUrl, hyper::Client,
hyper_rustls::HttpsConnectorBuilder, std::str::FromStr,
crate::storage::url, crate::storage::url::ValidatedUrl, http::Uri as InnerUrl,
reqwest::ClientBuilder, std::str::FromStr,
};

use crate::config::tests::{test_config_from_env, test_config_from_file};
Expand Down Expand Up @@ -528,14 +528,7 @@ mod tests {
#[cfg(feature = "url-storage")]
#[tokio::test]
async fn resolver_resolve_url_request() {
let client = Client::builder().build(
HttpsConnectorBuilder::new()
.with_native_roots()
.https_or_http()
.enable_http1()
.enable_http2()
.build(),
);
let client = ClientBuilder::new().build().unwrap();
let url_storage = UrlStorageClient::new(
ValidatedUrl(url::Url {
inner: InnerUrl::from_str("https://example.com/").unwrap(),
Expand Down
70 changes: 49 additions & 21 deletions htsget-config/src/storage/url.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
use std::str::FromStr;

use http::Uri as InnerUrl;
use hyper::client::HttpConnector;
use hyper::Client;
use hyper_rustls::{HttpsConnector, HttpsConnectorBuilder};
use reqwest::Client;
use serde::{Deserialize, Serialize};
use serde_with::with_prefix;

use crate::error::Error::ParseError;
use crate::error::{Error, Result};
use crate::storage::local::default_authority;
use crate::tls::TlsClientConfig;
use crate::tls::client::TlsClientConfig;

fn default_url() -> ValidatedUrl {
ValidatedUrl(Url {
Expand All @@ -32,31 +30,41 @@ pub struct UrlStorage {
}

#[derive(Deserialize, Debug, Clone)]
#[serde(from = "UrlStorage")]
#[serde(try_from = "UrlStorage")]
pub struct UrlStorageClient {
url: ValidatedUrl,
response_url: ValidatedUrl,
forward_headers: bool,
client: Client<HttpsConnector<HttpConnector>>,
client: Client,
}

impl From<UrlStorage> for UrlStorageClient {
fn from(storage: UrlStorage) -> Self {
let client = Client::builder().build(
HttpsConnectorBuilder::new()
.with_tls_config(storage.tls.into_inner())
.https_or_http()
.enable_http1()
.enable_http2()
.build(),
);

Self::new(
impl TryFrom<UrlStorage> for UrlStorageClient {
type Error = Error;

fn try_from(storage: UrlStorage) -> Result<Self> {
let mut builder = Client::builder();

let (certs, identity) = storage.tls.into_inner();

if let Some(certs) = certs {
for cert in certs {
builder = builder.add_root_certificate(cert);
}
}
if let Some(identity) = identity {
builder = builder.identity(identity);
}

let client = builder
.build()
.map_err(|err| ParseError(format!("building url storage client: {}", err)))?;

Ok(Self::new(
storage.url,
storage.response_url,
storage.forward_headers,
client,
)
))
}
}

Expand All @@ -66,7 +74,7 @@ impl UrlStorageClient {
url: ValidatedUrl,
response_url: ValidatedUrl,
forward_headers: bool,
client: Client<HttpsConnector<HttpConnector>>,
client: Client,
) -> Self {
Self {
url,
Expand All @@ -91,7 +99,8 @@ impl UrlStorageClient {
self.forward_headers
}

pub fn client_cloned(&self) -> Client<HttpsConnector<HttpConnector>> {
/// Get an owned client by cloning.
pub fn client_cloned(&self) -> Client {
self.client.clone()
}
}
Expand Down Expand Up @@ -181,10 +190,29 @@ impl Default for UrlStorage {
#[cfg(test)]
mod tests {
use crate::config::tests::test_config_from_file;
use crate::storage::url::{UrlStorage, UrlStorageClient};
use crate::storage::Storage;
use crate::tls::client::tests::client_config_from_path;

use crate::tls::tests::with_test_certificates;

use super::*;

#[tokio::test]
async fn test_building_client() {
with_test_certificates(|path, _, _| {
let client_config = client_config_from_path(path);
let url_storage = UrlStorageClient::try_from(UrlStorage::new(
"https://example.com".parse::<InnerUrl>().unwrap(),
"https://example.com".parse::<InnerUrl>().unwrap(),
true,
client_config,
));

assert!(url_storage.is_ok());
});
}

#[test]
fn config_storage_url_file() {
with_test_certificates(|path, _, _| {
Expand Down
96 changes: 96 additions & 0 deletions htsget-config/src/tls/client.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
//! TLS configuration related to HTTP clients.
//!

use crate::error::Error;
use crate::error::Error::IoError;
use crate::tls::RootCertStorePair;
use crate::tls::{load_certs, read_bytes};
use serde::Deserialize;

/// A certificate and key pair used for TLS. Serialization is not implemented because there
/// is no way to convert back to a `PathBuf`.
#[derive(Deserialize, Debug, Clone, Default)]
#[serde(try_from = "RootCertStorePair")]
pub struct TlsClientConfig {
cert: Option<Vec<reqwest::Certificate>>,
identity: Option<reqwest::Identity>,
}

impl TlsClientConfig {
/// Create a new TlsClientConfig.
pub fn new(cert: Option<Vec<reqwest::Certificate>>, identity: Option<reqwest::Identity>) -> Self {
Self { cert, identity }
}

/// Get the inner client config.
pub fn into_inner(self) -> (Option<Vec<reqwest::Certificate>>, Option<reqwest::Identity>) {
(self.cert, self.identity)
}
}

impl TryFrom<RootCertStorePair> for TlsClientConfig {
type Error = Error;

fn try_from(root_store_pair: RootCertStorePair) -> crate::error::Result<Self> {
let (key_pair, root_store) = root_store_pair.into_inner();

let cert = root_store
.clone()
.map(|cert_path| {
let certs = load_certs(cert_path)?;

certs
.into_iter()
.map(|cert| {
reqwest::Certificate::from_der(&cert.0)
.map_err(|err| IoError(format!("failed to read certificate from pem: {}", err)))
})
.collect::<crate::error::Result<Vec<_>>>()
})
.transpose()?;

let identity = key_pair
.clone()
.map(|pair| {
let key = read_bytes(pair.key)?;
let certs = read_bytes(pair.cert)?;

reqwest::Identity::from_pem(&[certs, key].concat())
.map_err(|err| IoError(format!("failed to pkcs8 pem identity: {}", err)))
})
.transpose()?;

Ok(Self::new(cert, identity))
}
}

#[cfg(test)]
pub(crate) mod tests {
use crate::tls::tests::with_test_certificates;
use crate::tls::{CertificateKeyPairPath, RootCertStorePair};
use std::path::Path;

use super::*;

#[tokio::test]
async fn test_tls_client_config() {
with_test_certificates(|path, _, _| {
let client_config = client_config_from_path(path);
let (certs, identity) = client_config.into_inner();

assert_eq!(certs.unwrap().len(), 1);
assert!(identity.is_some());
});
}

pub(crate) fn client_config_from_path(path: &Path) -> TlsClientConfig {
TlsClientConfig::try_from(RootCertStorePair::new(
Some(CertificateKeyPairPath::new(
path.join("cert.pem"),
path.join("key.pem"),
)),
Some(path.join("cert.pem")),
))
.unwrap()
}
}
Loading

0 comments on commit 4cff66c

Please sign in to comment.