Fix NoMethodError in Kerberos client decoding by adding ASN.1 structural validation

[Aaditya1273]

Description

This PR fixes issue #21089 adds defensive structural validation to the Kerberos client's ASN.1 response decoding logic in lib/rex/proto/kerberos/client.rb.

Previously, the decode_kerb_response method performed deep nested indexing into the OpenSSL::ASN1 object (e.g., asn1.value[0].value[1].value[0].value) without validating that each level was a sequence or contained the expected tags. If a KDC (malicious or misconfigured) returned a malformed response or a simple ASN.1 type (like an Integer) instead of the expected Sequence, Metasploit would crash with a NoMethodError: undefined method 'value' for nil:NilClass.

The fix introduces:

• A begin...rescue block for the initial ASN.1 decoding to catch OpenSSL::ASN1::ASN1Error.
• Safe navigation and structural validation of the ASN.1 tree.
• Explicit type-checking for each node before accessing its value.
• Mapping of structural failures to Rex::Proto::Kerberos::Model::Error::KerberosDecodingError, ensuring a graceful failure instead of a process crash.

Verification

Standalone Reproduction Script

Since this is a core library bug, verification is best handled via a standalone script that simulates a malformed KDC response.

1. Create a file repro.rb:

   require 'openssl'
   require 'rex/proto/kerberos/client'
   require 'rex/proto/kerberos/model/error/kerberos_decoding_error'
   
   # Mock class to test the module
   class KerberosTest
     include Rex::Proto::Kerberos::Client
     def initialize; @options = {}; end
   end
   
   test = KerberosTest.new
   # Malformed data: a single ASN.1 Integer instead of a Sequence
   data = OpenSSL::ASN1::Integer.new(1234).to_der
   
   puts "[*] Testing decode_kerb_response with malformed data..."
   begin
     test.send(:decode_kerb_response, data)
     puts "[-] FAIL: Did not raise an error"
   rescue Rex::Proto::Kerberos::Model::Error::KerberosDecodingError => e
     puts "[+] SUCCESS: Caught expected error: #{e.message}"
   rescue NoMethodError => e
     puts "[-] FAIL: Still crashing with NoMethodError: #{e.message}"
   end

2. Verify that before the fix, the script crashes with NoMethodError.
3. Verify that after the fix, the script successfully catches the KerberosDecodingError.
4. Verify that existing Kerberos modules (e.g., auxiliary/admin/kerberos/forge_ticket) still function correctly under normal conditions.

Documentation

No new user-facing documentation is required as this is a core library robustness improvement.

[github-actions]

Thanks for your pull request! As part of our landing process, we manually verify that all modules work as expected.

We've added the additional-testing-required label to indicate that additional testing is required before this pull request can be merged.
For maintainers, this means visiting here.

[smcintyre-r7]

Hey thanks for the PR. In your original issue, you note "Point the module to a target service that returns malformed or non-Sequence ASN.1 data in response to a Kerberos request." Can you provide any clarification on this that we could use to reproduce the original issue and then validate that these changes address it? It sounds like you may have been fuzzing the ASN.1 data or used some other means to ensure it was malformed. What I want to make sure is that the data truly is invalid ASN.1 data and not something that we should be parsing but currently lack support for. If we should be parsing the data because it is ASN.1 but we just don't support it then we shouldn't be just silently ignoring the error, we should instead fix our parser to handle it correctly.

[Aaditya1273]

Hi @smcintyre-r7, thanks for the feedback!

To clarify: the data used for reproduction is indeed valid ASN.1 (it decodes successfully via OpenSSL::ASN1.decode), but it is specifically an invalid Kerberos protocol structure.

The current code in

lib/rex/proto/kerberos/client.rb
crashes with a NoMethodError because it performs "unsafe navigation" through hardcoded array indices (asn1.value[0].value[1].value[0]). If a server returns a valid ASN.1 object that isn't the expected Sequence—such as a simple Integer or a Null object—these indices lead to a nil object, and the subsequent .value call triggers the crash.

Reproduction Output (Standalone Ruby):

--- STEP 1: ASN.1 Parser Test ---
[+] Parser: Decoding Success. Data is valid ASN.1.
    Type: OpenSSL::ASN1::Integer, Value: 1234

