Merge pull request #20895 from rudraditya21/fixed/ldap-entry-cache-misses

added: negative caching for LDAP lookup misses

Author: sjanusz-r7

lib/msf/core/exploit/remote/ldap/active_directory.rb

@@ -156,12 +156,17 @@ def adds_query_member_groups(ldap, member_dn, base_dn: nil, inherited: true)
       # @rtype [Net::LDAP::Entry,nil]
       def adds_get_object_by_dn(ldap, object_dn)
         object = ldap_entry_cache.get_by_dn(object_dn)
+        return nil if ldap_entry_cache.missing_entry?(object)
         return object if object
 
         object = ldap.search(base: object_dn, controls: [adds_build_ldap_sd_control], scope: Net::LDAP::SearchScope_BaseObject)&.first
         validate_query_result!(ldap.get_operation_result.table)
 
-        ldap_entry_cache << object if object
+        if object
+          ldap_entry_cache << object
+        else
+          ldap_entry_cache.mark_missing_by_dn(object_dn)
+        end
         object
       end
 
@@ -173,6 +178,7 @@ def adds_get_object_by_dn(ldap, object_dn)
       # @rtype [Net::LDAP::Entry,nil]
       def adds_get_object_by_samaccountname(ldap, object_samaccountname)
         object = ldap_entry_cache.get_by_samaccountname(object_samaccountname)
+        return nil if ldap_entry_cache.missing_entry?(object)
         return object if object
 
         filter = "(sAMAccountName=#{ldap_escape_filter(object_samaccountname)})"
@@ -184,7 +190,11 @@ def adds_get_object_by_samaccountname(ldap, object_samaccountname)
         end
         validate_query_result!(ldap.get_operation_result.table, filter)
 
-        ldap_entry_cache << object if object
+        if object
+          ldap_entry_cache << object
+        else
+          ldap_entry_cache.mark_missing_by_samaccountname(object_samaccountname)
+        end
         object
       end
 
@@ -197,13 +207,18 @@ def adds_get_object_by_samaccountname(ldap, object_samaccountname)
       def adds_get_object_by_sid(ldap, object_sid)
         object_sid = Rex::Proto::MsDtyp::MsDtypSid.new(object_sid)
         object = ldap_entry_cache.get_by_sid(object_sid)
+        return nil if ldap_entry_cache.missing_entry?(object)
         return object if object
 
         filter = "(objectSID=#{ldap_escape_filter(object_sid.to_s)})"
         object = ldap.search(base: ldap.base_dn, controls: [adds_build_ldap_sd_control], filter: filter)&.first
         validate_query_result!(ldap.get_operation_result.table, filter)
 
-        ldap_entry_cache << object if object
+        if object
+          ldap_entry_cache << object
+        else
+          ldap_entry_cache.mark_missing_by_sid(object_sid)
+        end
         object
       end
 

lib/msf/core/exploit/remote/ldap/entry_cache.rb

@@ -9,6 +9,14 @@ module Msf
   module Exploit::Remote::LDAP
     module EntryCache
       class LDAPEntryCache < LruRedux::Cache
+        MissingEntry = Object.new.freeze
+
+        def initialize(max_size: 1000)
+          super(max_size)
+          @missing_samaccountname = LruRedux::Cache.new(max_size)
+          @missing_sid = LruRedux::Cache.new(max_size)
+        end
+
         def <<(entry)
           raise TypeError unless entry.is_a? Net::LDAP::Entry
 
@@ -20,22 +28,43 @@ def get_by_dn(dn)
         end
 
         def get_by_samaccountname(samaccountname)
-          entry = @data.values.reverse_each.find { _1[:sAMAccountName]&.first == samaccountname }
+          entry = @data.values.reverse_each.find { _1.is_a?(Net::LDAP::Entry) && _1[:sAMAccountName]&.first == samaccountname }
           @data[entry.dn] = entry if entry # update it as recently used
-          entry
+          return entry if entry
+
+          MissingEntry if @missing_samaccountname[samaccountname]
         end
 
         def get_by_sid(sid)
           sid = Rex::Proto::MsDtyp::MsDtypSid.new(sid)
 
-          entry = @data.values.reverse_each.find { _1[:objectSid]&.first == sid.to_binary_s  }
+          entry = @data.values.reverse_each.find { _1.is_a?(Net::LDAP::Entry) && _1[:objectSid]&.first == sid.to_binary_s  }
           @data[entry.dn] = entry if entry # update it as recently used
