From 7c60c955662319351859bf62837bba2d91c702e6 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Mon, 8 Nov 2021 10:14:28 +1100 Subject: [PATCH] feat: allow webhook certificates to be configured in the pact_broker.yml file --- docs/configuration.yml | 15 ++++ lib/pact_broker/certificates/service.rb | 34 ++++++++- .../config/runtime_configuration.rb | 9 +++ spec/integration/webhooks/certificate_spec.rb | 16 ++++- .../pact_broker/certificates/service_spec.rb | 69 ++++++++++++++----- spec/support/config_webhook_certificates.yml | 24 +++++++ 6 files changed, 147 insertions(+), 20 deletions(-) create mode 100644 spec/support/config_webhook_certificates.yml diff --git a/docs/configuration.yml b/docs/configuration.yml index 626fd1bec..b551aeec4 100644 --- a/docs/configuration.yml +++ b/docs/configuration.yml @@ -249,6 +249,21 @@ groups: - 10.2.3.41/24 - /.*\\.foo\\.com$/ more_info: https://docs.pact.io/pact_broker/configuration/#webhook-whitelists + webhook_certificates: + description: |- + A list of SSL certificate configuration objects with the keys `description`, and either `content` or `path`. These + certificates are used when a webhook needs to connect to a server that uses a self signed certificate. + + Each certificate configuration item accepts a chain of certificates in PEM format - there may be multiple 'BEGIN CERTIFICATE' and 'END CERTIFICATE' in the content of each item. + + When setting the content, use the syntax "content: |-" followed by a new line, and then the contents of the certificate + chain in PEM format, indented by 2 more characters. + + When setting the path, the full path to the certificate file in PEM format must be specified. + + The certificate configuration is not validated on startup. If any of the configured certificates cannot be loaded during the execution of a webhook, an error + will be logged, and they will be ignored. You can check if the configuration is working by testing the execution of + a webhook that connects to the server with the self signed certificate by following these instructions https://docs.pact.io/pact_broker/webhooks/debugging_webhooks#testing-webhook-execution disable_ssl_verification: description: "If set to true, SSL verification will be disabled for the HTTP requests made by the webhooks" default_value: false diff --git a/lib/pact_broker/certificates/service.rb b/lib/pact_broker/certificates/service.rb index 2a6f47305..8c96f2a67 100644 --- a/lib/pact_broker/certificates/service.rb +++ b/lib/pact_broker/certificates/service.rb @@ -25,9 +25,12 @@ def cert_store end def find_all_certificates + certificates_from_database + certificates_from_config + end + + def certificates_from_database Certificate.collect do | certificate | - cert_arr = certificate.content.split(/(-----END [^\-]+-----)/).each_slice(2).map(&:join).map(&:strip).select{|s| !s.empty?} - cert_arr.collect do |c| + split_certificate_chain(certificate.content).collect do |c| begin OpenSSL::X509::Certificate.new(c) rescue StandardError => e @@ -37,6 +40,33 @@ def find_all_certificates end end.flatten.compact end + + def certificates_from_config + PactBroker.configuration.webhook_certificates.select{| c| c[:content] || c[:path] }.collect do | certificate_config, i | + load_certificate_config(certificate_config, i) + end.flatten.compact + end + + def load_certificate_config(certificate_config, i) + begin + content = certificate_config[:content] || File.read(certificate_config[:path]) + split_certificate_chain(content).collect do |c| + begin + OpenSSL::X509::Certificate.new(c) + rescue StandardError => e + logger.warn("Error creating certificate object from webhook_certificate at index #{i} with description #{certificate_config[:description]}", e) + nil + end + end + rescue StandardError => e + logger.warn("Error loading webhook_certificate at index #{i} with description #{certificate_config[:description]}", e) + nil + end + end + + def split_certificate_chain(content) + content.split(/(-----END [^\-]+-----)/).each_slice(2).map(&:join).map(&:strip).select{|s| !s.empty?} + end end end end diff --git a/lib/pact_broker/config/runtime_configuration.rb b/lib/pact_broker/config/runtime_configuration.rb index 945fdd2f6..b88d255aa 100644 --- a/lib/pact_broker/config/runtime_configuration.rb +++ b/lib/pact_broker/config/runtime_configuration.rb @@ -52,6 +52,7 @@ class RuntimeConfiguration < Anyway::Config webhook_scheme_whitelist: ["https"], webhook_host_whitelist: [], disable_ssl_verification: false, + webhook_certificates: [], user_agent: "Pact Broker v#{PactBroker::VERSION}", ) @@ -174,6 +175,14 @@ def features= features super(value_to_string_array(features, "features").collect(&:downcase)) end + def webhook_certificates= webhook_certificates + if webhook_certificates.is_a?(Array) + super(webhook_certificates.collect(&:symbolize_keys)) + elsif !webhook_certificates.nil? + raise_validation_error("webhook_certificates must be an array") + end + end + def warning_error_classes warning_error_class_names.collect do | class_name | begin diff --git a/spec/integration/webhooks/certificate_spec.rb b/spec/integration/webhooks/certificate_spec.rb index bbc874be3..9cfd0734b 100644 --- a/spec/integration/webhooks/certificate_spec.rb +++ b/spec/integration/webhooks/certificate_spec.rb @@ -15,7 +15,6 @@ def wait_for_server_to_start wait_for_server_to_start end - let(:td) { TestDataBuilder.new } let(:webhook_request) do PactBroker::Domain::WebhookRequest.new( method: "get", @@ -30,7 +29,7 @@ def wait_for_server_to_start end end - context "with the correct cacert" do + context "with the correct cacert in the database" do let!(:certificate) do td.create_certificate(path: "spec/fixtures/certificates/cacert.pem") end @@ -41,6 +40,19 @@ def wait_for_server_to_start end end + context "with the correct embedded cacert in the configuration" do + include Anyway::Testing::Helpers + + around do |ex| + with_env("PACT_BROKER_CONF" => PactBroker.project_root.join("spec/support/config_webhook_certificates.yml").to_s, &ex) + end + + it "succeeds" do + puts subject.body unless subject.code == "200" + expect(subject.code).to eq "200" + end + end + after(:all) do Process.kill "KILL", @pipe.pid end diff --git a/spec/lib/pact_broker/certificates/service_spec.rb b/spec/lib/pact_broker/certificates/service_spec.rb index e56255c65..82067e771 100644 --- a/spec/lib/pact_broker/certificates/service_spec.rb +++ b/spec/lib/pact_broker/certificates/service_spec.rb @@ -41,37 +41,74 @@ module Certificates end describe "#find_all_certificates" do - let!(:certificate) do - Certificate.create(uuid: "1234", content: certificate_content) - end - subject { Service.find_all_certificates } - context "with a valid certificate chain" do + context "with a certificate from the configuration (embedded)" do + before do + allow(PactBroker.configuration).to receive(:webhook_certificates).and_return([{ description: "foo", content: File.read("spec/fixtures/certificates/cacert.pem") }]) + end + it "returns all the X509 Certificate objects" do - expect(subject.size).to eq 2 + expect(subject.size).to eq 1 end end - context "with a valid CA file" do - let(:certificate_content) { File.read("spec/fixtures/certificates/cacert.pem") } + context "with a certificate from the configuration (path)" do + before do + allow(PactBroker.configuration).to receive(:webhook_certificates).and_return([{ description: "foo", path: "spec/fixtures/certificates/cacert.pem" }]) + end it "returns all the X509 Certificate objects" do - expect(logger).to_not receive(:error).with(/Error.*1234/) expect(subject.size).to eq 1 end + + context "when the file does not exist" do + before do + allow(PactBroker.configuration).to receive(:webhook_certificates).and_return([{ description: "foo", path: "wrong" }]) + end + + it "logs an error" do + expect(logger).to receive(:warn).with(/Error.*foo/, StandardError) + subject + end + + it "does not return the certificate" do + expect(subject.size).to eq 0 + end + end end - context "with an invalid certificate file" do - let(:certificate_content) { File.read("spec/fixtures/certificate-invalid.pem") } + context "with a certificate in the database" do + let!(:certificate) do + Certificate.create(uuid: "1234", content: certificate_content) + end - it "logs an error" do - expect(logger).to receive(:warn).with(/Error.*1234/, StandardError) - subject + context "with a valid certificate chain" do + it "returns all the X509 Certificate objects" do + expect(subject.size).to eq 2 + end end - it "returns all the valid X509 Certificate objects" do - expect(subject.size).to eq 1 + context "with a valid CA file" do + let(:certificate_content) { File.read("spec/fixtures/certificates/cacert.pem") } + + it "returns all the X509 Certificate objects" do + expect(logger).to_not receive(:error).with(/Error.*1234/) + expect(subject.size).to eq 1 + end + end + + context "with an invalid certificate file" do + let(:certificate_content) { File.read("spec/fixtures/certificate-invalid.pem") } + + it "logs an error" do + expect(logger).to receive(:warn).with(/Error.*1234/, StandardError) + subject + end + + it "returns all the valid X509 Certificate objects" do + expect(subject.size).to eq 1 + end end end end diff --git a/spec/support/config_webhook_certificates.yml b/spec/support/config_webhook_certificates.yml new file mode 100644 index 000000000..2b8d54dd4 --- /dev/null +++ b/spec/support/config_webhook_certificates.yml @@ -0,0 +1,24 @@ +webhook_certificates: + - description: test + content: |- + -----BEGIN CERTIFICATE----- + MIIDZDCCAkygAwIBAgIBATANBgkqhkiG9w0BAQsFADBCMRMwEQYKCZImiZPyLGQB + GRYDb3JnMRkwFwYKCZImiZPyLGQBGRYJcnVieS1sYW5nMRAwDgYDVQQDDAdSdWJ5 + IENBMCAXDTE4MDUxNzA3NDQzNVoYDzIxMTgwNDIzMDc0NDM1WjBCMRMwEQYKCZIm + iZPyLGQBGRYDb3JnMRkwFwYKCZImiZPyLGQBGRYJcnVieS1sYW5nMRAwDgYDVQQD + DAdSdWJ5IENBMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA1tcEnh7S + iiL46HNfx5LkGxBZ9Dn7tUm6CKlPzykjlCGZ9a5VpnZxwEK5LkIvbMiBz8UjhcVh + 8pUghbyvwSHdtiMDNU4zpKIgrTXZ/tiqctgYYSFbtEtE17VrVM8JZFSxzLSJEvEZ + rhkOqeVEGJJY28taItbjxfHYkTlQYTjn6tA18KT13nGAUMEoC0HZTHYr2nCY7MzI + cEISvm5PP7gXKHrOfpbm+E3qMm9kyDQLkez8iGfq2aGSshuT4mcAvxq5dS6TsPSy + ZphnfHw3THqgBrR8Bw1tMhsnLhD96Miy5sRnY2gQEAQngccLZ/F6ls6a+5Adka2o + zFmJVZXhHbVeRwIDAQABo2MwYTAPBgNVHRMBAf8EBTADAQH/MA4GA1UdDwEB/wQE + AwIBBjAdBgNVHQ4EFgQUBX8ZMupoE2NMwyE1kdlVptFti/kwHwYDVR0jBBgwFoAU + BX8ZMupoE2NMwyE1kdlVptFti/kwDQYJKoZIhvcNAQELBQADggEBAIxS0jDTRC9R + mxsr2j/o9oQvRi5+74qDlXs7YzbQ1V7dy++g48St6Yjk4xfdGdgAS8IrS9vIRKUy + jnlwUklnkvoWk2DKF9NFA32c1mxZhau5QGu3VH7pgmcWQawXttqpgHbSLEDAf9wU + jgTRdL8LxMIf6xy2uPL8GZWFbmdU5HOb3czS1drouE0U3ZI+1uzAlR3vqGo0Mvhd + MwYBodIJlWa0mXKMnfZxYLtiv7m5H5I2zBfget3+3ovuN79Zn6RA3ecnxn75jalA + R6MNlS/tzpXcS/gwnSKrwHSjb1V+B4Q96EsfulWC2UpTa0WTxngyiqtp6GU6RZva + jHT1Ty2CglM= + -----END CERTIFICATE-----