diff --git a/lib/puppetserver/ca/action/sign.rb b/lib/puppetserver/ca/action/sign.rb index b573add..c894b31 100644 --- a/lib/puppetserver/ca/action/sign.rb +++ b/lib/puppetserver/ca/action/sign.rb @@ -66,17 +66,33 @@ def run(input) return 1 if Errors.handle_with_usage(@logger, puppet.errors) ca = Puppetserver::Ca::CertificateAuthority.new(@logger, puppet.settings) + bulk_sign = ca.server_has_bulk_signing_endpoints + + # Bulk sign endpoints don't allow setting TTL, so + # use single signing endpoint if TTL is specified. + success = false + if input['ttl'] || !bulk_sign + if input['all'] + requested_certnames = get_all_pending_certs(ca) + return 1 if requested_certnames.nil? + return 24 if requested_certnames.empty? + else + requested_certnames = input['certname'] + end - if input['all'] - requested_certnames = get_all_pending_certs(ca) - return 1 if requested_certnames.nil? - return 24 if requested_certnames.empty? + success = ca.sign_certs(requested_certnames, input['ttl']) + return success ? 0 : 1 else - requested_certnames = input['certname'] + result = input['all'] ? ca.sign_all : ca.sign_bulk(input['certname']) + case result + when :success + return 0 + when :no_requests + return 24 + else + return 1 + end end - - success = ca.sign_certs(requested_certnames, input['ttl']) - return success ? 0 : 1 end def get_all_pending_certs(ca) diff --git a/lib/puppetserver/ca/certificate_authority.rb b/lib/puppetserver/ca/certificate_authority.rb index 77483a3..a7f43ce 100644 --- a/lib/puppetserver/ca/certificate_authority.rb +++ b/lib/puppetserver/ca/certificate_authority.rb @@ -19,7 +19,6 @@ class CertificateAuthority } REVOKE_BODY = JSON.dump({ desired_state: 'revoked' }) - SIGN_BODY = JSON.dump({ desired_state: 'signed' }) def initialize(logger, settings) @logger = logger @@ -28,6 +27,15 @@ def initialize(logger, settings) @ca_port = settings[:ca_port] end + def server_has_bulk_signing_endpoints + url = HttpClient::URL.new('https', @ca_server, @ca_port, 'status', 'v1', 'services') + result = @client.with_connection(url) do |connection| + connection.get(url) + end + version = process_results(:server_version, nil, result) + return version >= Gem::Version.new('8.4.0') + end + def worst_result(previous_result, current_result) %i{success invalid not_found error}.each do |state| if previous_result == state @@ -61,13 +69,26 @@ def process_ttl_input(ttl) end end + def sign_all + return post(resource_type: 'sign', + resource_name: 'all', + body: '{}', + type: :sign_all) + end + + def sign_bulk(certnames) + return post(resource_type: 'sign', + body: "{\"certnames\":#{certnames}}", + type: :sign_bulk + ) + end + def sign_certs(certnames, ttl=nil) results = [] if ttl lifetime = process_ttl_input(ttl) return false if lifetime.nil? - body = JSON.dump({ desired_state: 'signed', - cert_ttl: lifetime}) + body = JSON.dump({ desired_state: 'signed', cert_ttl: lifetime}) results = put(certnames, resource_type: 'certificate_status', body: body, @@ -75,7 +96,7 @@ def sign_certs(certnames, ttl=nil) else results = put(certnames, resource_type: 'certificate_status', - body: SIGN_BODY, + body: JSON.dump({ desired_state: 'signed' }), type: :sign) end @@ -119,6 +140,48 @@ def put(certnames, resource_type:, body:, type:, headers: {}) end end + # Make an HTTP POST request to CA + # @param endpoint [String] the endpoint to post to for the url + # @param body [JSON/String] body of the post request + # @param type [Symbol] type of error processing to perform on result + # @return [Boolean] whether all requests were successful + def post(resource_type:, resource_name: nil, body:, type:, headers: {}) + url = make_ca_url(resource_type, resource_name) + results = @client.with_connection(url) do |connection| + result = connection.post(body, url, headers) + process_results(type, nil, result) + end + end + + # Handle the result data from the /sign and /sign/all endpoints + def process_bulk_sign_result_data(result) + data = JSON.parse(result.body) + signed = data.dig('signed') || [] + no_csr = data.dig('no-csr') || [] + signing_errors = data.dig('signing-errors') || [] + + if !signed.empty? + @logger.inform "Successfully signed the following certificate requests:" + signed.each { |s| @logger.inform " #{s}" } + end + + @logger.err 'Error:' if !no_csr.empty? || !signing_errors.empty? + if !no_csr.empty? + @logger.err ' No certificate request found for the following nodes when attempting to sign:' + no_csr.each { |s| @logger.err " #{s}" } + end + if !signing_errors.empty? + @logger.err ' Error encountered when attempting to sign the certificate request for the following nodes:' + signing_errors.each { |s| @logger.err " #{s}" } + end + if no_csr.empty? && signing_errors.empty? + @logger.err 'No waiting certificate requests to sign.' if signed.empty? + return signed.empty? ? :no_requests : :success + else + return :error + end + end + # logs the action and returns true/false for success def process_results(type, certname, result) case type @@ -138,6 +201,50 @@ def process_results(type, certname, result) @logger.err " body: #{result.body.to_s}" if result.body return :error end + when :sign_all + if result.code == '200' + if !result.body + @logger.err 'Error:' + @logger.err ' Response from /sign/all endpoint did not include a body. Unable to verify certificate requests were signed.' + return :error + end + begin + return process_bulk_sign_result_data(result) + rescue JSON::ParserError + @logger.err 'Error:' + @logger.err ' Unable to parse the response from the /sign/all endpoint.' + @logger.err " body #{result.body.to_s}" + return :error + end + else + @logger.err 'Error:' + @logger.err ' When attempting to sign all certificate requests, received:' + @logger.err " code: #{result.code}" + @logger.err " body: #{result.body.to_s}" if result.body + return :error + end + when :sign_bulk + if result.code == '200' + if !result.body + @logger.err 'Error:' + @logger.err ' Response from /sign endpoint did not include a body. Unable to verify certificate requests were signed.' + return :error + end + begin + return process_bulk_sign_result_data(result) + rescue JSON::ParserError + @logger.err 'Error:' + @logger.err ' Unable to parse the response from the /sign endpoint.' + @logger.err " body #{result.body.to_s}" + return :error + end + else + @logger.err 'Error:' + @logger.err ' When attempting to sign certificate requests, received:' + @logger.err " code: #{result.code}" + @logger.err " body: #{result.body.to_s}" if result.body + return :error + end when :revoke case result.code when '200', '204' @@ -170,6 +277,19 @@ def process_results(type, certname, result) @logger.err " body: #{result.body.to_s}" if result.body return :error end + when :server_version + if result.code == '200' && result.body + begin + data = JSON.parse(result.body) + version_str = data.dig('ca','service_version') + return Gem::Version.new(version_str.match('^\d+\.\d+\.\d+')[0]) + rescue JSON::ParserError, NoMethodError + # If we get bad JSON, version_str is nil, or the matcher doesn't match, + # fall through to returning a version of 0. + end + end + @logger.debug 'Could not detect server version. Defaulting to legacy signing endpoints.' + return Gem::Version.new(0) end end diff --git a/lib/puppetserver/ca/utils/http_client.rb b/lib/puppetserver/ca/utils/http_client.rb index 2c57299..72cd32c 100644 --- a/lib/puppetserver/ca/utils/http_client.rb +++ b/lib/puppetserver/ca/utils/http_client.rb @@ -130,6 +130,19 @@ def put(body, url_override = nil, header_overrides = {}) Result.new(result.code, result.body) end + def post(body, url_override = nil, header_overrides = {}) + url = url_override || @url + headers = @default_headers.merge(header_overrides) + + @logger.debug("Making a POST request at #{url.full_url}") + + request = Net::HTTP::Post.new(url.to_uri, headers) + request.body = body + result = @conn.request(request) + + Result.new(result.code, result.body) + end + def delete(url_override = nil, header_overrides = {}) url = url_override || @url headers = @default_headers.merge(header_overrides) @@ -151,7 +164,7 @@ def delete(url_override = nil, header_overrides = {}) :resource_type, :resource_name, :query) do def full_url url = protocol + '://' + host + ':' + port + '/' + - [endpoint, version, resource_type, resource_name].join('/') + [endpoint, version, resource_type, resource_name].compact.join('/') url = url + "?" + URI.encode_www_form(query) unless query.nil? || query.empty? return url diff --git a/spec/puppetserver/ca/action/sign_spec.rb b/spec/puppetserver/ca/action/sign_spec.rb index a910b8c..586e910 100644 --- a/spec/puppetserver/ca/action/sign_spec.rb +++ b/spec/puppetserver/ca/action/sign_spec.rb @@ -1,6 +1,7 @@ require 'puppetserver/ca/action/sign' require 'puppetserver/ca/cli' require 'puppetserver/ca/logger' +require 'puppetserver/ca/utils/http_client' RSpec.describe 'Puppetserver::Ca::SignAction' do let(:err) { StringIO.new } @@ -64,7 +65,11 @@ let(:get_success) { response.new('200', 'Stuff') } let(:not_found) { response.new('404', 'Not Found') } let(:empty) { response.new('404', '[]') } - + let(:status_url) { Puppetserver::Ca::Utils::HttpClient::URL.new('https','localhost','8140','status','v1','services') } + let(:bulk_sign_url) { Puppetserver::Ca::Utils::HttpClient::URL.new('https','localhost','8140','puppet-ca','v1','sign', nil, {}) } + let(:bulk_sign_all_url) { Puppetserver::Ca::Utils::HttpClient::URL.new('https','localhost','8140','puppet-ca','v1','sign','all', {}) } + let(:status_old_server) { response.new('200', '{"ca":{"service_version":"7.4.1"}}') } + let(:status_new_server) { response.new('200', '{"ca":{"service_version":"8.4.1"}}') } let(:connection) { double } before do @@ -78,56 +83,184 @@ to receive(:load_key) end - it 'logs and exits with zero with successful PUT' do - allow(connection).to receive(:put).and_return(success) - exit_code = action.run({'certname' => ['foo']}) - expect(exit_code).to eq(0) - expect(out.string).to include('signed certificate request for foo') - end + describe 'bulk signing available' do + before { allow(connection).to receive(:get).with(status_url).and_return(status_new_server) } - it 'logs and exits with zero with successful PUT with a custom ttl' do - allow(connection).to receive(:put).with(/3600/, any_args).and_return(success) - exit_code = action.run({'certname' => ['foo'], 'ttl' => '1h'}) - expect(exit_code).to eq(0) - expect(out.string).to include('signed certificate request for foo') - end + describe 'using --certname' do + it 'uses /certificate_status endpoint when specifying TTL' do + allow(connection).to receive(:put).with(/3600/, any_args).and_return(success) + exit_code = action.run({'certname' => ['foo'], 'ttl' => '1h'}) + expect(exit_code).to eq(0) + expect(out.string).to include('signed certificate request for foo') + end - it 'fails when an invalid ttl is specified' do - exit_code = action.run({'certname' => ['foo'], 'ttl' => '1x'}) - expect(exit_code).to eq(1) - expect(err.string).to match(/Error.* invalid ttl value/m) - expect(connection).to_not receive(:put) - end + it 'uses the /sign endpoint' do + result = response.new('200', '{"signed":["foo","bar"],"no-csr":[],"signing-errors":[]}') + allow(connection).to receive(:post).with("{\"certnames\":[\"foo\", \"bar\"]}", bulk_sign_url, {}).and_return(result) + exit_code = action.run({'certname' => ['foo','bar']}) + expect(exit_code).to eq(0) + expect(out.string).to match(/Successfully signed the following.*foo.*bar/m) + end - it 'logs and exits with zero with successful GET and PUT' do - allow(connection).to receive(:put).and_return(success) - allow(connection).to receive(:get).and_return(get_success) - allow(action).to receive(:select_pending_certs).and_return(['ulla']) - exit_code = action.run({'all' => true}) - expect(exit_code).to eq(0) - expect(out.string).to include('signed certificate request for ulla') - end + it 'handles no CSR being present' do + result = response.new('200', '{"signed":["foo"],"no-csr":["bar"],"signing-errors":[]}') + allow(connection).to receive(:post).with("{\"certnames\":[\"foo\", \"bar\"]}", bulk_sign_url, {}).and_return(result) + exit_code = action.run({'certname' => ['foo','bar']}) + expect(exit_code).to eq(1) + expect(out.string).to match(/Successfully signed the following.*foo/m) + expect(err.string).to match(/No certificate request.*bar/m) + end - it 'fails when GET request errors' do - allow(connection).to receive(:get).and_return(not_found) - exit_code = action.run({'all' => true}) - expect(exit_code).to eq(1) - end + it 'handles an error signing a cert' do + result = response.new('200', '{"signed":["foo"],"no-csr":[],"signing-errors":["bar"]}') + allow(connection).to receive(:post).with("{\"certnames\":[\"foo\", \"bar\"]}", bulk_sign_url, {}).and_return(result) + exit_code = action.run({'certname' => ['foo','bar']}) + expect(exit_code).to eq(1) + expect(out.string).to match(/Successfully signed the following.*foo/m) + expect(err.string).to match(/Error encountered when attempting to sign.*bar/m) + end + + it 'handles no body in the response' do + result = response.new('200', nil) + allow(connection).to receive(:post).with("{\"certnames\":[\"foo\", \"bar\"]}", bulk_sign_url, {}).and_return(result) + exit_code = action.run({'certname' => ['foo','bar']}) + expect(exit_code).to eq(1) + expect(err.string).to match(/Response from \/sign endpoint did not include a body/m) + end + + it 'handles a non-JSON response' do + result = response.new('200', 'nu uh, not gonna sign nuthin') + allow(connection).to receive(:post).with("{\"certnames\":[\"foo\", \"bar\"]}", bulk_sign_url, {}).and_return(result) + exit_code = action.run({'certname' => ['foo','bar']}) + expect(exit_code).to eq(1) + expect(err.string).to match(/Unable to parse the response from the \/sign endpoint/m) + end + + it 'handles a non-200 response' do + result = response.new('404', 'Not found') + allow(connection).to receive(:post).with("{\"certnames\":[\"foo\", \"bar\"]}", bulk_sign_url, {}).and_return(result) + exit_code = action.run({'certname' => ['foo','bar']}) + expect(exit_code).to eq(1) + expect(err.string).to match(/When attempting to sign certificate requests.*404.*Not found/m) + end + end + + describe 'using --all' do + it 'uses /certificate_status endpoint when specifying TTL and --all' do + allow(connection).to receive(:put).with(/3600/, any_args).and_return(success) + allow(connection).to receive(:get).and_return(get_success) + allow(action).to receive(:select_pending_certs).and_return(['ulla']) + exit_code = action.run({'all' => true, 'ttl' => '1h'}) + expect(exit_code).to eq(0) + expect(out.string).to include('signed certificate request for ulla') + end + + it 'uses /sign/all when specifying --all' do + result = response.new('200', '{"signed":["foo","bar"],"no-csr":[],"signing-errors":[]}') + allow(connection).to receive(:post).with("{}", bulk_sign_all_url, {}).and_return(result) + exit_code = action.run({'all' => true}) + expect(exit_code).to eq(0) + expect(out.string).to match(/Successfully signed the following.*foo.*bar/m) + end - it 'returns 24 when no pending certs' do - allow_any_instance_of(Puppetserver::Ca::CertificateAuthority). - to receive(:get_certificate_statuses).and_return(empty) - exit_code = action.run({'all' => true}) - expect(exit_code).to eq(24) - expect(err.string).to include('No waiting certificate requests to sign') + it 'handles no CSRs waiting to be signed when specifying --all' do + result = response.new('200', '{"signed":[],"no-csr":[],"signing-errors":[]}') + allow(connection).to receive(:post).with("{}", bulk_sign_all_url, {}).and_return(result) + exit_code = action.run({'all' => true}) + expect(exit_code).to eq(24) + expect(err.string).to match(/No waiting certificate requests to sign/) + end + + it 'handles an error signing a cert' do + result = response.new('200', '{"signed":["foo"],"no-csr":[],"signing-errors":["bar"]}') + allow(connection).to receive(:post).with("{}", bulk_sign_all_url, {}).and_return(result) + exit_code = action.run({'all' => true}) + expect(exit_code).to eq(1) + expect(out.string).to match(/Successfully signed the following.*foo/m) + expect(err.string).to match(/Error encountered when attempting to sign.*bar/m) + end + + it 'handles no body in the response' do + result = response.new('200', nil) + allow(connection).to receive(:post).with("{}", bulk_sign_all_url, {}).and_return(result) + exit_code = action.run({'all' => true}) + expect(exit_code).to eq(1) + expect(err.string).to match(/Response from \/sign\/all endpoint did not include a body/m) + end + + it 'handles a non-JSON response' do + result = response.new('200', 'nu uh, not gonna sign nuthin') + allow(connection).to receive(:post).with("{}", bulk_sign_all_url, {}).and_return(result) + exit_code = action.run({'all' => true}) + expect(exit_code).to eq(1) + expect(err.string).to match(/Unable to parse the response from the \/sign\/all endpoint/m) + end + + it 'handles a non-200 response' do + result = response.new('404', 'Not found') + allow(connection).to receive(:post).with("{}", bulk_sign_all_url, {}).and_return(result) + exit_code = action.run({'all' => true}) + expect(exit_code).to eq(1) + expect(err.string).to match(/When attempting to sign all certificate requests.*404.*Not found/m) + end + end end - it 'continues signing certs after failed request' do - allow(connection).to receive(:put).and_return(success, not_found, success) - exit_code = action.run({'certname' => ['foo','bar','baz']}) - expect(exit_code).to eq(1) - expect(out.string).to match(/signed certificate request for foo.*signed certificate request for baz/m) - expect(err.string).to include('Could not find certificate request for bar') + + describe 'bulk signing unavailable' do + before { allow(connection).to receive(:get).with(status_url).and_return(status_old_server) } + + it 'logs and exits with zero with successful PUT' do + allow(connection).to receive(:put).and_return(success) + exit_code = action.run({'certname' => ['foo']}) + expect(exit_code).to eq(0) + expect(out.string).to include('signed certificate request for foo') + end + + it 'logs and exits with zero with successful PUT with a custom ttl' do + allow(connection).to receive(:put).with(/3600/, any_args).and_return(success) + exit_code = action.run({'certname' => ['foo'], 'ttl' => '1h'}) + expect(exit_code).to eq(0) + expect(out.string).to include('signed certificate request for foo') + end + + it 'fails when an invalid ttl is specified' do + exit_code = action.run({'certname' => ['foo'], 'ttl' => '1x'}) + expect(exit_code).to eq(1) + expect(err.string).to match(/Error.* invalid ttl value/m) + expect(connection).to_not receive(:put) + end + + it 'logs and exits with zero with successful GET and PUT' do + allow(connection).to receive(:put).and_return(success) + allow(connection).to receive(:get).and_return(get_success) + allow(action).to receive(:select_pending_certs).and_return(['ulla']) + exit_code = action.run({'all' => true}) + expect(exit_code).to eq(0) + expect(out.string).to include('signed certificate request for ulla') + end + + it 'fails when GET request errors' do + allow(connection).to receive(:get).and_return(not_found) + exit_code = action.run({'all' => true}) + expect(exit_code).to eq(1) + end + + it 'returns 24 when no pending certs' do + allow_any_instance_of(Puppetserver::Ca::CertificateAuthority). + to receive(:get_certificate_statuses).and_return(empty) + exit_code = action.run({'all' => true}) + expect(exit_code).to eq(24) + expect(err.string).to include('No waiting certificate requests to sign') + end + + it 'continues signing certs after failed request' do + allow(connection).to receive(:put).and_return(success, not_found, success) + exit_code = action.run({'certname' => ['foo','bar','baz']}) + expect(exit_code).to eq(1) + expect(out.string).to match(/signed certificate request for foo.*signed certificate request for baz/m) + expect(err.string).to include('Could not find certificate request for bar') + end end end end