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

Bug: reqwest can send an invalid TLS SNI extension, by using IP in the host name #1328

Closed
Alvenix opened this issue Sep 6, 2021 · 6 comments

Comments

@Alvenix
Copy link
Contributor

Alvenix commented Sep 6, 2021

This get rejected by servers based on rustls as can be seen here. From this issue, @ctz says this is not permitted by the standard.

RFC3546 describes the SNI extension, and says "Literal IPv4 and IPv6 addresses are not permitted in "HostName"."

Below I provide how to reproduce the bug.

Client src/main.rs

use std::error::Error;

fn main() -> Result<(), Box<dyn Error>> {
    let client = reqwest::blocking::ClientBuilder::new()
        .danger_accept_invalid_certs(true)
        .build()?;

    // This works
    client.get("https://localhost:8000").send()?.text()?;

    // This does not work
    client.get("https://127.0.0.1:8000").send()?.text()?;

    Ok(())
}

Cargo.toml

[package]
name = "test_reqwest_bug"
version = "0.1.0"
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
reqwest = { version = "0.11.4", features = ["blocking"] }

To run a sample server locally

$ git clone https://github.com/SergioBenitez/Rocket
$ cd Rocket/examples/tls
$ cargo run

The server output:

Error: connection accept error: received corrupt message of type Handshake
Error: optimistically retrying now

The client output:

Error: reqwest::Error { kind: Request, url: Url { scheme: "https", cannot_be_a_base: false, username: "", password: None, host: Some(Ipv4(127.0.0.1)), port: Some(8000), path: "/", query: None, fragment: None }, source: hyper::Error(Connect, Ssl(Error { code: ErrorCode(1), cause: Some(Ssl(ErrorStack([Error { code: 336151578, library: "SSL routines", function: "ssl3_read_bytes", reason: "tlsv1 alert decode error", file: "../ssl/record/rec_layer_s3.c", line: 1543, data: "SSL alert number 50" }]))) }, X509VerifyResult { code: 0, error: "ok" })) }

To check for the SNI extension, inside tls folder from rocket run:

openssl s_server -cert private/ca_cert.pem -key private/ca_key.pem -port 8000 -tlsextdebug

Relevant Output:

TLS client extension "server name" (id=0), len=14
0000 - 00 0c 00 00 09 31 32 37-2e 30 2e 30 2e 31         .....127.0.0.1
@Alvenix Alvenix changed the title Bug: reqwest can send an invalid SNI extension, by using IP in the host name Bug: reqwest can send an invalid TLS SNI extension, by using IP in the host name Sep 6, 2021
@Lut99
Copy link

Lut99 commented Nov 4, 2022

Is there any progress on this? I'm pretty sure I'm running into the same issue...

I have a client that runs reqwest connecting to a server that uses rustls for TLS. When I use IP addresses (with matching certificates) to connect to the server, I get "invalid SNI" errors - but if I use hostnames, it works.

It would be really great if there was some fix or workaround for this, since I can't use hostnames for all the domains in my project - and moreover, testing locally is a nightmare since I'm using Docker containers (which don't use the host's /etc/hosts file, so simply adding a shortcut there doesn't really work...).

For what it's worth, I'm using the rustls backend in reqwest, so it seems it's not an issue of the native_tls backend.

@Alvenix
Copy link
Contributor Author

Alvenix commented Nov 4, 2022

Is there any progress on this? I'm pretty sure I'm running into the same issue...

I have a client that runs reqwest connecting to a server that uses rustls for TLS. When I use IP addresses (with matching certificates) to connect to the server, I get "invalid SNI" errors - but if I use hostnames, it works.

It would be really great if there was some fix or workaround for this, since I can't use hostnames for all the domains in my project - and moreover, testing locally is a nightmare since I'm using Docker containers (which don't use the host's /etc/hosts file, so simply adding a shortcut there doesn't really work...).

For what it's worth, I'm using the rustls backend in reqwest, so it seems it's not an issue of the native_tls backend.

If I am remembering correctly, the current workaround is to do the following:

-use rustls feature of reqwest (make sure to disable default features or force use of rustls by API)
-Make sure you are using latest version of rustls (Update your Cargo.lock file)
-Set danger_accept_invalid_certs to true

but then you are effectively ignoring certificate validation.

@Lut99
Copy link

Lut99 commented Nov 4, 2022

If I am remembering correctly, the current workaround is to do the following:

-use rustls feature of reqwest (make sure to disable default features or force use of rustls by API)
-Make sure you are using latest version of rustls (Update your Cargo.lock file)
-Set danger_accept_invalid_certs to true

but then you are effectively ignoring certificate validation.

I see, thanks! I might be able to use that for local testing :)

But since this indeed disables host verification, I won't be able to use that when trying to connect to the domains which only have an IP - so I'll keep looking for a solution for that. Maybe that I can do something with a custom DNS resolver, if that doesn't prove too hard to implement myself (and if reqwest supports that). If I manage, I'll post it here, just for future reference.

Thanks!

@Alvenix
Copy link
Contributor Author

Alvenix commented Nov 4, 2022

If I am remembering correctly, the current workaround is to do the following:
-use rustls feature of reqwest (make sure to disable default features or force use of rustls by API)
-Make sure you are using latest version of rustls (Update your Cargo.lock file)
-Set danger_accept_invalid_certs to true
but then you are effectively ignoring certificate validation.

I see, thanks! I might be able to use that for local testing :)

But since this indeed disables host verification, I won't be able to use that when trying to connect to the domains which only have an IP - so I'll keep looking for a solution for that. Maybe that I can do something with a custom DNS resolver, if that doesn't prove too hard to implement myself (and if reqwest supports that). If I manage, I'll post it here, just for future reference.

Thanks!

I think native-tls works with IPs out of box if the other end is not based on rustls or something with the same problem..

@Lut99
Copy link

Lut99 commented Nov 7, 2022

If I am remembering correctly, the current workaround is to do the following:
-use rustls feature of reqwest (make sure to disable default features or force use of rustls by API)
-Make sure you are using latest version of rustls (Update your Cargo.lock file)
-Set danger_accept_invalid_certs to true
but then you are effectively ignoring certificate validation.

I see, thanks! I might be able to use that for local testing :)
But since this indeed disables host verification, I won't be able to use that when trying to connect to the domains which only have an IP - so I'll keep looking for a solution for that. Maybe that I can do something with a custom DNS resolver, if that doesn't prove too hard to implement myself (and if reqwest supports that). If I manage, I'll post it here, just for future reference.
Thanks!

I think native-tls works with IPs out of box if the other end is not based on rustls or something with the same problem..

Well, the other end is based on rustls, so maybe that doesn't improve that much... But thanks for the suggestion nonetheless :)

On that note - thanks even more for creating a pull request with the workaround! I'm excited to see this merged :D

@Alvenix
Copy link
Contributor Author

Alvenix commented Apr 1, 2023

Closed as it was fixed in rust-openssl (which is used by native tls).

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

2 participants