Code review and fixes

- Fix Pkcs12 filer to use case insensitive username and realm
- Handle nil values in `StoredPkcs12`
- Use `fallbacks` options in `ldap_login`
- Small fixes

Author: cdelafuente-r7

lib/msf/core/exploit/remote/pkcs12/storage.rb

@@ -44,8 +44,6 @@ def filter_pkcs12(options)
 
       filter = {}
       filter[:id] = options[:id] if options[:id].present?
-      filter[:user] = options[:username] if options[:username].present?
-      filter[:realm] = options[:realm] if options[:realm].present?
 
       creds = framework.db.creds(
         workspace: options.fetch(:workspace) { workspace },
@@ -54,6 +52,15 @@ def filter_pkcs12(options)
       ).select do |cred|
         # this is needed since if a filter is provided (e.g. `id:`) framework.db.creds will ignore the type:
         next false unless cred.private.type == 'Metasploit::Credential::Pkcs12'
+
+        if options[:username].present?
+          next false if options[:username].casecmp(cred.public.username) != 0
+        end
+
+        if options[:realm].present? && cred.realm
+          next false if options[:realm].casecmp(cred.realm.value) != 0
+        end
+
         if options[:status].present?
           # If status is not set on the credential, considere it is `active`
           status = cred.private.status || 'active'

lib/msf/core/exploit/remote/pkcs12/stored_pkcs12.rb

@@ -14,27 +14,27 @@ def openssl_pkcs12
     end
 
     def adcs_ca
-      private_cred.adcs_ca
+      private_cred.adcs_ca || ''
     end
 
     def adcs_template
-      private_cred.adcs_template
+      private_cred.adcs_template || ''
     end
 
     def private_cred
       @pkcs12.private
     end
 
     def username
-      @pkcs12.public.username
+      @pkcs12.public&.username || ''
     end
 
     def realm
-      @pkcs12.realm.value
+      @pkcs12.realm&.value || ''
     end
 
     def status
-      private_cred.status
+      private_cred.status || ''
     end
 
     # @return [TrueClass, FalseClass] True if the certificate is valid within the not_before/not_after, false otherwise

modules/auxiliary/scanner/ldap/ldap_login.rb

@@ -37,9 +37,9 @@ def initialize(info = {})
           'APPEND_DOMAIN', [true, 'Appends `@<DOMAIN> to the username for authentication`', false],
           conditions: ['LDAP::Auth', 'in', [Msf::Exploit::Remote::AuthOption::AUTO, Msf::Exploit::Remote::AuthOption::PLAINTEXT]]
         ),
-        Msf::OptString.new('LDAPDomain', [false, 'The domain to authenticate to']),
-        Msf::OptString.new('LDAPUsername', [false, 'The username to authenticate with'], aliases: ['BIND_DN']),
-        Msf::OptString.new('LDAPPassword', [false, 'The password to authenticate with'], aliases: ['BIND_PW']),
+        Msf::OptString.new('LDAPDomain', [false, 'The domain to authenticate to'], fallbacks: ['DOMAIN']),
+        Msf::OptString.new('LDAPUsername', [false, 'The username to authenticate with'], fallbacks: ['USERNAME'], aliases: ['BIND_DN']),
+        Msf::OptString.new('LDAPPassword', [false, 'The password to authenticate with'], fallbacks: ['PASSWORD'], aliases: ['BIND_PW']),
         OptInt.new('SessionKeepalive', [true, 'Time (in seconds) for sending protocol-level keepalive messages', 10 * 60])
       ]
     )
@@ -93,10 +93,13 @@ def validate_connect_options!
   end
 
   def run_host(ip)
-    ignore_public = datastore['LDAP::Auth'] == Msf::Exploit::Remote::AuthOption::SCHANNEL
-    ignore_private =
-      datastore['LDAP::Auth'] == Msf::Exploit::Remote::AuthOption::SCHANNEL ||
-      (Msf::Exploit::Remote::AuthOption::KERBEROS && !datastore['ANONYMOUS_LOGIN'] && !datastore['LDAPPassword'])
+    ignore_public = ignore_private = false
+    case datastore['LDAP::Auth']
+    when Msf::Exploit::Remote::AuthOption::SCHANNEL
+      ignore_public = ignore_private = true
+    when Msf::Exploit::Remote::AuthOption::KERBEROS
+      ignore_private = !datastore['ANONYMOUS_LOGIN'] && !datastore['LDAPPassword']
+    end
 
     cred_collection = build_credential_collection(
       username: datastore['LDAPUsername'],
@@ -109,7 +112,7 @@ def run_host(ip)
     )
 
     opts = {
-      ldap_domain: datastore['LDAPDomain'],
+      domain: datastore['LDAPDomain'],
       append_domain: datastore['APPEND_DOMAIN'],
       ssl: datastore['SSL'],
       proxies: datastore['PROXIES'],