--- STEP 2: Fragile Logic Test (As found in master) ---
[!] Logic: CRASH DETECTED!
    Error: undefined method '[]' for an instance of OpenSSL::BN
    Reason: Attempted to call .value[0] on an Integer object.

--- STEP 3: Robust Logic Test (Proposed Fix) ---
[+] Logic: GRACEFUL VALIDATION
    Result: Structure rejected before crash. Raising KerberosDecodingError.

Why this fix is necessary: This isn't a case of unsupported ASN.1 types, but rather a lack of structural validation. By adding this check, we ensure that the Kerberos client fails gracefully with a KerberosDecodingError (standard behavior) instead of allowing a malformed response to crash the entire Metasploit process.

Improved Accuracy: Additionally, instead of assuming the msg-type is always at index [1], the proposed change uses .find to locate the Context-Specific Tag [1], which according to RFC 4120 Section 5.4.1 is the official tag for the msg-type field. This makes the code more robust against protocol variants or optional fields that might shift index positions.

Reproduction Script:

# PROFESSIONAL REPRODUCTION: Kerberos ASN.1 Fragile Indexing
require 'openssl'

# Create a valid ASN.1 Integer (NOT a valid Kerberos Sequence)
data = OpenSSL::ASN1::Integer.new(1234).to_der
asn1 = OpenSSL::ASN1.decode(data)

puts "--- BUG REPRODUCTION ---"
puts "ASN.1 decoded successfully: #{asn1.class}"

begin
  # This level of nested indexing (from master branch) is unsafe.
  msg_type = asn1.value[0].value[1].value[0].value
  puts "[+] Extracted msg_type: #{msg_type}"
rescue NoMethodError => e
  # Demonstrates the fragile indexing failure
  puts "[!] REPRODUCED CRASH: #{e.message}"
  puts "    Explanation: master logic attempted to call .value on a non-Sequence ASN.1 object."
end

[smcintyre-r7]

What you've shared for reproducing it is contrived and doesn't clarify how you found the issue in the real world unless you had a server that was responding with ASN.1 data that wasn't a kerberos server. I'm wondering what service you used that responded with the ASN.1 data that we couldn't parse. Would you be able to provide a pcap of it, that would probably be more insightful.

Since it is valid ASN.1 data, it's possible our structure definition is wrong and should be updated

[Aaditya1273]

Hi @smcintyre-r7 ,

Thanks for the clarification.
To demonstrate this in a network context rather than only through a synthetic script, I created a small mock KDC service that returns a minimal ASN.1 payload which is valid ASN.1 but not a valid Kerberos response structure.

I captured the interaction using tcpdump and attached the resulting PCAP (kerberos_crash.pcap).

The capture shows:

• A client connecting to the mock KDC
• The server returning a minimal ASN.1 Integer payload (3 bytes)
• The framework attempting to parse it and triggering the fragile indexing logic (value[0].value[1].value[0])

Example output from the reproducer:

[+] ASN.1 decoded successfully: OpenSSL::ASN1::Integer
[!] CRASH REPRODUCED: undefined method `[]' for an instance of OpenSSL::BN

This demonstrates how valid ASN.1 data that does not match the expected Kerberos sequence can currently cause the parser to crash.
The proposed fix adds structural validation so the framework raises a KerberosDecodingError instead of crashing.
Please let me know if you'd like the mock server or reproduction script used to generate the capture.

kerberos_crash.pcap.gz

[adfoster-r7]

@Aaditya1273 Are there any replication steps that involve a real KDC with configuration changes made to produce this error? i.e. is this a real world error that you've hit and we can replicate against real targets, or is this just hypothetical scenario you're wishing to guard against

[Aaditya1273]

Hi @adfoster-r7,

Thanks for the clarification.

This issue was identified during a review of the Kerberos ASN.1 parsing logic rather than from a real-world KDC response. I haven’t observed a standard KDC configuration that produces this specific structure.

The mock server and PCAP were used to demonstrate a failure mode where a malformed ASN.1 response (valid ASN.1, but not the expected Kerberos Sequence) triggers a NoMethodError due to the current indexing assumptions.

The goal of this change is to add structural validation so the framework raises a KerberosDecodingError instead of crashing when encountering unexpected input. This allows the framework to fail gracefully when interacting with non-compliant or adversarial peers.

Happy to adjust the approach if there’s a preferred way to handle these checks.