Skip to content

Commit

Permalink
Return a ValueAbsent for all the downstream-tls related functions ins…
Browse files Browse the repository at this point in the history
…tead of a NotAvailable (#315)

* Return a ValueAbsent for all the downstream-tls related functions

Currently these functions return a NotAvailable error, I believe this is because we don't support https within Viceroy.

We do support http though, and http is also supported on Fastly, this patch adds an implementation for the downstream-tls functions which is valid for http (non-tls) connections. As Viceroy does not yet support https, we could say this is a full implementation as-of-today. In the future if/when Viceroy supports https, we would need to come back to these functions and make them return the correct, expected values based on the incoming connection (tls or not).

* Remove ja3 call in test as the current rust sdk will panic from that call

* Add note that the test for get_tls_raw_client_certificate() on non-tls requests is commented out as it currently panics - we are waiting on a patch to land in the fastly crate to make it not panic in this situation
  • Loading branch information
Jake Champion authored Apr 4, 2024
1 parent 8feaf78 commit 4067d0f
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 22 deletions.
45 changes: 23 additions & 22 deletions lib/src/wiggle_abi/req_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,14 @@ impl FastlyHttpReq for Session {
Err(Error::NotAvailable("Client original header fingerprint"))
}

#[allow(unused_variables)] // FIXME KTM 2020-06-25: Remove this directive once implemented.
fn downstream_tls_cipher_openssl_name(
fn downstream_tls_cipher_openssl_name<'a>(
&mut self,
cipher_out: &GuestPtr<'_, u8>,
cipher_max_len: u32,
nwritten_out: &GuestPtr<u32>,
_cipher_out: &GuestPtr<'a, u8>,
_cipher_max_len: u32,
_nwritten_out: &GuestPtr<u32>,
) -> Result<(), Error> {
Err(Error::NotAvailable("Client TLS data"))
// FIXME JDC 2023-09-27: For now, we don't support incoming TLS connections, this function currently only implements the solution for non-tls connections.
Err(Error::ValueAbsent)
}

#[allow(unused_variables)] // FIXME ACF 2022-05-03: Remove this directive once implemented.
Expand Down Expand Up @@ -184,14 +184,14 @@ impl FastlyHttpReq for Session {
Err(Error::NotAvailable("Redirect to Fanout/GRIP proxy"))
}

#[allow(unused_variables)] // FIXME KTM 2020-06-25: Remove this directive once implemented.
fn downstream_tls_protocol(
fn downstream_tls_protocol<'a>(
&mut self,
protocol_out: &GuestPtr<'_, u8>,
protocol_max_len: u32,
nwritten_out: &GuestPtr<u32>,
_protocol_out: &GuestPtr<'a, u8>,
_protocol_max_len: u32,
_nwritten_out: &GuestPtr<u32>,
) -> Result<(), Error> {
Err(Error::NotAvailable("Client TLS data"))
// FIXME JDC 2023-09-27: For now, we don't support incoming TLS connections, this function currently only implements the solution for non-tls connections.
Err(Error::ValueAbsent)
}

#[allow(unused_variables)] // FIXME KTM 2020-06-25: Remove this directive once implemented.
Expand All @@ -201,29 +201,30 @@ impl FastlyHttpReq for Session {
chello_max_len: u32,
nwritten_out: &GuestPtr<u32>,
) -> Result<(), Error> {
Err(Error::NotAvailable("Client TLS data"))
// FIXME JDC 2023-09-27: For now, we don't support incoming TLS connections, this function currently only implements the solution for non-tls connections.
Err(Error::ValueAbsent)
}

#[allow(unused_variables)] // FIXME HL 2022-09-19: Remove this directive once implemented.
fn downstream_tls_raw_client_certificate(
fn downstream_tls_raw_client_certificate<'a>(
&mut self,
_raw_client_cert_out: &GuestPtr<'_, u8>,
_tokio_rustlsraw_client_cert_out: &GuestPtr<'a, u8>,
_raw_client_cert_max_len: u32,
_nwritten_out: &GuestPtr<u32>,
) -> Result<(), Error> {
Err(Error::NotAvailable("Client TLS data"))
// FIXME JDC 2023-09-27: For now, we don't support incoming TLS connections, this function currently only implements the solution for non-tls connections.
Err(Error::ValueAbsent)
}

#[allow(unused_variables)] // FIXME HL 2022-09-19: Remove this directive once implemented.
fn downstream_tls_client_cert_verify_result(
&mut self,
) -> Result<ClientCertVerifyResult, Error> {
Err(Error::NotAvailable("Client TLS data"))
// FIXME JDC 2023-09-27: For now, we don't support incoming TLS connections, this function currently only implements the solution for non-tls connections.
Err(Error::ValueAbsent)
}

#[allow(unused_variables)] // FIXME ACF 2022-05-03: Remove this directive once implemented.
fn downstream_tls_ja3_md5(&mut self, ja3_md5_out: &GuestPtr<u8>) -> Result<u32, Error> {
Err(Error::NotAvailable("Client TLS JA3 hash"))
fn downstream_tls_ja3_md5(&mut self, _ja3_md5_out: &GuestPtr<u8>) -> Result<u32, Error> {
// FIXME JDC 2023-09-27: For now, we don't support incoming TLS connections, this function currently only implements the solution for non-tls connections.
Err(Error::ValueAbsent)
}

#[allow(unused_variables)] // FIXME UFSM 2024-02-19: Remove this directive once implemented.
Expand Down
9 changes: 9 additions & 0 deletions test-fixtures/src/bin/downstream-req.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,13 @@ fn main() {

let localhost: IpAddr = "127.0.0.1".parse().unwrap();
assert_eq!(client_req.get_client_ip_addr().unwrap(), localhost);

assert_eq!(client_req.get_tls_cipher_openssl_name(), None);
assert_eq!(client_req.get_tls_cipher_openssl_name_bytes(), None);
assert_eq!(client_req.get_tls_client_hello(), None);
assert_eq!(client_req.get_tls_protocol(), None);
assert_eq!(client_req.get_tls_protocol_bytes(), None);
// NOTE: This currently fails, waiting on a patch to land in the fastly crate
// assert_eq!(client_req.get_tls_raw_client_certificate(), None);
assert_eq!(client_req.get_tls_raw_client_certificate_bytes(), None);
}

0 comments on commit 4067d0f

Please sign in to comment.