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

Openssl::with_cert_and_key certificate verification #752

Closed
jimmycuadra opened this issue Mar 29, 2016 · 6 comments
Closed

Openssl::with_cert_and_key certificate verification #752

jimmycuadra opened this issue Mar 29, 2016 · 6 comments

Comments

@jimmycuadra
Copy link

I noticed that Openssl::with_cert_and_key disables certificate verification, shown below for convenience:

pub fn with_cert_and_key<C, K>(cert: C, key: K) -> Result<Openssl, SslError>
where C: AsRef<Path>, K: AsRef<Path> {
    let mut ctx = try!(SslContext::new(SslMethod::Sslv23));
    try!(ctx.set_cipher_list("DEFAULT"));
    try!(ctx.set_certificate_file(cert.as_ref(), X509FileType::PEM));
    try!(ctx.set_private_key_file(key.as_ref(), X509FileType::PEM));
    ctx.set_verify(SSL_VERIFY_NONE, None);
    Ok(Openssl { context: Arc::new(ctx) })
}

I'm guessing this is done because the expected use is for a server accepting connections from clients without client certificates. However, if someone uses this method for setting up a client to present a client certificate to a server, this will disable verification of the server's certificate.

Is my understanding here correct? If so, I think there should be two separate methods depending on whether it's intended for client or server use. At the very least, the documentation for the current method should make it very clear that it's not safe to use on the client side when using client certificates.

@seanmonstar
Copy link
Member

Yes, your guess is correct. I assumed most people wanting an easy constructor with a cert and key were using it for a server, not a client.

@frewsxcv
Copy link
Contributor

frewsxcv commented Apr 6, 2016

Related to #472

@frewsxcv
Copy link
Contributor

If so, I think there should be two separate methods depending on whether it's intended for client or server use.

Ever since #757, there are now separate traits for server and client TLS use. That said, there still needs to exist a crate that actually verifies the certificates for this to be completed.

In my opinion, this issue can be closed as a duplicate of #472.

@jimmycuadra
Copy link
Author

jimmycuadra commented Apr 17, 2016

Isn't hostname verification different than certificate verification?

@sfackler
Copy link
Contributor

The ssl feature has been removed, so I believe this issue can be closed.

@seanmonstar
Copy link
Member

Indeed, SSL was removed, see #985.

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

No branches or pull requests

4 participants