Unverified Commit 82d3ac94 authored by Steven Fackler's avatar Steven Fackler Committed by GitHub
Browse files

Merge pull request #795 from sfackler/host-overhaul 

Allow SNI and hostname verification to be configured separately
parents 4c47aca5 25984aa1

openssl/src/dh.rs

+14 −21
Original line number Diff line number Diff line
@@ -95,7 +95,7 @@ mod compat { 
mod tests {

    use dh::Dh;

    use bn::BigNum;

    use ssl::{SslMethod, SslContext};

    use ssl::{SslContext, SslMethod};



    #[test]

    #[cfg(any(all(feature = "v102", ossl102), all(feature = "v110", ossl110)))]
@@ -113,30 +113,23 @@ mod tests { 
    fn test_dh() {

        let mut ctx = SslContext::builder(SslMethod::tls()).unwrap();

        let p = BigNum::from_hex_str(

            "87A8E61DB4B6663CFFBBD19C651959998CEEF608660DD0F25D2CEED4435\

                                      E3B00E00DF8F1D61957D4FAF7DF4561B2AA3016C3D91134096FAA3BF429\

                                      6D830E9A7C209E0C6497517ABD5A8A9D306BCF67ED91F9E6725B4758C02\

                                      2E0B1EF4275BF7B6C5BFC11D45F9088B941F54EB1E59BB8BC39A0BF1230\

                                      7F5C4FDB70C581B23F76B63ACAE1CAA6B7902D52526735488A0EF13C6D9\

                                      A51BFA4AB3AD8347796524D8EF6A167B5A41825D967E144E5140564251C\

                                      CACB83E6B486F6B3CA3F7971506026C0B857F689962856DED4010ABD0BE\

                                      621C3A3960A54E710C375F26375D7014103A4B54330C198AF126116D227\

                                      6E11715F693877FAD7EF09CADB094AE91E1A1597",

            "87A8E61DB4B6663CFFBBD19C651959998CEEF608660DD0F25D2CEED4435E3B00E00DF8F1D61957D4FAF7DF\

             4561B2AA3016C3D91134096FAA3BF4296D830E9A7C209E0C6497517ABD5A8A9D306BCF67ED91F9E6725B47\

             58C022E0B1EF4275BF7B6C5BFC11D45F9088B941F54EB1E59BB8BC39A0BF12307F5C4FDB70C581B23F76B6\

             3ACAE1CAA6B7902D52526735488A0EF13C6D9A51BFA4AB3AD8347796524D8EF6A167B5A41825D967E144E5\

             140564251CCACB83E6B486F6B3CA3F7971506026C0B857F689962856DED4010ABD0BE621C3A3960A54E710\

             C375F26375D7014103A4B54330C198AF126116D2276E11715F693877FAD7EF09CADB094AE91E1A1597",

        ).unwrap();

        let g = BigNum::from_hex_str(

            "3FB32C9B73134D0B2E77506660EDBD484CA7B18F21EF205407F4793A1A0\

                                      BA12510DBC15077BE463FFF4FED4AAC0BB555BE3A6C1B0C6B47B1BC3773\

                                      BF7E8C6F62901228F8C28CBB18A55AE31341000A650196F931C77A57F2D\

                                      DF463E5E9EC144B777DE62AAAB8A8628AC376D282D6ED3864E67982428E\

                                      BC831D14348F6F2F9193B5045AF2767164E1DFC967C1FB3F2E55A4BD1BF\

                                      FE83B9C80D052B985D182EA0ADB2A3B7313D3FE14C8484B1E052588B9B7\

                                      D2BBD2DF016199ECD06E1557CD0915B3353BBB64E0EC377FD028370DF92\

                                      B52C7891428CDC67EB6184B523D1DB246C32F63078490F00EF8D647D148\

                                      D47954515E2327CFEF98C582664B4C0F6CC41659",

            "3FB32C9B73134D0B2E77506660EDBD484CA7B18F21EF205407F4793A1A0BA12510DBC15077BE463FFF4FED\

             4AAC0BB555BE3A6C1B0C6B47B1BC3773BF7E8C6F62901228F8C28CBB18A55AE31341000A650196F931C77A\

             57F2DDF463E5E9EC144B777DE62AAAB8A8628AC376D282D6ED3864E67982428EBC831D14348F6F2F9193B5\

             045AF2767164E1DFC967C1FB3F2E55A4BD1BFFE83B9C80D052B985D182EA0ADB2A3B7313D3FE14C8484B1E\

             052588B9B7D2BBD2DF016199ECD06E1557CD0915B3353BBB64E0EC377FD028370DF92B52C7891428CDC67E\

             B6184B523D1DB246C32F63078490F00EF8D647D148D47954515E2327CFEF98C582664B4C0F6CC41659",

        ).unwrap();

        let q = BigNum::from_hex_str(

            "8CF83642A709A097B447997640129DA299B1A47D1EB3750BA308B0FE64F\

                                      5FBD3",

            "8CF83642A709A097B447997640129DA299B1A47D1EB3750BA308B0FE64F5FBD3",

        ).unwrap();

        let dh = Dh::from_params(p, g, q).unwrap();

        ctx.set_tmp_dh(&dh).unwrap();

openssl/src/pkcs12.rs

+6 −5
Original line number Diff line number Diff line
@@ -147,16 +147,17 @@ impl Pkcs12Builder { 
        password: &str,

        friendly_name: &str,

        pkey: &PKeyRef,

        cert: &X509,

        cert: &X509, // FIXME X509Ref

    ) -> Result<Pkcs12, ErrorStack> {

        unsafe {

            let pass = CString::new(password).unwrap();

            let friendly_name = CString::new(friendly_name).unwrap();

            let pkey = pkey.as_ptr();

            let cert = cert.as_ptr();

            let ca = self.ca.as_ref().map(|ca| ca.as_ptr()).unwrap_or(

                ptr::null_mut(),

            );

            let ca = self.ca

                .as_ref()

                .map(|ca| ca.as_ptr())

                .unwrap_or(ptr::null_mut());

            let nid_key = self.nid_key.as_raw();

            let nid_cert = self.nid_cert.as_raw();

openssl/src/ssl/connector.rs

+73 −64
Original line number Diff line number Diff line
@@ -132,13 +132,9 @@ impl SslConnector { 
        self.configure()?.connect(domain, stream)

    }



    /// Initiates a client-side TLS session on a stream without performing hostname verification.

    ///

    /// # Warning

    ///

    /// You should think very carefully before you use this method. If hostname verification is not

    /// used, *any* valid certificate for *any* site will be trusted for use from any other. This

    /// introduces a significant vulnerability to man-in-the-middle attacks.

    #[deprecated(

        since = "0.9.24",

        note = "use `ConnectConfiguration::verify_hostname` and `ConnectConfiguration::use_server_name_indication` instead")]

    pub fn danger_connect_without_providing_domain_for_certificate_verification_and_server_name_indication<

        S,

    >(
@@ -149,53 +145,80 @@ impl SslConnector { 
        S: Read + Write,

    {

        self.configure()?

            .danger_connect_without_providing_domain_for_certificate_verification_and_server_name_indication(stream)

            .use_server_name_indication(false)

            .verify_hostname(false)

            .connect("", stream)

    }



    /// Returns a structure allowing for configuration of a single TLS session before connection.

    pub fn configure(&self) -> Result<ConnectConfiguration, ErrorStack> {

        Ssl::new(&self.0).map(ConnectConfiguration)

        Ssl::new(&self.0).map(|ssl| ConnectConfiguration { ssl, sni: true, verify_hostname: true })

    }

}



/// A type which allows for configuration of a client-side TLS session before connection.

pub struct ConnectConfiguration(Ssl);

pub struct ConnectConfiguration {

    ssl: Ssl,

    sni: bool,

    verify_hostname: bool,

}



impl ConnectConfiguration {

    #[deprecated(since = "0.9.23",

                 note = "ConnectConfiguration now implements Deref<Target=SslRef>")]

    pub fn ssl(&self) -> &Ssl {

        &self.0

        &self.ssl

    }



    #[deprecated(since = "0.9.23",

                 note = "ConnectConfiguration now implements DerefMut<Target=SslRef>")]

    pub fn ssl_mut(&mut self) -> &mut Ssl {

        &mut self.0

        &mut self.ssl

    }



    /// Initiates a client-side TLS session on a stream.

    /// Configures the use of Server Name Indication (SNI) when connecting.

    ///

    /// The domain is used for SNI and hostname verification.

    pub fn connect<S>(mut self, domain: &str, stream: S) -> Result<SslStream<S>, HandshakeError<S>>

    where

        S: Read + Write,

    {

        self.0.set_hostname(domain)?;

        setup_verify_hostname(&mut self.0, domain)?;



        self.0.connect(stream)

    /// Defaults to `true`.

    pub fn use_server_name_indication(mut self, use_sni: bool) -> ConnectConfiguration {

        self.sni = use_sni;

        self

    }



    /// Initiates a client-side TLS session on a stream without performing hostname verification.

    /// Configures the use of hostname verification when connecting.

    ///

    /// The verification configuration of the connector's `SslContext` is not overridden.

    /// Defaults to `true`.

    ///

    /// # Warning

    ///

    /// You should think very carefully before you use this method. If hostname verification is not

    /// used, *any* valid certificate for *any* site will be trusted for use from any other. This

    /// introduces a significant vulnerability to man-in-the-middle attacks.

    pub fn verify_hostname(mut self, verify_hostname: bool) -> ConnectConfiguration {

        self.verify_hostname = verify_hostname;

        self

    }



    /// Initiates a client-side TLS session on a stream.

    ///

    /// The domain is used for SNI and hostname verification if enabled.

    pub fn connect<S>(mut self, domain: &str, stream: S) -> Result<SslStream<S>, HandshakeError<S>>

    where

        S: Read + Write,

    {

        if self.sni {

            self.ssl.set_hostname(domain)?;

        }



        if self.verify_hostname {

            setup_verify_hostname(&mut self.ssl, domain)?;

        }



        self.ssl.connect(stream)

    }



    #[deprecated(

        since = "0.9.24",

        note = "use `ConnectConfiguration::verify_hostname` and `ConnectConfiguration::use_server_name_indication` instead")]

    pub fn danger_connect_without_providing_domain_for_certificate_verification_and_server_name_indication<

        S,

    >(
@@ -205,7 +228,7 @@ impl ConnectConfiguration { 
    where

        S: Read + Write,

    {

        self.0.connect(stream)

        self.use_server_name_indication(false).verify_hostname(false).connect("", stream)

    }

}
@@ -213,13 +236,13 @@ impl Deref for ConnectConfiguration { 
    type Target = SslRef;



    fn deref(&self) -> &SslRef {

        &self.0

        &self.ssl

    }

}



impl DerefMut for ConnectConfiguration {

    fn deref_mut(&mut self) -> &mut SslRef {

        &mut self.0

        &mut self.ssl

    }

}
@@ -471,7 +494,7 @@ mod verify { 
                }

                Err(_) => {

                    if let Some(pattern) = name.dnsname() {

                        if matches_dns(pattern, domain, false) {

                        if matches_dns(pattern, domain) {

                            return true;

                        }

                    }
@@ -483,26 +506,25 @@ mod verify { 
    }



    fn verify_subject_name(domain: &str, subject_name: &X509NameRef) -> bool {

        if let Some(pattern) = subject_name.entries_by_nid(nid::COMMONNAME).next() {

        match subject_name.entries_by_nid(nid::COMMONNAME).next() {

            Some(pattern) => {

                let pattern = match str::from_utf8(pattern.data().as_slice()) {

                    Ok(pattern) => pattern,

                    Err(_) => return false,

                };



            // Unlike with SANs, IP addresses in the subject name don't have a

            // different encoding. We need to pass this down to matches_dns to

            // disallow wildcard matches with bogus patterns like *.0.0.1

            let is_ip = domain.parse::<IpAddr>().is_ok();



            if matches_dns(&pattern, domain, is_ip) {

                return true;

                // Unlike SANs, IP addresses in the subject name don't have a

                // different encoding.

                match domain.parse::<IpAddr>() {

                    Ok(ip) => pattern.parse::<IpAddr>().ok().map_or(false, |pattern| pattern == ip),

                    Err(_) => matches_dns(pattern, domain),

                }

            }



        false

            None => false,

        }

    }



    fn matches_dns(mut pattern: &str, mut hostname: &str, is_ip: bool) -> bool {

    fn matches_dns(mut pattern: &str, mut hostname: &str) -> bool {

        // first strip trailing . off of pattern and hostname to normalize

        if pattern.ends_with('.') {

            pattern = &pattern[..pattern.len() - 1];
@@ -511,12 +533,12 @@ mod verify { 
            hostname = &hostname[..hostname.len() - 1];

        }



        matches_wildcard(pattern, hostname, is_ip).unwrap_or_else(|| pattern == hostname)

        matches_wildcard(pattern, hostname).unwrap_or_else(|| pattern == hostname)

    }



    fn matches_wildcard(pattern: &str, hostname: &str, is_ip: bool) -> Option<bool> {

        // IP addresses and internationalized domains can't involved in wildcards

        if is_ip || pattern.starts_with("xn--") {

    fn matches_wildcard(pattern: &str, hostname: &str) -> Option<bool> {

        // internationalized domains can't involved in wildcards

        if pattern.starts_with("xn--") {

            return None;

        }
@@ -578,22 +600,9 @@ mod verify { 
    }



    fn matches_ip(expected: &IpAddr, actual: &[u8]) -> bool {

        match (expected, actual.len()) {

            (&IpAddr::V4(ref addr), 4) => actual == addr.octets(),

            (&IpAddr::V6(ref addr), 16) => {

                let segments = [

                    ((actual[0] as u16) << 8) | actual[1] as u16,

                    ((actual[2] as u16) << 8) | actual[3] as u16,

                    ((actual[4] as u16) << 8) | actual[5] as u16,

                    ((actual[6] as u16) << 8) | actual[7] as u16,

                    ((actual[8] as u16) << 8) | actual[9] as u16,

                    ((actual[10] as u16) << 8) | actual[11] as u16,

                    ((actual[12] as u16) << 8) | actual[13] as u16,

                    ((actual[14] as u16) << 8) | actual[15] as u16,

                ];

                segments == addr.segments()

            }

            _ => false,

        match *expected {

            IpAddr::V4(ref addr) => actual == addr.octets(),

            IpAddr::V6(ref addr) => actual == addr.octets(),

        }

    }

}

openssl/src/ssl/tests/mod.rs

+51 −43
Original line number Diff line number Diff line
@@ -6,10 +6,10 @@ use std::io::prelude::*; 
use std::io::{self, BufReader};

use std::iter;

use std::mem;

use std::net::{TcpStream, TcpListener, SocketAddr};

use std::net::{SocketAddr, TcpListener, TcpStream};

use std::path::Path;

use std::process::{Command, Child, Stdio, ChildStdin};

use std::sync::atomic::{AtomicBool, ATOMIC_BOOL_INIT, Ordering};

use std::process::{Child, ChildStdin, Command, Stdio};

use std::sync::atomic::{AtomicBool, Ordering, ATOMIC_BOOL_INIT};

use std::thread;

use std::time::Duration;

use tempdir::TempDir;
@@ -18,10 +18,9 @@ use dh::Dh; 
use hash::MessageDigest;

use ocsp::{OcspResponse, RESPONSE_STATUS_UNAUTHORIZED};

use ssl;

use ssl::{SslMethod, HandshakeError, SslContext, SslStream, Ssl, ShutdownResult,

          SslConnectorBuilder, SslAcceptorBuilder, Error, SSL_VERIFY_PEER, SSL_VERIFY_NONE,

          STATUS_TYPE_OCSP};

use x509::{X509StoreContext, X509, X509Name, X509_FILETYPE_PEM};

use ssl::{Error, HandshakeError, ShutdownResult, Ssl, SslAcceptorBuilder, SslConnectorBuilder,

          SslContext, SslMethod, SslStream, SSL_VERIFY_NONE, SSL_VERIFY_PEER, STATUS_TYPE_OCSP};

use x509::{X509, X509Name, X509StoreContext, X509_FILETYPE_PEM};

#[cfg(any(all(feature = "v102", ossl102), all(feature = "v110", ossl110)))]

use x509::verify::X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS;

use pkey::PKey;
@@ -35,7 +34,7 @@ static CERT: &'static [u8] = include_bytes!("../../../test/cert.pem"); 
static KEY: &'static [u8] = include_bytes!("../../../test/key.pem");



fn next_addr() -> SocketAddr {

    use std::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT, Ordering};

    use std::sync::atomic::{AtomicUsize, Ordering, ATOMIC_USIZE_INIT};

    static PORT: AtomicUsize = ATOMIC_USIZE_INIT;

    let port = 15411 + PORT.fetch_add(1, Ordering::SeqCst);
@@ -104,15 +103,13 @@ impl Server { 


    #[allow(dead_code)]

    fn new_alpn() -> (Server, TcpStream) {

        Server::new_tcp(

            &[

        Server::new_tcp(&[

            "-www",

            "-nextprotoneg",

            "http/1.1,spdy/3.1",

            "-alpn",

            "http/1.1,spdy/3.1",

            ],

        )

        ])

    }

}
@@ -157,10 +154,9 @@ macro_rules! run_test( 
    );

);



run_test!(

    new_ctx,

    |method, _| { SslContext::builder(method).unwrap(); }

);

run_test!(new_ctx, |method, _| {

    SslContext::builder(method).unwrap();

});



run_test!(verify_untrusted, |method, stream| {

    let mut ctx = SslContext::builder(method).unwrap();
@@ -309,7 +305,7 @@ run_test!(verify_callback_data, |method, stream| { 
});



run_test!(ssl_verify_callback, |method, stream| {

    use std::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT, Ordering};

    use std::sync::atomic::{AtomicUsize, Ordering, ATOMIC_USIZE_INIT};



    static CHECKED: AtomicUsize = ATOMIC_USIZE_INIT;
@@ -437,9 +433,9 @@ fn test_read() { 
    let mut stream = Ssl::new(&ctx.build()).unwrap().connect(tcp).unwrap();

    stream.write_all("GET /\r\n\r\n".as_bytes()).unwrap();

    stream.flush().unwrap();

    io::copy(&mut stream, &mut io::sink()).ok().expect(

        "read error",

    );

    io::copy(&mut stream, &mut io::sink())

        .ok()

        .expect("read error");

}



#[test]
@@ -994,9 +990,8 @@ fn verify_valid_hostname() { 
    ctx.set_verify(SSL_VERIFY_PEER);



    let mut ssl = Ssl::new(&ctx.build()).unwrap();

    ssl.param_mut().set_hostflags(

        X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS,

    );

    ssl.param_mut()

        .set_hostflags(X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS);

    ssl.param_mut().set_host("google.com").unwrap();



    let s = TcpStream::connect("google.com:443").unwrap();
@@ -1019,9 +1014,8 @@ fn verify_invalid_hostname() { 
    ctx.set_verify(SSL_VERIFY_PEER);



    let mut ssl = Ssl::new(&ctx.build()).unwrap();

    ssl.param_mut().set_hostflags(

        X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS,

    );

    ssl.param_mut()

        .set_hostflags(X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS);

    ssl.param_mut().set_host("foobar.com").unwrap();



    let s = TcpStream::connect("google.com:443").unwrap();
@@ -1057,7 +1051,12 @@ fn connector_invalid_no_hostname_verification() { 
    let connector = SslConnectorBuilder::new(SslMethod::tls()).unwrap().build();



    let s = TcpStream::connect("google.com:443").unwrap();

    connector.danger_connect_without_providing_domain_for_certificate_verification_and_server_name_indication(s)

    connector

        .configure()

        .unwrap()

        .use_server_name_indication(false)

        .verify_hostname(false)

        .connect("foobar.com", s)

        .unwrap();

}
@@ -1067,8 +1066,14 @@ fn connector_no_hostname_still_verifies() { 


    let connector = SslConnectorBuilder::new(SslMethod::tls()).unwrap().build();



    assert!(connector.danger_connect_without_providing_domain_for_certificate_verification_and_server_name_indication(tcp)

        .is_err());

    assert!(

        connector

            .configure()

            .unwrap()

            .verify_hostname(false)

            .connect("fizzbuzz.com", tcp)

            .is_err()

    );

}



#[test]
@@ -1079,7 +1084,12 @@ fn connector_no_hostname_can_disable_verify() { 
    connector.set_verify(SSL_VERIFY_NONE);

    let connector = connector.build();



    connector.danger_connect_without_providing_domain_for_certificate_verification_and_server_name_indication(tcp).unwrap();

    connector

        .configure()

        .unwrap()

        .verify_hostname(false)

        .connect("foobar.com", tcp)

        .unwrap();

}



#[test]
@@ -1101,9 +1111,7 @@ fn connector_client_server_mozilla_intermediate() { 
    });



    let mut connector = SslConnectorBuilder::new(SslMethod::tls()).unwrap();

    connector

        .set_ca_file("test/root-ca.pem")

        .unwrap();

    connector.set_ca_file("test/root-ca.pem").unwrap();

    let connector = connector.build();



    let stream = TcpStream::connect(("127.0.0.1", port)).unwrap();
@@ -1135,9 +1143,7 @@ fn connector_client_server_mozilla_modern() { 
    });



    let mut connector = SslConnectorBuilder::new(SslMethod::tls()).unwrap();

    connector

        .set_ca_file("test/root-ca.pem")

        .unwrap();

    connector.set_ca_file("test/root-ca.pem").unwrap();

    let connector = connector.build();



    let stream = TcpStream::connect(("127.0.0.1", port)).unwrap();
@@ -1239,7 +1245,8 @@ fn tmp_dh_callback() { 
}



#[test]

#[cfg(any(all(feature = "v101", ossl101, not(any(libressl261, libressl262, libressl26x))), all(feature = "v102", ossl102)))]

#[cfg(any(all(feature = "v101", ossl101, not(any(libressl261, libressl262, libressl26x))),

          all(feature = "v102", ossl102)))]

fn tmp_ecdh_callback() {

    use ec::EcKey;

    use nid;
@@ -1306,7 +1313,8 @@ fn tmp_dh_callback_ssl() { 
}



#[test]

#[cfg(any(all(feature = "v101", ossl101, not(any(libressl261, libressl262, libressl26x))), all(feature = "v102", ossl102)))]

#[cfg(any(all(feature = "v101", ossl101, not(any(libressl261, libressl262, libressl26x))),

          all(feature = "v102", ossl102)))]

fn tmp_ecdh_callback_ssl() {

    use ec::EcKey;

    use nid;

test/add_target.sh

+1 −1
Original line number Diff line number Diff line 
#!/bin/bash

set -e

set -eux



case "${TARGET}" in

"x86_64-unknown-linux-gnu")