From 11130fe49cf390aa91c212eaed7e222e5d389948 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Co=C3=AAlho?= Date: Sun, 22 Oct 2023 13:08:08 +0000 Subject: [PATCH 1/2] Extract and test PKCS7 signature --- lib/rubrik.rb | 1 + lib/rubrik/fill_signature.rb | 5 +---- lib/rubrik/pkcs7_signature.rb | 24 ++++++++++++++++++++++++ test/pkcs7_signature_test.rb | 29 +++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 lib/rubrik/pkcs7_signature.rb create mode 100644 test/pkcs7_signature_test.rb diff --git a/lib/rubrik.rb b/lib/rubrik.rb index 08105d1..54cb179 100644 --- a/lib/rubrik.rb +++ b/lib/rubrik.rb @@ -12,4 +12,5 @@ class Error < StandardError; end require_relative "rubrik/document/increment" require_relative "rubrik/document/serialize_object" require_relative "rubrik/fill_signature" +require_relative "rubrik/pkcs7_signature" require_relative "rubrik/sign" diff --git a/lib/rubrik/fill_signature.rb b/lib/rubrik/fill_signature.rb index d739ad5..dc6483e 100644 --- a/lib/rubrik/fill_signature.rb +++ b/lib/rubrik/fill_signature.rb @@ -1,8 +1,6 @@ # typed: true # frozen_string_literal: true -require "openssl" - module Rubrik module FillSignature extend T::Sig @@ -55,8 +53,7 @@ def call(io, signature_value_ref:, private_key:, public_key:, certificate_chain: io.pos = second_offset data_to_sign += T.must(io.read(second_length)) - signature = OpenSSL::PKCS7.sign(public_key, private_key, data_to_sign, certificate_chain, - OpenSSL::PKCS7::DETACHED | OpenSSL::PKCS7::BINARY).to_der + signature = PKCS7Signature.call(data_to_sign, private_key:, certificate: public_key) hex_signature = T.let(signature, String).unpack1("H*") padded_contents_field = "<#{hex_signature.ljust(Document::SIGNATURE_SIZE, "0")}>" diff --git a/lib/rubrik/pkcs7_signature.rb b/lib/rubrik/pkcs7_signature.rb new file mode 100644 index 0000000..edbbb84 --- /dev/null +++ b/lib/rubrik/pkcs7_signature.rb @@ -0,0 +1,24 @@ +# typed: true +# frozen_string_literal: true + +require "openssl" + +module Rubrik + module PKCS7Signature + extend T::Sig + extend self + + OPEN_SSL_FLAGS = OpenSSL::PKCS7::DETACHED | OpenSSL::PKCS7::BINARY + + sig {params( + data: String, + private_key: OpenSSL::PKey::RSA, + certificate: OpenSSL::X509::Certificate, + certificate_chain: T::Array[OpenSSL::X509::Certificate] + ).returns(String) + } + def call(data, private_key:, certificate:, certificate_chain: []) + OpenSSL::PKCS7.sign(certificate, private_key, data, certificate_chain, OPEN_SSL_FLAGS).to_der + end + end +end diff --git a/test/pkcs7_signature_test.rb b/test/pkcs7_signature_test.rb new file mode 100644 index 0000000..e7e4224 --- /dev/null +++ b/test/pkcs7_signature_test.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true +# typed: true + +require "test_helper" + +class Rubrik::PKCS7SignatureTest < Rubrik::Test + def test_signature_generation + # Arrange + certificate_file = File.open("test/support/demo_cert.pem", "rb") + + private_key = OpenSSL::PKey::RSA.new(certificate_file, "") + certificate_file.rewind + certificate = OpenSSL::X509::Certificate.new(certificate_file) + + # Act + raw_result = Rubrik::PKCS7Signature.call("test", private_key:, certificate:) + + # Assert + result = OpenSSL::ASN1.decode(raw_result) + + assert_kind_of(OpenSSL::ASN1::Sequence, result) + + content_type = result.value.first + assert_equal("pkcs7-signedData", content_type.value) + + ensure + certificate_file&.close + end +end From c18eda83393ec75c533c43aa4a06966324bbd126 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Co=C3=AAlho?= Date: Sun, 22 Oct 2023 13:08:15 +0000 Subject: [PATCH 2/2] Little refactor --- test/rubrik/sign_test.rb | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/test/rubrik/sign_test.rb b/test/rubrik/sign_test.rb index 09f526b..bae849b 100644 --- a/test/rubrik/sign_test.rb +++ b/test/rubrik/sign_test.rb @@ -23,13 +23,13 @@ def test_document_with_interactive_form expected_output.readlines.zip(output_pdf.readlines).each do |(expected_line, actual_line)| # We must erase the signature because it is timestampped - if T.must(actual_line).match?("/Type /Sig") - T.must(actual_line).sub!(/<[a-f0-9]+>/, "") - T.must(expected_line).sub!(/<[a-f0-9]+>/, "") + if actual_line&.match?("/Type /Sig") + actual_line.sub!(/<[a-f0-9]+>/, "") + expected_line.sub!(/<[a-f0-9]+>/, "") # The signature field name is also random - elsif T.must(actual_line).match?("Signature-[a-f0-9]{4}") - T.must(actual_line).sub!(/Signature-[a-f0-9]{4}/, "") - T.must(expected_line).sub!(/Signature-[a-f0-9]{4}/, "") + elsif actual_line&.match?(/Signature-[a-f0-9]{4}/) + actual_line.sub!(/Signature-[a-f0-9]{4}/, "") + expected_line.sub!(/Signature-[a-f0-9]{4}/, "") end assert_equal(expected_line, actual_line) @@ -59,13 +59,13 @@ def test_document_without_interactive_form expected_output.readlines.zip(output_pdf.readlines).each do |(expected_line, actual_line)| # We can't verify the signature because it changes on every run - if T.must(actual_line).match?("/Type /Sig") - T.must(actual_line).sub!(/<[a-f0-9]+>/, "") - T.must(expected_line).sub!(/<[a-f0-9]+>/, "") + if actual_line&.match?("/Type /Sig") + actual_line.sub!(/<[a-f0-9]+>/, "") + expected_line.sub!(/<[a-f0-9]+>/, "") # The signature field name is also random - elsif T.must(actual_line).match?("Signature-[a-f0-9]{4}") - T.must(actual_line).sub!(/Signature-[a-f0-9]{4}/, "") - T.must(expected_line).sub!(/Signature-[a-f0-9]{4}/, "") + elsif actual_line&.match?(/Signature-[a-f0-9]{4}/) + actual_line.sub!(/Signature-[a-f0-9]{4}/, "") + expected_line.sub!(/Signature-[a-f0-9]{4}/, "") end assert_equal(expected_line, actual_line)