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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Connection reset by peer #1297

Closed
dowusu opened this issue May 16, 2023 · 31 comments
Closed

Connection reset by peer #1297

dowusu opened this issue May 16, 2023 · 31 comments

Comments

@dowusu
Copy link

dowusu commented May 16, 2023

Hi 馃憢 , so I combined a client key and client certificate with cat to be able to use it with rustls, when I use nativetls with the individual files, it works but when I combine them to use with rustls via reqwest, below error is generated

source: hyper::Error(Connect, Ssl(Error { code: ErrorCode(5), cause: Some(Io(Os { code: 104, kind: ConnectionReset, message: "Connection reset by peer" })) }, X509VerifyResult { code: 0, error: "ok" })) })

   let buf = fs::read(config.root_cacert)?;
    let cacert = reqwest::Certificate::from_pem(&buf)?;

    //let cert = fs::read(config.client_cert)?;
    let key = fs::read(config.client_key)?;
    let id = reqwest::Identity::from_pem(&key)?;

    let client = reqwest::ClientBuilder::new()
        .default_headers(headers)
        .timeout(Duration::from_secs(10))
        .add_root_certificate(cacert)
        .identity(id)
        .build()?;

I tried this on MacOs and Rocky Linux

@djc
Copy link
Member

djc commented May 16, 2023

Sounds interesting, but I'd need a bunch more information in order to check if there's an issue in rustls.

For my understanding: the error looks like it's using native-tls for the client, is it connecting to a rustls server (if you know what TLS stack the server is running)?

Ideally, can you give me an example client key and certificate generated using the process you used here? (Make sure not to send me any key material that you're using for things on the internet.)

It might also be useful if you can attach a packet capture of the connection. We'd probably need a plaintext capture, so you'll want to set up an SSLKEYLOGFILE.

@dowusu
Copy link
Author

dowusu commented May 17, 2023

By TLS stack, I assume you're referring to TLS details "SSL connection using TLSv1.2 / ECDHE-RSA-AES128-GCM-SHA256" - obtained via curl. Unfortunately the key material is meant for production.

WRT to the SSLKEYLOGFILE, my experience with other libraries & curl indicates it's a feature of the library or tool. I tried setting up rustls key log but run out of luck, sample snippet is outdated.

@jsha
Copy link
Contributor

jsha commented May 17, 2023

@dowusu TLS stack means what TLS library is the server using. OpenSSL? Rustls? Something else?

Also, it sounds reasonably likely the server is rejecting your connection, possibly because the client certificate being sent is incorrect in some way. Do you have access to the server logs to see if it states any reason for rejecting connections?

I notice in your example code that you're reading the key and cert from config.client_key; are you sure that's where you've put the concatenated files? And in what order have you concatenated them?

@dowusu
Copy link
Author

dowusu commented May 18, 2023

I can safely assume OpenSSL. I don't have access to the server/server logs, it's a third party server. The config.client_key points to the concatenated file read from the config.

I ran curl with option -E to test the concatenated files and it works curl --cacert <ca_cert> -E <concatenated_file>

I intended changing reqwest to use native tls with the concatenated file to see how it pans out; I realised the function is only available for rustls feature unless I convert it to pkcs#12.

@jakswa
Copy link

jakswa commented Jun 23, 2023

In case an example domain/URL is what's missing in this thread, I am seeing similar behavior with rustls + reqwest only when hitting https://itsmarta.com/google_transit_feed/google_transit.zip (so far).

I've verified locally and on remote VPS so my guess is it's some wonky servers causing this?

Error: reqwest::Error { kind: Request, url: Url { scheme: "https", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("itsmarta.com")), port: None, path: "/google_transit_feed/google_transit.zip", query: None, fragment: None }, source: hyper::Error(Connect, Custom { kind: Other, error: Os { code: 104, kind: ConnectionReset, message: "Connection reset by peer" } }) }

This request succeeds if i change my Cargo.toml like:

- reqwest = { version = "0.11.8", default-features=false, features=["rustls-tls"]}
+ reqwest = { version = "0.11.8" }

@jakswa
Copy link

jakswa commented Jun 23, 2023

I've created a simple reproduction based off the rust cookbook for HTTP/reqwest to show this behavior. All you have to do is swap the rustls-tls piece in Cargo.toml in and out to witness the change:

https://gist.github.com/jakswa/866778bd6d207a102c3b85153212e102

Here's the piece you change:

reqwest = { version = "0.11.8", default-features=false, features=["rustls-tls", "blocking"]}

# succeeds if you change it to:
# reqwest = { version = "0.11.8", features=["blocking"]}

@jsha
Copy link
Contributor

jsha commented Jun 23, 2023

Thanks for the example, @jakswa ! According to https://www.ssllabs.com/ssltest/analyze.html?d=itsmarta.com, the only "strong" cipher suites offered by that server are:

TLS_DHE_RSA_WITH_AES_256_GCM_SHA384
TLS_DHE_RSA_WITH_AES_128_GCM_SHA256

But neither of those is supported by rustls:

https://docs.rs/rustls/latest/src/rustls/suites.rs.html#125-143

So I suspect itsmarta.com is resetting the connection because it does not support any of the cipher suites that rustls offered in the Client Hello.

The main fix is that the server should implement some of the suggestions from SSL Labs, like removing weak cipher suites and enabling some other strong ones.

There are a variety of reasons a server might issue a connection reset, so there's no guarantee that @dowusu's problem is the same, but your example is a good reminder: @dowusu's problem might not be related to client certificates at all, but might just be that the server doesn't support cipher suites (or TLS versions) that overlap with what rustls supports.

@dowusu are you able to run your affected site through the SSL Server Test I linked above? Can you share the hostname and port?

@jakswa
Copy link

jakswa commented Jun 23, 2023

So I suspect itsmarta.com is resetting the connection because it does not support any of the cipher suites that rustls offered in the Client Hello.

Thank you, I didn't know how to dig as deep as you just did but it makes sense. This server looks to be running a Microsoft IIS version from 2013 馃槱. Consider my mystery solved!

@cpu
Copy link
Member

cpu commented Jul 4, 2023

I think until we hear more from the original poster about the results of testing the affected site w/ SSL labs we can't take any action on this issue.

I'm going to close it for now and we can revisit if there is new information. Thanks!

@cpu cpu closed this as completed Jul 4, 2023
@dowusu
Copy link
Author

dowusu commented Jul 20, 2023

Sincere apologies, not sure of how I missed the comments. I checked the endpoint using the ssllabs.com, site supports only TLS1.2 with two strong cipher suites TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 & TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 being supported by rustls.

@djc djc reopened this Jul 24, 2023
@cpu
Copy link
Member

cpu commented Aug 2, 2023

@dowusu Can you share the hostname and port you're testing against?

@dowusu
Copy link
Author

dowusu commented Aug 8, 2023

api.elevygh.org, default port 443.

@cpu
Copy link
Member

cpu commented Aug 8, 2023

api.elevygh.org, default port 443.

I notice that when I try to reproduce with tlsclient-mio and no client certificate offer, I see the connection reset by peer you observe:

cargo run --package rustls-examples --bin tlsclient-mio -- -p 443 --http api.elevygh.org
...
TLS read error: Os { code: 104, kind: ConnectionReset, message: "Connection reset by peer" }
Connection closed

The pcap shows the remote peer sending a TCP reset immediately after the client finishes its portion of the handshake. Potentially this is a firewall? It's not sending any TLS level alert or close notification.

When I try to send a client certificate (just a random cert from our test infrastructure), I get a HTTP level 400 bad request result:

cargo run --package rustls-examples --bin tlsclient-mio -- -p 443 --http api.elevygh.org --auth-key ./test-ca/ecdsa/client.key --auth-certs ./test-ca/ecdsa/client.cert
...
HTTP/1.0 400 Bad Request
apigw-requestid: JW+RAmGPDoEEK6g=
content-length: 25
date: Tue, 08 Aug 2023 20:49:48 GMT

{"message":"Bad Request"}Connection closed

To me this indicates the issue is with the reqwest client configuration you're setting up, and that it isn't a ciphersuite mismatch.

@dowusu As a next step, could you try using the tlsclient-mio example I shared above, but change --auth-key and --auth-certs to a RSA or ECDSA client key and associated client certificate that the server trusts? If you find you can make a successful request that way it would confirm my suspicion and we can figure out the right reqwest configuration to match what tlsclient-mio is doing.

@dowusu
Copy link
Author

dowusu commented Aug 10, 2023

It looks like my reqwest configuration might be wrong, below is the output from from tlsclient-mio. I took a tcpdump as well.
HTTP/1.0 400 Bad Request
apigw-requestid: JddUWkUdjoEEJww=
content-length: 25
date: Thu, 10 Aug 2023 20:03:13 GMT

{"message":"Bad Request"}Connection closed

image

So when I use the certificate separate, it works fine

 let buf = fs::read(config.root_cacert)?;
    let cacert = reqwest::Certificate::from_pem(&buf)?;

        
    let cert = fs::read(config.client_cert)?;
    let key = fs::read(config.client_key)?;


    //let id = reqwest::Identity::from_pem(&key)?;
   
    let id = reqwest::Identity::from_pkcs8_pem(&cert, &key)?;

However if I combine the certificate as indicated in my initial submission, that's when the error occurs.

@cpu
Copy link
Member

cpu commented Aug 10, 2023

It looks like my reqwest configuration might be wrong, below is the output from from tlsclient-mio

Aha, great. We're narrowing in on the problem then :-)

So when I use the certificate separate, it works fine
...
let id = reqwest::Identity::from_pkcs8_pem(&cert, &key)?;

I think that might be using the native-tls backend? I see from_pkcs8_pem as gated by #[cfg(feature = "native_tls")].

@dowusu
Copy link
Author

dowusu commented Aug 10, 2023

Yeah, with native_tls, however when I combine the cert and use rustls, that's when I run into connectivity issue. The native tls means I would have to depend on openssl, something I'm trying to avoid.

@cpu
Copy link
Member

cpu commented Aug 11, 2023

@dowusu I think the issue is likely related to the client certificate and key you're combining to give to reqwest::Identity::from_pem.

I was able to set up a tlserver-mio instance that required mTLS client authentication and connect to it using a Reqwest client using Rustls and a combined identity file created with cat key.pem cert.pem > combined.pem that I fed through reqwest::Identity::from_pem as in your example.

Can you speak to how you combined the files? Did you order it private key and then certificate? Are the input files both PEM encoded? Are you using an RSA client certificate/private key, or a different algorithm?

@dowusu
Copy link
Author

dowusu commented Aug 14, 2023

I combined the private key "-----BEGIN PRIVATE KEY-----" which was converted from PKCS#1 to PKCS#8 and combined with the certificate X509 "-----BEGIN CERTIFICATE-----" using cat private_key certificate > chain.pem which resulted in the below error source: hyper::Error(Connect, Error { code: -9806, message: "connection closed via error".
I printed chain.pem to confirm it's been read.

image
Above is a tcpdump screenshot. Maybe I can close this issue whiles I read around to understand exactly what I'm doing wrong.

@ctz
Copy link
Member

ctz commented Aug 15, 2023

Above is a tcpdump screenshot

It feels very suspicious that your client's Certificate message is only 78 bytes. That is really too short to contain a certificate. I would dig into that message to see precisely what it does contain?

@dowusu
Copy link
Author

dowusu commented Aug 15, 2023

Certificate message is only 78 bytes

Thank you for the observation, I had to explicitly add use_rustls_tls to the client builder to get the certificate working. I was of the belief that enabling rustls feature via reqwest = { version = "0.11", features = ["json", "rustls-tls"] } should suffice without any additional configuration on the client builder (please correct me)

   let buf = fs::read(config.root_cacert)?;
   let cacert = reqwest::Certificate::from_pem(&buf)?;

   let key = fs::read(config.client_key)?;
   let id = reqwest::Identity::from_pem(&key)?;

   let client = reqwest::ClientBuilder::new()
        .use_rustls_tls() //- Had to add this line
        .default_headers(headers)
        .timeout(Duration::from_secs(10))
        .add_root_certificate(cacert)
        .identity(id)
        .build()?;

Thank you for the support.

@ctz ctz closed this as completed Aug 15, 2023
@djc
Copy link
Member

djc commented Aug 15, 2023

This really sounds like a bug in request, I recommend filing an issue there.

@cpu
Copy link
Member

cpu commented Aug 15, 2023

Glad you figured it out :-)

This really sounds like a bug in request, I recommend filing an issue there.

+1 - it seems problematic that your desired config ends up dropped on the floor and no client cert is sent.

For what it's worth, when I was using reqwest successfully to test your problem the code I used was:

use std::time::Duration;

#[tokio::main]
async fn main() -> Result<(), reqwest::Error> {
    let buf = include_bytes!("../minica.pem").as_slice();
    let cacert = reqwest::Certificate::from_pem(&buf)?;

    let client_ident_pem = include_bytes!("../combined.pem").as_slice();
    let id = reqwest::Identity::from_pem(&client_ident_pem)?;

    let client = reqwest::ClientBuilder::new()
        .timeout(Duration::from_secs(30))
        .add_root_certificate(cacert)
        .identity(id)
        .build()?;

    client.get("https://api.elevygh.org:4443/").send().await?;

    Ok(())
}

I didn't need to include use_rustls_tls() on the builder and I suspect it's because I had specified only the rustls-tls feature of reqwest and not any default features

reqwest = { version = "0.11.18", features = ["rustls-tls"], default-features = false }

Perhaps the bug is specific to having multiple TLS backends enabled?

@dowusu
Copy link
Author

dowusu commented Aug 15, 2023

Turning off default-features = false and removing the explicit use_rustls_tls() works. Per the doc default-tls is enabled which I suspect might be the root cause of the error.

@djc
Copy link
Member

djc commented Aug 16, 2023

To be clear, it still seems like a bug (which would be worth reporting) that you set an Identity and it was effectively ignored (or mangled) because two TLS providers were enabled at the same time.

@ctz
Copy link
Member

ctz commented Aug 16, 2023

I'd agree with that.

I'm wondering whether this would have been quicker to debug if we had a warn!-level log when we reply to a CertificateRequest with an empty "I don't have a certificate" Certificate message? Because that's not an error, but it does usually point to a misconfiguration.

@cpu
Copy link
Member

cpu commented Aug 16, 2023

I'm going to work on taking this upstream and will consider the warn! log on our side in parallel.

@cpu
Copy link
Member

cpu commented Aug 16, 2023

After looking closer it turns out there's an existing upstream issue: seanmonstar/reqwest#903 and a proposed fix: seanmonstar/reqwest#1852

edit: I added a comment to 903 and pinged on 1852 to see if there's anything I can do to help land a fix.

@cpu
Copy link
Member

cpu commented Aug 16, 2023

I'm wondering whether this would have been quicker to debug if we had a warn!-level log when we reply to a CertificateRequest with an empty "I don't have a certificate" Certificate message? Because that's not an error, but it does usually point to a misconfiguration.

Running a tlsserver-mio binary that requires client auth with --verbose I do see an error being logged:

[2023-08-16T12:58:12Z ERROR tlsserver_mio] cannot process packet: NoCertificatesPresented

There's also existing debug traces for the case where client auth is optional but the peer presents no certificate:

debug!("client auth requested but no certificate supplied");

debug!("client auth requested but no certificate supplied");

Given that we log the Error condition for the mandatory auth case I think there's probably not much value to adding an additional debug! to those codepaths.

@ctz
Copy link
Member

ctz commented Aug 16, 2023

I mean adding the logging to the client, not the server.

@cpu
Copy link
Member

cpu commented Aug 16, 2023

I mean adding the logging to the client, not the server.

Ah, I don't think that would have helped in this case because the code path that sends the empty certificate message in reqwest with the problematic configuration is using the native-tls backend.

@ctz
Copy link
Member

ctz commented Aug 16, 2023

Ah! Sorry, I misunderstood. I though the identity() configuration API was going to to native-tls, but then the connection was happening with rustls. As I re-read the above, you're right -- it is using native-tls all the way.

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

6 participants