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

Make SSL cert verification errors work again. Add a horrible, no-good… #6935

Merged
merged 1 commit into from Nov 13, 2015
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Make SSL cert verification errors work again. Add a horrible, no-good…

…, very bad regression test.
  • Loading branch information
jdm committed Nov 13, 2015
commit 5e123df7a77e8d973b6c7a8c8a13faa47aa90a70
@@ -27,6 +27,7 @@ use msg::constellation_msg::{PipelineId};
use net_traits::ProgressMsg::{Done, Payload};
use net_traits::hosts::replace_hosts;
use net_traits::{CookieSource, IncludeSubdomains, LoadConsumer, LoadData, Metadata};
use openssl::ssl::error::{SslError, OpensslError};
use openssl::ssl::{SSL_VERIFY_PEER, SslContext, SslMethod};
use resource_task::{CancellationListener, send_error, start_sending_sniffed_opt};
use std::borrow::ToOwned;
@@ -210,29 +211,21 @@ impl HttpRequestFactory for NetworkHttpRequestFactory {
fn create(&self, url: Url, method: Method) -> Result<WrappedHttpRequest, LoadError> {
let connection = Request::with_connector(method, url.clone(), &*self.connector);

let ssl_err_string = "Some(OpenSslErrors([UnknownError { library: \"SSL routines\", \
function: \"SSL3_GET_SERVER_CERTIFICATE\", \
reason: \"certificate verify failed\" }]))";
if let Err(HttpError::Ssl(ref error)) = connection {
let error: &(Error + Send + 'static) = &**error;
if let Some(&SslError::OpenSslErrors(ref errors)) = error.downcast_ref::<SslError>() {
if errors.iter().any(is_cert_verify_error) {
return Err(
LoadError::Ssl(url, format!("ssl error: {:?} {:?}",
error.description(),
error.cause())));
}
}
}

let request = match connection {
Ok(req) => req,

Err(HttpError::Io(ref io_error)) if (
io_error.kind() == io::ErrorKind::Other &&
io_error.description() == "Error in OpenSSL" &&
// FIXME: This incredibly hacky. Make it more robust, and at least test it.
format!("{:?}", io_error.cause()) == ssl_err_string
) => {
return Err(
LoadError::Ssl(
url,
format!("ssl error {:?}: {:?} {:?}",
io_error.kind(),
io_error.description(),
io_error.cause())
)
)
},
Err(e) => {
return Err(LoadError::Connection(url, e.description().to_owned()))
}
@@ -756,3 +749,14 @@ fn send_data<R: Read>(reader: &mut R,

let _ = progress_chan.send(Done(Ok(())));
}

// FIXME: This incredibly hacky. Make it more robust, and at least test it.
fn is_cert_verify_error(error: &OpensslError) -> bool {
match error {

This comment has been minimized.

Copy link
@SimonSapin

SimonSapin Aug 3, 2015

Member

Nit: you can use let instead of match here since there is only one variant.

(This is still hacky, but I feel much better about this than what we could do before Error::downcast_ref was added.)

&OpensslError::UnknownError { ref library, ref function, ref reason } => {
library == "SSL routines" &&
function == "SSL3_GET_SERVER_CERTIFICATE" &&
reason == "certificate verify failed"
}
}
}
@@ -4559,6 +4559,12 @@
"url": "/_mozilla/mozilla/MouseEvent.html"
}
],
"mozilla/bad_cert_detected.html": [
{
"path": "mozilla/bad_cert_detected.html",
"url": "/_mozilla/mozilla/bad_cert_detected.html"
}
],
"mozilla/blob.html": [
{
"path": "mozilla/blob.html",
@@ -9622,4 +9628,4 @@
"rev": null,
"url_base": "/_mozilla/",
"version": 2
}
}
@@ -0,0 +1,28 @@
<html>
<head>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="resources/origin_helpers.js?pipe=sub"></script>
</head>
<body>
<script>
var t = async_test("Invalid SSL cert noticed");
t.step(function() {
var target = location.href.replace(HTTP_ORIGIN, HTTPS_ORIGIN)
.replace('bad_cert_detected.html',
'resources/origin_helpers.js');
// Servo currently lacks the ability to introspect any content that is blocked
// due to a cert error, so we use a roundabout method to infer that that's happened.
// When the worker has a cert failure, that translates into attempting to evaluate the
// contents of badcert.html as JS, which triggers an exception that currently does not
// propagate to the parent scope. If we _do_ get an error event in the parent scope,
// that means that the cert verification was treated no different than any other
// network error, since we dispatch an error event in that case.
var w = new Worker(target);
w.addEventListener('error', t.unreached_func("cert not detected as invalid"), false);
// We infer that we detected an invalid cert if nothing happens for a few seconds.
setTimeout(function() { t.done() }, 3000);
});
</script>
</body>
</html>
@@ -0,0 +1,5 @@
var HTTP_PORT = '{{ports[http][0]}}';
var HTTPS_PORT = '{{ports[https][0]}}';
var ORIGINAL_HOST = '\'{{host}}\'';
var HTTP_ORIGIN = 'http://' + ORIGINAL_HOST + ':' + HTTP_PORT;
var HTTPS_ORIGIN = 'https://' + ORIGINAL_HOST + ':' + HTTPS_PORT;
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.