From 4067d0f52c61ae64cb14fbd2bbb012b9d862c0c3 Mon Sep 17 00:00:00 2001 From: Jake Champion Date: Thu, 4 Apr 2024 15:08:10 +0100 Subject: [PATCH] Return a ValueAbsent for all the downstream-tls related functions instead 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 --- lib/src/wiggle_abi/req_impl.rs | 45 +++++++++++++------------ test-fixtures/src/bin/downstream-req.rs | 9 +++++ 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/lib/src/wiggle_abi/req_impl.rs b/lib/src/wiggle_abi/req_impl.rs index 2545916a..7cc4770a 100644 --- a/lib/src/wiggle_abi/req_impl.rs +++ b/lib/src/wiggle_abi/req_impl.rs @@ -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, + _cipher_out: &GuestPtr<'a, u8>, + _cipher_max_len: u32, + _nwritten_out: &GuestPtr, ) -> 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. @@ -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, + _protocol_out: &GuestPtr<'a, u8>, + _protocol_max_len: u32, + _nwritten_out: &GuestPtr, ) -> 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. @@ -201,29 +201,30 @@ impl FastlyHttpReq for Session { chello_max_len: u32, nwritten_out: &GuestPtr, ) -> 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, ) -> 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 { - 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) -> Result { - Err(Error::NotAvailable("Client TLS JA3 hash")) + fn downstream_tls_ja3_md5(&mut self, _ja3_md5_out: &GuestPtr) -> Result { + // 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. diff --git a/test-fixtures/src/bin/downstream-req.rs b/test-fixtures/src/bin/downstream-req.rs index cbc169bb..8515d1fc 100644 --- a/test-fixtures/src/bin/downstream-req.rs +++ b/test-fixtures/src/bin/downstream-req.rs @@ -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); }