From 0ba600f71f0d88964fa7a5f160c0d1881c0878ca Mon Sep 17 00:00:00 2001 From: rishichawda Date: Tue, 7 Sep 2021 16:11:34 +0530 Subject: [PATCH 1/4] don't raise error in get if certificate is empty Signed-off-by: rishichawda --- lib/win32/certstore/store_base.rb | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/lib/win32/certstore/store_base.rb b/lib/win32/certstore/store_base.rb index 0156a32..4cc23c5 100644 --- a/lib/win32/certstore/store_base.rb +++ b/lib/win32/certstore/store_base.rb @@ -92,13 +92,8 @@ def cert_get(certificate_thumbprint, store_name:, store_location:) thumbprint = update_thumbprint(certificate_thumbprint) cert_pem = get_cert_pem(thumbprint, store_name: store_name, store_location: store_location) cert_pem = format_pem(cert_pem) - if cert_pem.empty? - raise ArgumentError, "Unable to retrieve the certificate" - end - unless cert_pem.empty? - build_openssl_obj(cert_pem) - end + cert_pem.empty? ? cert_pem : build_openssl_obj(cert_pem) end # Listing certificate of open certstore and return list in json From df7f4954232ba3510ab509001dd8f6f0bedc143b Mon Sep 17 00:00:00 2001 From: rishichawda Date: Tue, 7 Sep 2021 18:12:46 +0530 Subject: [PATCH 2/4] add get! method, remove string return from verify_certificate Signed-off-by: rishichawda --- lib/win32/certstore.rb | 11 +++++++++++ lib/win32/certstore/store_base.rb | 2 -- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/win32/certstore.rb b/lib/win32/certstore.rb index 363297c..1af8567 100644 --- a/lib/win32/certstore.rb +++ b/lib/win32/certstore.rb @@ -78,6 +78,17 @@ def get(certificate_thumbprint, store_name: @store_name, store_location: @store_ cert_get(certificate_thumbprint, store_name: store_name, store_location: store_location) end + # Return `OpenSSL::X509` certificate object if present otherwise raise a "Certificate not found!" error + # @param request [thumbprint] of certificate + # @return [Object] of certificates in OpenSSL::X509 format + def get!(certificate_thumbprint, store_name: @store_name, store_location: @store_location) + cert_pem = cert_get(certificate_thumbprint, store_name: store_name, store_location: store_location) + + raise ArgumentError, "Unable to retrieve the certificate" if cert_pem.empty? + + cert_pem + end + # Returns all the certificates in a store # @param [nil] # @return [Array] array of certificates list diff --git a/lib/win32/certstore/store_base.rb b/lib/win32/certstore/store_base.rb index 4cc23c5..45346a1 100644 --- a/lib/win32/certstore/store_base.rb +++ b/lib/win32/certstore/store_base.rb @@ -218,8 +218,6 @@ def update_thumbprint(certificate_thumbprint) # Verify OpenSSL::X509::Certificate object def verify_certificate(cert_pem) - return "Certificate not found" if cert_pem.empty? - valid_duration?(build_openssl_obj(cert_pem)) end From c52c0fd44c31941efac39b1f61f547429fd9fc5c Mon Sep 17 00:00:00 2001 From: rishichawda Date: Tue, 7 Sep 2021 18:37:27 +0530 Subject: [PATCH 3/4] update tests Signed-off-by: rishichawda --- spec/win32/functional/win32/certstore_spec.rb | 21 ++++++++++++++- spec/win32/unit/certstore_spec.rb | 26 +++++++++++-------- 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/spec/win32/functional/win32/certstore_spec.rb b/spec/win32/functional/win32/certstore_spec.rb index 064a44e..a41a94a 100644 --- a/spec/win32/functional/win32/certstore_spec.rb +++ b/spec/win32/functional/win32/certstore_spec.rb @@ -38,10 +38,29 @@ @store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: "My") end + # passing invalid thumbprint + it "returns nil if certificate not found" do + thumbprint = "b1bc968bd4f49d622aa89a81f2150152a41d829cab" + cert_obj = @store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: "My") + expect(cert_obj).to be_empty + end + end + + describe "#get!" do + before { add_cert } + let(:cert_pem) { File.read('.\spec\win32\assets\GlobalSignRootCA.pem') } + + # passing valid thumbprint + it "returns the certificate_object if found" do + thumbprint = "b1bc968bd4f49d622aa89a81f2150152a41d829c" + expect(@store).to receive(:cert_get).with(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: "My").and_return(cert_pem) + @store.get!(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: "My") + end + # passing invalid thumbprint it "returns ArgumentError if certificate not found" do thumbprint = "b1bc968bd4f49d622aa89a81f2150152a41d829cab" - expect { @store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: "My") }.to raise_error(ArgumentError) + expect { @store.get!(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: "My") }.to raise_error(ArgumentError) end end diff --git a/spec/win32/unit/certstore_spec.rb b/spec/win32/unit/certstore_spec.rb index 27af68c..b71b3ef 100644 --- a/spec/win32/unit/certstore_spec.rb +++ b/spec/win32/unit/certstore_spec.rb @@ -131,9 +131,10 @@ before(:each) do allow_any_instance_of(certbase).to receive(:get_cert_pem).and_return("") end - it "raises ArgumentError" do + it "returns empty string" do store = certstore.open(store_name) - expect { store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: store_name) }.to raise_error(ArgumentError, "Unable to retrieve the certificate") + cert_obj = store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: store_name) + expect(cert_obj).to be_empty end end @@ -259,9 +260,10 @@ before(:each) do allow_any_instance_of(certbase).to receive(:get_cert_pem).and_return("") end - it "raises Error" do + it "returns empty string" do store = certstore.open(store_name) - expect { store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: store_name) }.to raise_error(ArgumentError, "Unable to retrieve the certificate") + cert_obj = store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: store_name) + expect(cert_obj).to be_empty end end end @@ -300,7 +302,7 @@ end it "returns Certificate not found" do store = certstore.open(store_name) - expect(store.valid?(thumbprint)).to eql("Certificate not found") + expect(store.valid?(thumbprint)).to eql(false) end end @@ -597,9 +599,10 @@ before(:each) do allow_any_instance_of(certbase).to receive(:get_cert_pem).and_return("") end - it "returns nil" do + it "returns empty string" do store = certstore.open(store_name, store_location: store_location) - expect { store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: store_name) }.to raise_error(ArgumentError, "Unable to retrieve the certificate") + cert_obj = store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: store_name) + expect(cert_obj).to be_empty end end @@ -732,9 +735,10 @@ before(:each) do allow_any_instance_of(certbase).to receive(:get_cert_pem).and_return("") end - it "returns nil" do + it "returns empty string" do store = certstore.open(store_name, store_location: store_location) - expect { store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: store_name) }.to raise_error(ArgumentError, "Unable to retrieve the certificate") + cert_obj = store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: store_name) + expect(cert_obj).to be_empty end end end @@ -775,9 +779,9 @@ before(:each) do allow_any_instance_of(certbase).to receive(:get_cert_pem).and_return("") end - it "returns Certificate not found" do + it "returns false" do store = certstore.open(store_name, store_location: store_location) - expect(store.valid?(thumbprint)).to eql("Certificate not found") + expect(store.valid?(thumbprint)).to eql(false) end end From 0049fca6c80682846ae138d5eeab858ab3168b34 Mon Sep 17 00:00:00 2001 From: rishichawda Date: Thu, 16 Sep 2021 22:31:35 +0530 Subject: [PATCH 4/4] return only boolean on valid? method Signed-off-by: rishichawda --- lib/win32/certstore.rb | 8 +++++++- lib/win32/certstore/store_base.rb | 2 ++ spec/win32/unit/certstore_spec.rb | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/win32/certstore.rb b/lib/win32/certstore.rb index 1af8567..326e4eb 100644 --- a/lib/win32/certstore.rb +++ b/lib/win32/certstore.rb @@ -114,7 +114,13 @@ def search(search_token) # @param request[thumbprint] of certificate # @return [true, false] only true or false def valid?(certificate_thumbprint, store_location: "", store_name: "") - cert_validate(certificate_thumbprint, store_location: store_location, store_name: store_name) + cert_validate(certificate_thumbprint, store_location: store_location, store_name: store_name).yield_self do |x| + if x.is_a?(TrueClass) || x.is_a?(FalseClass) + x + else + false + end + end end # To close and destroy pointer of open certificate store handler diff --git a/lib/win32/certstore/store_base.rb b/lib/win32/certstore/store_base.rb index 45346a1..c7407ca 100644 --- a/lib/win32/certstore/store_base.rb +++ b/lib/win32/certstore/store_base.rb @@ -142,6 +142,8 @@ def cert_validate(certificate_thumbprint, store_location:, store_name:) thumbprint = update_thumbprint(certificate_thumbprint) cert_pem = get_cert_pem(thumbprint, store_name: store_name, store_location: store_location) cert_pem = format_pem(cert_pem) + return "Certificate not found" if cert_pem.empty? + verify_certificate(cert_pem) end diff --git a/spec/win32/unit/certstore_spec.rb b/spec/win32/unit/certstore_spec.rb index b71b3ef..a090fa2 100644 --- a/spec/win32/unit/certstore_spec.rb +++ b/spec/win32/unit/certstore_spec.rb @@ -300,7 +300,7 @@ before(:each) do allow_any_instance_of(certbase).to receive(:get_cert_pem).and_return("") end - it "returns Certificate not found" do + it "returns false" do store = certstore.open(store_name) expect(store.valid?(thumbprint)).to eql(false) end