From ad7e324eee61e81f47e894d26579217c78a0d3b2 Mon Sep 17 00:00:00 2001 From: Nimesh-Msys Date: Fri, 28 Dec 2018 15:51:18 +0530 Subject: [PATCH] Fixing deletion of a certificate by its thumbprint. - Using the thumbprint directly to fetch a certificate - Using `CertFindCertificateInStore` with `CRYPT_HASH_BLOB` structure to achieve this. - Minor cleanup and refactor. - Minor update as per the review comment - Consider `store_name` in powershell command while fetching certificate details by thumbprint - Fixes MSYS-951 and MSYS-937 Signed-off-by: Nimesh-Msys --- lib/win32/certstore/mixin/crypto.rb | 26 +++++++++++++++++++-- lib/win32/certstore/mixin/unicode.rb | 8 ------- lib/win32/certstore/store_base.rb | 22 +++++------------- spec/win32/unit/certstore_spec.rb | 34 ++++++++-------------------- 4 files changed, 39 insertions(+), 51 deletions(-) diff --git a/lib/win32/certstore/mixin/crypto.rb b/lib/win32/certstore/mixin/crypto.rb index 44a33c9..97409e4 100644 --- a/lib/win32/certstore/mixin/crypto.rb +++ b/lib/win32/certstore/mixin/crypto.rb @@ -55,6 +55,7 @@ def safe_attach_function(win32_func, *args) PKCS_7_ASN_ENCODING = 0x00010000 PKCS_7_NDR_ENCODING = 0x00020000 PKCS_7_OR_X509_ASN_ENCODING = (PKCS_7_ASN_ENCODING | X509_ASN_ENCODING) + ENCODING_TYPE = X509_ASN_ENCODING | PKCS_7_ASN_ENCODING # Certificate Display Format CERT_NAME_EMAIL_TYPE = 1 @@ -67,8 +68,12 @@ def safe_attach_function(win32_func, *args) CERT_NAME_UPN_TYPE = 8 # Retrieve Certificates flag - CERT_FIND_SUBJECT_STR = 0x00080007 - CERT_FIND_ISSUER_STR = 0x00080004 + CERT_COMPARE_SHA1_HASH = 1 + CERT_INFO_SUBJECT_FLAG = 7 + CERT_COMPARE_NAME_STR_W = 8 + CERT_COMPARE_SHIFT = 16 + CERT_FIND_SHA1_HASH = CERT_COMPARE_SHA1_HASH << CERT_COMPARE_SHIFT + CERT_FIND_SUBJECT_STR = CERT_COMPARE_NAME_STR_W << CERT_COMPARE_SHIFT | CERT_INFO_SUBJECT_FLAG # List Certificates Flag CERT_NAME_ISSUER_FLAG = 0x1 @@ -122,6 +127,23 @@ def initialize(str = nil) end end + class CRYPT_HASH_BLOB < FFI::Struct + layout :cbData, DWORD, # Count, in bytes, of data + :pbData, :pointer # Pointer to data buffer + + def initialize(str = nil) + super(nil) + if str + byte_arr = [str].pack("H*").unpack("C*") # Converting string to its byte array + + buffer = FFI::MemoryPointer.new(:char, byte_arr.size) # Create the pointer to the array + buffer.put_array_of_char 0, byte_arr # Fill the memory location with data + self[:pbData] = buffer + self[:cbData] = byte_arr.size + end + end + end + class CERT_EXTENSION < FFI::Struct layout :pszObjId, LPTSTR, :fCritical, BOOL, diff --git a/lib/win32/certstore/mixin/unicode.rb b/lib/win32/certstore/mixin/unicode.rb index c501425..1d0b173 100644 --- a/lib/win32/certstore/mixin/unicode.rb +++ b/lib/win32/certstore/mixin/unicode.rb @@ -40,11 +40,3 @@ def read_wstring(num_wchars = nil) end end end - -class String - include Win32::Certstore::Mixin::String - - def to_wstring - utf8_to_wide(self) - end -end diff --git a/lib/win32/certstore/store_base.rb b/lib/win32/certstore/store_base.rb index b5c937c..e575032 100644 --- a/lib/win32/certstore/store_base.rb +++ b/lib/win32/certstore/store_base.rb @@ -85,21 +85,16 @@ def cert_list(store_handler) def cert_delete(store_handler, certificate_thumbprint) validate_thumbprint(certificate_thumbprint) thumbprint = update_thumbprint(certificate_thumbprint) - cert_pem = format_pem(get_cert_pem(thumbprint)) - cert_rdn = get_rdn(build_openssl_obj(cert_pem)) unless cert_pem.empty? cert_delete_flag = false begin - cert_args = cert_find_args(store_handler, cert_rdn) - if (pcert_context = CertFindCertificateInStore(*cert_args)) && !pcert_context.null? + cert_args = cert_find_args(store_handler, thumbprint) + pcert_context = CertFindCertificateInStore(*cert_args) + if !pcert_context.null? cert_delete_flag = CertDeleteCertificateFromStore(CertDuplicateCertificateContext(pcert_context)) || lookup_error end CertFreeCertificateContext(pcert_context) rescue - if cert_pem.empty? - raise SystemCallError.new("Invalid thumbprint #{certificate_thumbprint}.") - else - lookup_error("delete") - end + lookup_error("delete") end cert_delete_flag end @@ -143,8 +138,8 @@ def cert_add_args(store_handler, certificate_obj) end # Build arguments for CertFindCertificateInStore - def cert_find_args(store_handler, cert_rdn) - [store_handler, X509_ASN_ENCODING, 0, CERT_FIND_ISSUER_STR, cert_rdn.to_wstring, nil] + def cert_find_args(store_handler, thumbprint) + [store_handler, ENCODING_TYPE, 0, CERT_FIND_SHA1_HASH, CRYPT_HASH_BLOB.new(thumbprint), nil] end # Match certificate CN exist in cert_rdn @@ -191,11 +186,6 @@ def get_cert_pem(thumbprint) get_data.stdout end - # To get RDN from certificate object - def get_rdn(cert_obj) - cert_obj.issuer.to_s.concat("/").scan(/=(.*?)\//).join(", ") - end - # Format pem def format_pem(cert_pem) cert_pem.delete("\r") diff --git a/spec/win32/unit/certstore_spec.rb b/spec/win32/unit/certstore_spec.rb index b5250bb..c246d80 100644 --- a/spec/win32/unit/certstore_spec.rb +++ b/spec/win32/unit/certstore_spec.rb @@ -227,41 +227,25 @@ context "When passing valid thumbprint" do let(:store_name) { "root" } - let(:thumbprint) { "b1bc968bd4f49d622aa89a81f2150152a41d829909c" } let(:cert_pem) { File.read('.\spec\win32\unit\assets\GlobalSignRootCA.pem') } before(:each) do - allow_any_instance_of(certbase).to receive(:get_cert_pem).and_return(cert_pem) + allow_any_instance_of(certbase).to receive(:CertFindCertificateInStore).and_return(FFI::MemoryPointer.new(1)) + allow_any_instance_of(certbase).to receive(:CertDuplicateCertificateContext).and_return(true) allow_any_instance_of(certbase).to receive(:CertDeleteCertificateFromStore).and_return(true) + allow_any_instance_of(certbase).to receive(:CertFreeCertificateContext).and_return(true) end - it "returns true" do + it "returns true when thumbprint has no spaces" do + thumbprint = "b1bc968bd4f49d622aa89a81f2150152a41d829909c" store = certstore.open(store_name) expect(store.delete(thumbprint)).to eql(true) end - end - - context "When passing valid thumbprint with spaces" do - let(:store_name) { "root" } - let(:thumbprint) { "b1 bc 96 8b d4 f4 9d 62 2a a8 9a 81 f2 15 01 52 a4 1d 82 9c" } - let(:cert_pem) { File.read('.\spec\win32\unit\assets\GlobalSignRootCA.pem') } - before(:each) do - allow_any_instance_of(certbase).to receive(:get_cert_pem).and_return(cert_pem) - allow_any_instance_of(certbase).to receive(:CertDeleteCertificateFromStore).and_return(true) - end - it "returns true" do + it "returns true when thumbprint has spaces" do + thumbprint = "b1 bc 96 8b d4 f4 9d 62 2a a8 9a 81 f2 15 01 52 a4 1d 82 9c" store = certstore.open(store_name) expect(store.delete(thumbprint)).to eql(true) end - end - - context "When passing valid thumbprint with :" do - let(:store_name) { "root" } - let(:thumbprint) { "b1:bc:96:8b:d4:f4:9d:62:2a:a8:9a:81:f2:15:01:52:a4:1d:82:9c" } - let(:cert_pem) { File.read('.\spec\win32\unit\assets\GlobalSignRootCA.pem') } - before(:each) do - allow_any_instance_of(certbase).to receive(:get_cert_pem).and_return(cert_pem) - allow_any_instance_of(certbase).to receive(:CertDeleteCertificateFromStore).and_return(true) - end - it "returns true" do + it "returns true when thumbprint is colon(:) seperated" do + thumbprint = "b1:bc:96:8b:d4:f4:9d:62:2a:a8:9a:81:f2:15:01:52:a4:1d:82:9c" store = certstore.open(store_name) expect(store.delete(thumbprint)).to eql(true) end