Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix out of scope variable with original behaviour #19619

Merged
merged 1 commit into from
Nov 6, 2024

Conversation

smashery
Copy link
Contributor

@smashery smashery commented Nov 6, 2024

This fixes a regression introduced while implementing etype-respecting klist retrieval in #19553. If no offeredetypes are provided, it would crash with an invalid variable name:

Error: Msf::Exploit::Remote::SMB::Client::Ipc::SmbIpcAuthenticationError Unable to authenticate ([Rex::Proto::SMB::Exceptions::LoginError] Login Failed: undefined local variable or method `ticket' for #<Msf::Exploit::Remote::Kerberos::Ticket::Storage::ReadWrite:0x00007fecbfdaa8a0

Verification

To test this one, I wasn't sure of any modules that allowed null etypes; so I modified the code passing the options in to overwrite it with a null entry for SMB::KrbOfferedEncryptionTypes. Without this fix, the above error was encountered. After the fix, based on WireShark logs, I found that this would always use the earliest ticket.

If there's a module that allows this testing without modifying the code, that would be preferable.

@adfoster-r7
Copy link
Contributor

This was the original stack trace from the automated tests from the get_ticket module:

Auxiliary failed: NameError undefined local variable or method `ticket' for #<Msf::Exploit::Remote::Kerberos::Ticket::Storage::ReadWrite:0x00007f8927f31f08 @framework=#<Framework (0 sessions, 0 jobs, 0 plugins, postgresql database active)>, @framework_module=#<Module:auxiliary/admin/kerberos/get_ticket datastore=[#<Msf::ModuleDataStoreWithFallbacks:0x00007f89210ae3d8 @options={\"WORKSPACE\"=>#<Msf::OptString:0x00007f8935f58668 @name=\"WORKSPACE\", @advanced=true, @evasion=false, @aliases=[], @max_length=nil, @conditions=[], @fallbacks=[], @req
...
..>]>>]>>]>>\n
[-] [2024.11.05-15:16:46] Call stack:\n
[-] [2024.11.05-15:16:46]   /home/msfadmin/pro/vendor/bundle/ruby/3.2.0/gems/metasploit-framework-6.4.34/lib/msf/core/exploit/remote/kerberos/ticket/storage/read_mixin.rb:22:in `load_credential'\n
[-] [2024.11.05-15:16:46]   /home/msfadmin/pro/vendor/bundle/ruby/3.2.0/gems/metasploit-framework-6.4.34/lib/msf/core/exploit/remote/kerberos/service_authenticator/base.rb:1035:in `get_cached_credential'\n
[-] [2024.11.05-15:16:46]   /home/msfadmin/pro/vendor/bundle/ruby/3.2.0/gems/metasploit-framework-6.4.34/lib/msf/core/exploit/remote/kerberos/service_authenticator/base.rb:353:in `request_tgt_only'\n
[-] [2024.11.05-15:16:46]   /home/msfadmin/pro/vendor/bundle/ruby/3.2.0/gems/metasploit-framework-6.4.34/modules/auxiliary/admin/kerberos/get_ticket.rb:207:in `action_get_tgs'\n
[-] [2024.11.05-15:16:46]   /home/msfadmin/pro/vendor/bundle/ruby/3.2.0/gems/metasploit-framework-6.4.34/modules/auxiliary/admin/kerberos/get_ticket.rb:145:in `run'\n

@adfoster-r7
Copy link
Contributor

adfoster-r7 commented Nov 6, 2024

Before 🔴

msf6 auxiliary(admin/kerberos/get_ticket) > run rhost=10.140.10.10 domain=ad.pro.local username=RBCD01$ password=RBCD01$P4$$W0RD SPN=cifs/mspro-dc.ad.pro.local impersonate=Administrator action=GET_TGS 
[*] Running module against 10.140.10.10

Did you mean?  tickets
[-] Call stack:
[-]   /Users/user/Documents/code/metasploit-framework/lib/msf/core/exploit/remote/kerberos/ticket/storage/read_mixin.rb:22:in `load_credential'

After 🟢

msf6 auxiliary(admin/kerberos/get_ticket) > run rhost=10.140.10.10 domain=ad.pro.local username=RBCD01$ password=RBCD01$P4$$W0RD SPN=cifs/mspro-dc.ad.pro.local impersonate=Administrator action=GET_TGT
[*] Running module against 10.140.10.10

[*] 10.140.10.10:88 - Getting TGT for RBCD01$@ad.pro.local
[+] 10.140.10.10:88 - Received a valid TGT-Response
[*] 10.140.10.10:88 - TGT MIT Credential Cache ticket saved to /Users/user/.msf4/loot/20241106112110_default_10.140.10.10_mit.kerberos.cca_601200.bin
[*] Auxiliary module execution completed
msf6 auxiliary(admin/kerberos/get_ticket) > 

To test this one, I wasn't sure of any modules that allowed null etypes

     5: def load_credential(options = {})
     6:   return nil unless active_db?
     7: 
     8:   now = Time.now.utc
     9:   available_tickets = tickets(options).select do |ticket|
    10:     !ticket.expired?(now)
    11:   end
    12:   require 'pry-byebug'; binding.pry
 => 13:   if options[:offered_etypes].present?
    14:     # Prefer etypes mentioned first
    15:     options[:offered_etypes].each do |etype|
    16:       available_tickets.each do |t|
    17:         if t.enctype == etype
    18:           return t.ccache.credentials.first
    19:         end
    20:       end
    21:     end
    22:   else
    23:     return available_tickets.first.ccache.credentials.first
    24:   end
    25: 
    26:   nil
    27: end

[2] pry(#<Msf::Exploit::Remote::Kerberos::Ticket::Storage::ReadWrite>)> options
=> {:host=>"10.140.10.10",
 :client=>"RBCD01$",
 :server=>#<Rex::Proto::Kerberos::Model::PrincipalName:0x000000011d602868 @name_string=["krbtgt", "ad.pro.local"], @name_type=2>,
 :realm=>"ad.pro.local",
 :offered_etypes=>nil}

@adfoster-r7 adfoster-r7 merged commit c27c943 into rapid7:master Nov 6, 2024
68 checks passed
@adfoster-r7
Copy link
Contributor

Release Notes

This fixes a regression crash in the auxiliary/admin/kerberos/get_ticket module

@adfoster-r7 adfoster-r7 added the rn-fix release notes fix label Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rn-fix release notes fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants