feat: Handle x509 certs in HTTP Client #142

gmolki · 2023-09-13T07:05:13Z

Goal of this PR is to enable the usage of x509 certificates in http requests.

Related PR in pact-support: pact-foundation/pact-support#99

mefellows · 2023-09-13T23:24:25Z

Thanks for this! I think env vars are a nice touch, but I wonder if having CLI options for these would be a preferred starting point for consistency with other flags?

In any case, it's a niche case, so if this supports your need then that shouldn't stop this from going through, assuming we're happy with the env var naming and approach.

In terms of consistency with env var naming, the convention is PACT_<THE_ENV_VAR>.

So X509_CERT_ENABLED would be PACT_X509_CERT_ENABLED and so on.

bethesque · 2023-09-14T03:30:40Z

lib/pact_broker/client/hal/http_client.rb

@@ -127,6 +133,18 @@ def verbose?
          verbose || ENV["VERBOSE"] == "true"
        end

+        def custom_x509_certificate?


Is this useful? Can we just use the presence of the env vars to work out whether to set the cert/key or not, the same way we do for the SSL cert env vars?

Sure, good idea!

I have renamed the method and handled the presence validation for the env vars in the method.
I think it's better to have a single method with this logic rather than have it in two lines repeated.
Although, if you prefer to remove this method I can move the logic to each line 😄

bethesque · 2023-09-14T03:34:23Z

lib/pact_broker/client/hal/http_client.rb

+        end
+
+        def x509_cert_file
+          File.read(ENV['X509_CERT_FILE_PATH'])


As Matt said, we do usually prefix all env vars with PACT_. We use SSL_CERT_FILE and SSL_CERT_DIR because they are pre-existing Unix standards, however.

There's no such standard for x509 certificates that I can find though, so we need to decide whether to make it match the SSL cert env vars, or the rest of the PACT env vars. The SSL env vars and the x509 env vars are likely to be used together, aren't they, given that this is about doing mutual certificate authentication. Given this is the client cert (not the server cert) it might make sense to include that in the name.
I think the most consistent approach would be to call them X509_CLIENT_CERT_FILE and X509_CLIENT_KEY_FILE.

We don't have command line arguments for the SSL certificates, so I don't think we need to add them for the x509 certs.

These are client certificates, that's correct. I agree with including that in the name for clarity.

I don't see anywhere in this code that requires the client to confirm the certificate (authenticity) of the server - so perhaps that's forthcoming, or not required for the use case.

i.e. as it stands, a malicious server could present a valid certificate and if it's in the CLI's CA then there are no client-side checks to ensure the requests are going to the right place.

bethesque · 2023-09-14T03:55:41Z

Testing is going to be a bit fiddly, but we can copy from some prior art

Here's a test that executes a request to a webrick server set up with SSL

https://github.com/pact-foundation/pact_broker/blob/1bb6088da7790c20405f0774ae497b94d6915774/spec/integration/webhooks/certificate_spec.rb#L16

This is the file it uses to run the server:

https://github.com/pact-foundation/pact_broker/blob/master/spec/support/ssl_webhook_server.rb

Looks like there is a verify client option :SSLVerifyClient => OpenSSL::SSL::VERIFY_FAIL_IF_NO_PEER_CERT | OpenSSL::SSL::VERIFY_PEER, referenced in this stack overflow issue https://stackoverflow.com/questions/54009531/ruby-webrick-server-not-able-to-verify-client-certificate

I'm guessing we would need to add SSLClientCA and SSLVerifyClient and maybe SSLVerifyDepth to the ssl_webhook_server.rb example to set it up for this scenario.

YOU54F · 2023-09-19T13:18:10Z

Just kicked off CI, thanks for the updates post review Gerard

gmolki · 2023-09-19T16:03:49Z

Just kicked off CI, thanks for the updates post review Gerard

Thanks @YOU54F.
I just pushed a new commit fixing some failing tests due to different OpenSSL versions handling errors differently.

YOU54F · 2023-09-19T17:42:02Z

ty, fyi, you can ignore the failing pact and can-i-deploy test, they are failing as due to it being a PR from a outside collab, you can't retrieve secrets, in order to login to the pact broker for the tests, The other tests should be a healthy indicator though 👍🏾

bethesque · 2023-09-19T23:05:50Z

spec/lib/pact_broker/client/hal/http_client_spec.rb

+        before(:all) do
+          @pipe = IO.popen("bundle exec ruby ./spec/support/ssl_server.rb")
+          ENV['SSL_CERT_FILE'] = "./spec/fixtures/certificates/ca_cert.pem"
+          ENV['SSL_CERT_DIR'] =  "./spec/fixtures/certificates/"


If SSL_CERT_FILE is set, is SSL_CERT_DIR necessary?

Good catch!
I thought both were required, but seems like with the SSL_CERT_FILE is enough.

bethesque · 2023-09-19T23:07:02Z

spec/lib/pact_broker/client/hal/http_client_spec.rb

+          it "fails raising SSL error" do
+            expect { do_get }
+              .to raise_error { |error|
+                expect([OpenSSL::SSL::SSLError, Errno::ECONNRESET]).to include(error.class)


That's curious. It could raise either error? Can you tell me more about that?

Seems like so.
I ran the server in debug mode to check why this happening, and I got the following output by running it in:

ubuntu-latest ruby v3.1

[2023-09-20 08:52:01] INFO WEBrick::HTTPServer#start: pid=508 port=4444 | with valid x509 client certificates | succeeds | when invalid x509 certificates are set | [2023-09-20 08:52:01] ERROR OpenSSL::SSL::SSLError: SSL_accept returned=1 errno=0 peeraddr=127.0.0.1:49104 state=error: certificate verify failed (self-signed certificate) | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `accept' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `block (2 levels) in start_thread' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/utils.rb:258:in `timeout' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:300:in `block in start_thread' | [2023-09-20 08:52:01] ERROR OpenSSL::SSL::SSLError: SSL_accept returned=1 errno=0 peeraddr=127.0.0.1:49106 state=error: certificate verify failed (self-signed certificate) | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `accept' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `block (2 levels) in start_thread' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/utils.rb:258:in `timeout' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:300:in `block in start_thread' | ERROR: Error making request - Errno::ECONNRESET Connection reset by peer /Users/jobandtalent/jobandtalent/pact_broker-client/lib/pact_broker/client/hal/http_client.rb:87:in `block (2 levels) in perform_request', attempt 1 of 5 | [2023-09-20 08:52:06] ERROR OpenSSL::SSL::SSLError: SSL_accept returned=1 errno=0 peeraddr=127.0.0.1:49112 state=error: certificate verify failed (self-signed certificate) | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `accept' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `block (2 levels) in start_thread' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/utils.rb:258:in `timeout' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:300:in `block in start_thread' | [2023-09-20 08:52:06] ERROR OpenSSL::SSL::SSLError: SSL_accept returned=1 errno=0 peeraddr=127.0.0.1:49122 state=error: certificate verify failed (self-signed certificate) | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `accept' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `block (2 levels) in start_thread' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/utils.rb:258:in `timeout' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:300:in `block in start_thread' | ERROR: Error making request - Errno::ECONNRESET Connection reset by peer /Users/jobandtalent/jobandtalent/pact_broker-client/lib/pact_broker/client/hal/http_client.rb:87:in `block (2 levels) in perform_request', attempt 2 of 5 | [2023-09-20 08:52:12] ERROR OpenSSL::SSL::SSLError: SSL_accept returned=1 errno=0 peeraddr=127.0.0.1:43724 state=error: certificate verify failed (self-signed certificate) | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `accept' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `block (2 levels) in start_thread' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/utils.rb:258:in `timeout' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:300:in `block in start_thread' | [2023-09-20 08:52:12] ERROR OpenSSL::SSL::SSLError: SSL_accept returned=1 errno=0 peeraddr=127.0.0.1:43736 state=error: certificate verify failed (self-signed certificate) | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `accept' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `block (2 levels) in start_thread' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/utils.rb:258:in `timeout' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:300:in `block in start_thread' | ERROR: Error making request - Errno::ECONNRESET Connection reset by peer /Users/jobandtalent/jobandtalent/pact_broker-client/lib/pact_broker/client/hal/http_client.rb:87:in `block (2 levels) in perform_request', attempt 3 of 5 | [2023-09-20 08:52:17] ERROR OpenSSL::SSL::SSLError: SSL_accept returned=1 errno=0 peeraddr=127.0.0.1:43752 state=error: certificate verify failed (self-signed certificate) | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `accept' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `block (2 levels) in start_thread' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/utils.rb:258:in `timeout' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:300:in `block in start_thread' | [2023-09-20 08:52:17] ERROR OpenSSL::SSL::SSLError: SSL_accept returned=1 errno=0 peeraddr=127.0.0.1:43764 state=error: certificate verify failed (self-signed certificate) | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `accept' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `block (2 levels) in start_thread' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/utils.rb:258:in `timeout' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:300:in `block in start_thread' | ERROR: Error making request - Errno::ECONNRESET Connection reset by peer /Users/jobandtalent/jobandtalent/pact_broker-client/lib/pact_broker/client/hal/http_client.rb:87:in `block (2 levels) in perform_request', attempt 4 of 5 | [2023-09-20 08:52:22] ERROR OpenSSL::SSL::SSLError: SSL_accept returned=1 errno=0 peeraddr=127.0.0.1:44812 state=error: certificate verify failed (self-signed certificate) | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `accept' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `block (2 levels) in start_thread' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/utils.rb:258:in `timeout' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:300:in `block in start_thread' | [2023-09-20 08:52:22] ERROR OpenSSL::SSL::SSLError: SSL_accept returned=1 errno=0 peeraddr=127.0.0.1:44818 state=error: certificate verify failed (self-signed certificate) | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `accept' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `block (2 levels) in start_thread' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/utils.rb:258:in `timeout' | /opt/hostedtoolcache/Ruby/3.1.4/x64/lib/ruby/gems/3.1.0/gems/webrick-1.8.1/lib/webrick/server.rb:300:in `block in start_thread' | ERROR: Error making request - Errno::ECONNRESET Connection reset by peer /Users/jobandtalent/jobandtalent/pact_broker-client/lib/pact_broker/client/hal/http_client.rb:87:in `block (2 levels) in perform_request', attempt 5 of 5 | fails raising SSL error

ubuntu-latest ruby v2.7

with valid x509 client certificates | succeeds | when invalid x509 certificates are set | [2023-09-20 08:59:59] ERROR OpenSSL::SSL::SSLError: SSL_accept returned=1 errno=0 state=error: certificate verify failed (self signed certificate) | /opt/hostedtoolcache/Ruby/2.7.8/x64/lib/ruby/gems/2.7.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `accept' | /opt/hostedtoolcache/Ruby/2.7.8/x64/lib/ruby/gems/2.7.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `block (2 levels) in start_thread' | /opt/hostedtoolcache/Ruby/2.7.8/x64/lib/ruby/gems/2.7.0/gems/webrick-1.8.1/lib/webrick/utils.rb:258:in `timeout' | /opt/hostedtoolcache/Ruby/2.7.8/x64/lib/ruby/gems/2.7.0/gems/webrick-1.8.1/lib/webrick/server.rb:300:in `block in start_thread' | [2023-09-20 08:59:59] ERROR OpenSSL::SSL::SSLError: SSL_accept returned=1 errno=0 state=error: certificate verify failed (self signed certificate) | /opt/hostedtoolcache/Ruby/2.7.8/x64/lib/ruby/gems/2.7.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `accept' | /opt/hostedtoolcache/Ruby/2.7.8/x64/lib/ruby/gems/2.7.0/gems/webrick-1.8.1/lib/webrick/server.rb:302:in `block (2 levels) in start_thread' | /opt/hostedtoolcache/Ruby/2.7.8/x64/lib/ruby/gems/2.7.0/gems/webrick-1.8.1/lib/webrick/utils.rb:258:in `timeout' | /opt/hostedtoolcache/Ruby/2.7.8/x64/lib/ruby/gems/2.7.0/gems/webrick-1.8.1/lib/webrick/server.rb:300:in `block in start_thread' | fails raising SSL error

Seems like depending on the OS and ruby version it handles the verification error differently

https://github.com/ruby/net-http/blob/beb20c036dce1b14b8e78c012c3537957aaf6590/test/net/http/test_https.rb#L242

Gotcha, thanks for digging into that.

bethesque · 2023-09-19T23:14:22Z

Can you check for me what the verify_mode is when the disable ssl verification is not set? I can't tell what the default is from the docs at https://docs.ruby-lang.org/en//3.2/Net/HTTP.html

gmolki · 2023-09-20T07:57:40Z

Can you check for me what the verify_mode is when the disable ssl verification is not set? I can't tell what the default is from the docs at https://docs.ruby-lang.org/en//3.2/Net/HTTP.html

In case the disable_ssl_verification is not set, if the use_ssl option is set to true then http.start would default the value to OpenSSL::SSL::VERIFY_PEER

net-http docs

Additionally, library code that applies this default can be found here

bethesque · 2023-10-02T00:34:42Z

Can you add docs under the SSL cert section here https://github.com/pact-foundation/pact-ruby-standalone/blob/master/README.md for the pact-ruby-standalone and under the "Using a custom certificate" section here https://github.com/pact-foundation/pact-ruby-cli/blob/master/README.md#using-a-custom-certificate for the pact-cli

bethesque · 2023-10-22T22:27:52Z

@gmolki the specs for this have suddenly started failing. Can you have a look please? https://github.com/pact-foundation/pact_broker-client/actions/runs/6606534979/job/17942830227#step:5:858

bethesque · 2023-10-23T00:23:56Z

I've worked out what it is - it's the new version of rack that was introduced by the pact mock service. Fixed it.

feat: handle-x509-certs

52deec6

gmolki changed the title ~~feat: handle-x509-certs~~ feat: Handle x509 certs in HTTP Client Sep 13, 2023

bethesque self-requested a review September 13, 2023 23:35

bethesque requested changes Sep 14, 2023

View reviewed changes

gmolki added 3 commits September 14, 2023 06:55

refactor: x509 env vars

5328408

Add faraday & faraday-retry in test env

9797fd9

test: x509 certificates

9626f23

test/fix: cover different openssl error handling

7820898

bethesque reviewed Sep 19, 2023

View reviewed changes

refactor(test): remove SSL_CERT_DIR env setup in test

ec96c2a

gmolki requested review from bethesque and mefellows September 21, 2023 07:49

bethesque approved these changes Oct 2, 2023

View reviewed changes

bethesque merged commit c3aa8dc into pact-foundation:master Oct 2, 2023
13 of 15 checks passed

This was referenced Oct 10, 2023

docs: Add x509 certificates usage docs pact-foundation/pact-ruby-standalone#119

Merged

docs: Add x509 certificates usage docs pact-foundation/pact-ruby-cli#122

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Handle x509 certs in HTTP Client #142

feat: Handle x509 certs in HTTP Client #142

gmolki commented Sep 13, 2023

mefellows commented Sep 13, 2023 •

edited

bethesque Sep 14, 2023

gmolki Sep 14, 2023

bethesque Sep 14, 2023

mefellows Sep 14, 2023

bethesque commented Sep 14, 2023

YOU54F commented Sep 19, 2023

gmolki commented Sep 19, 2023

YOU54F commented Sep 19, 2023

bethesque Sep 19, 2023

gmolki Sep 20, 2023

bethesque Sep 19, 2023

gmolki Sep 20, 2023

gmolki Sep 20, 2023

bethesque Oct 1, 2023

bethesque commented Sep 19, 2023

gmolki commented Sep 20, 2023

bethesque commented Oct 2, 2023

bethesque commented Oct 22, 2023 •

edited

bethesque commented Oct 23, 2023

feat: Handle x509 certs in HTTP Client #142

feat: Handle x509 certs in HTTP Client #142

Conversation

gmolki commented Sep 13, 2023

mefellows commented Sep 13, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bethesque commented Sep 14, 2023

YOU54F commented Sep 19, 2023

gmolki commented Sep 19, 2023

YOU54F commented Sep 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bethesque commented Sep 19, 2023

gmolki commented Sep 20, 2023

bethesque commented Oct 2, 2023

bethesque commented Oct 22, 2023 • edited

bethesque commented Oct 23, 2023

mefellows commented Sep 13, 2023 •

edited

bethesque commented Oct 22, 2023 •

edited