-          entry
+          return entry if entry
+
+          MissingEntry if @missing_sid[sid.to_s]
+        end
+
+        def mark_missing_by_dn(dn)
+          self[dn] = MissingEntry
+        end
+
+        def mark_missing_by_samaccountname(samaccountname)
+          @missing_samaccountname[samaccountname] = true
+        end
+
+        def mark_missing_by_sid(sid)
+          sid = Rex::Proto::MsDtyp::MsDtypSid.new(sid)
+          @missing_sid[sid.to_s] = true
+        end
+
+        def missing_entry?(entry)
+          entry.equal?(MissingEntry)
         end
       end
 
       def ldap_entry_cache
-        @ldap_entry_cache ||= LDAPEntryCache.new(1000)
+        @ldap_entry_cache ||= LDAPEntryCache.new(max_size: 1000)
       end
     end
   end

spec/lib/msf/core/exploit/remote/ldap/active_directory/entry_cache_spec.rb

@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'net/ldap'
+require 'msf/core/exploit/remote/ldap/active_directory'
+
+RSpec.describe Msf::Exploit::Remote::LDAP::ActiveDirectory do
+  subject(:helper) { Object.new.extend(described_class) }
+
+  describe '#adds_get_object_by_samaccountname' do
+    it 'returns nil without querying LDAP when a miss is cached' do
+      ldap = Object.new
+      helper.ldap_entry_cache.mark_missing_by_samaccountname('missing')
+
+      expect(helper.adds_get_object_by_samaccountname(ldap, 'missing')).to be_nil
+    end
+  end
+
+  describe '#adds_get_object_by_sid' do
+    it 'returns nil without querying LDAP when a miss is cached' do
+      ldap = Object.new
+      helper.ldap_entry_cache.mark_missing_by_sid('S-1-5-21-111111111-222222222-333333333-4444')
+
+      expect(helper.adds_get_object_by_sid(ldap, 'S-1-5-21-111111111-222222222-333333333-4444')).to be_nil
+    end
+  end
+
+  describe '#adds_get_object_by_dn' do
+    it 'returns nil without querying LDAP when a miss is cached' do
+      ldap = Object.new
+      helper.ldap_entry_cache.mark_missing_by_dn('cn=Missing,dc=example,dc=com')
+
+      expect(helper.adds_get_object_by_dn(ldap, 'cn=Missing,dc=example,dc=com')).to be_nil
+    end
+  end
+end

spec/lib/msf/core/exploit/remote/ldap/entry_cache_spec.rb

@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'net/ldap'
+require 'msf/core/exploit/remote/ldap/entry_cache'
+
+RSpec.describe Msf::Exploit::Remote::LDAP::EntryCache::LDAPEntryCache do
+  subject(:cache) { described_class.new(max_size: 1000) }
+
+  let(:dn) { 'cn=User One,dc=example,dc=com' }
+  let(:samaccountname) { 'user1' }
+  let(:sid) { 'S-1-5-21-111111111-222222222-333333333-4444' }
+
+  describe '#get_by_dn' do
+    it 'returns a missing sentinel when a DN miss is cached' do
+      cache.mark_missing_by_dn(dn)
+      expect(cache.get_by_dn(dn)).to eq described_class::MissingEntry
+    end
+  end
+
+  describe '#get_by_samaccountname' do
+    it 'returns a cached entry when present' do
+      entry = Net::LDAP::Entry.new(dn)
+      entry[:sAMAccountName] = [samaccountname]
+      cache << entry
+
+      expect(cache.get_by_samaccountname(samaccountname)).to eq entry
+    end
+
+    it 'returns a missing sentinel when a sAMAccountName miss is cached' do
+      cache.mark_missing_by_samaccountname(samaccountname)
+      expect(cache.get_by_samaccountname(samaccountname)).to eq described_class::MissingEntry
+    end
+  end
+
+  describe '#get_by_sid' do
+    it 'returns a cached entry when present' do
+      entry = Net::LDAP::Entry.new(dn)
+      entry[:objectSid] = [Rex::Proto::MsDtyp::MsDtypSid.new(sid).to_binary_s]
+      cache << entry
+
+      expect(cache.get_by_sid(sid)).to eq entry
+    end
+
+    it 'returns a missing sentinel when a SID miss is cached' do
+      cache.mark_missing_by_sid(sid)
+      expect(cache.get_by_sid(sid)).to eq described_class::MissingEntry
+    end
+  end
+end