Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tls_first_implementation #155

Merged
merged 3 commits into from
Feb 2, 2023
Merged

tls_first_implementation #155

merged 3 commits into from
Feb 2, 2023

Conversation

DanielePalaia
Copy link
Contributor

This first implementation of TLS support includes:

  • Integrate the tokio-native-tls library
  • Implements an Environment Helper Class "TlsConfiguration" to manage Tls options in Environment
  • Implements Tls trusting all hostnames and certificates at the moment (just encryption offered)
  • Adding a test on port 5551

@DanielePalaia DanielePalaia force-pushed the tls_first_implementation branch 10 times, most recently from 5708e75 to b852fbd Compare January 26, 2023 15:26
Comment on lines 399 to 405
let conn = tls_builder.build();

let conn = tokio_native_tls::TlsConnector::from(conn.unwrap());

let stream = conn.connect(broker.host.as_str(), stream).await;

GenericTcpStream::SecureTcp(stream.unwrap())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use ? here instead of unwrap()

Suggested change
let conn = tls_builder.build();
let conn = tokio_native_tls::TlsConnector::from(conn.unwrap());
let stream = conn.connect(broker.host.as_str(), stream).await;
GenericTcpStream::SecureTcp(stream.unwrap())
let conn = tokio_native_tls::TlsConnector::from(tls_builder.build()?);
let stream = conn.connect(broker.host.as_str(), stream).await?;
GenericTcpStream::SecureTcp(stream)

Comment on lines +140 to +144
pub struct TlsConfiguration {
pub(crate) enabled: bool,
pub(crate) hostname_verification: bool,
pub(crate) trust_everything: bool,
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the TlsConfiguration probably it's better if we do a builder pattern?

Something like this

pub struct TlsConfigurationBuilder(TlsConfiguration);

impl TlsConfigurationBuilder {

   pub fn trust_everything(mut self, trust_everything: bool)  -> TlsConfigurationBuilder {
        self.0.trust_everything = trust_everything;
        self
    }

    pub fn enable(mut self, enable: bool)  -> TlsConfigurationBuilder {
        self.0.enabled = enabled;
        self
    }


    pub fn hostname_verification_enable(mut self, hostname_verification: bool) -> TlsConfigurationBuilder  {
        self.0.hostname_verification = hostname_verification;
        self
    }

    pub fn build(self) -> TlsConfiguration {
       self.0
    }
}


impl TlsConfiguration {

   pub fn builder() -> TlsConfigurationBulder {
    TlsConfigurationBulder(TlsConfiguration::default())
   }
  
}

@DanielePalaia
Copy link
Contributor Author

Hey @wolf4ood thanks for your review! Addressed the issues!

@wolf4ood wolf4ood merged commit 8a28909 into main Feb 2, 2023
@DanielePalaia DanielePalaia mentioned this pull request Aug 1, 2023
@Gsantomaggio Gsantomaggio deleted the tls_first_implementation branch September 12, 2023 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants