Always return an array of PreAuthData

Author: zeroSteiner

lib/metasploit/framework/login_scanner/kerberos.rb

@@ -87,8 +87,11 @@ def self.login_status_for_kerberos_error(krb_err)
             # It doesn't appear to be documented anywhere, but Microsoft gives us a bit
             # of extra information in the e-data section
             begin
-              pa_data_entry = krb_err.res.e_data_as_pa_data_entry
-              if pa_data_entry && pa_data_entry.type == Rex::Proto::Kerberos::Model::PreAuthType::PA_PW_SALT
+              pa_data_entry = krb_err.res.e_data_as_pa_data.find do |pa_data|
+                pa_data.type == Rex::Proto::Kerberos::Model::PreAuthType::PA_PW_SALT
+              end
+
+              if pa_data_entry
                 pw_salt = pa_data_entry.decoded_value
                 if pw_salt.nt_status
                   case pw_salt.nt_status.value
@@ -107,7 +110,7 @@ def self.login_status_for_kerberos_error(krb_err)
                   Metasploit::Model::Login::Status::DISABLED
                 end
               else
-                  Metasploit::Model::Login::Status::DISABLED
+                Metasploit::Model::Login::Status::DISABLED
               end
             rescue Rex::Proto::Kerberos::Model::Error::KerberosDecodingError
               # Could be a non-MS implementation?

lib/rex/proto/kerberos/model/error.rb

@@ -176,9 +176,12 @@ def message_for(error_code)
                   pa_datas = res.e_data_as_pa_data
                 rescue OpenSSL::ASN1::ASN1Error
                 else
-                  superseded_pa_data = pa_datas.find { |pa_data| pa_data.type == Rex::Proto::Kerberos::Model::PreAuthType::KERB_SUPERSEDED_BY_USER }
-                  if superseded_pa_data
-                    error_code = "#{error_code}. This account has been superseded by #{superseded_pa_data.decoded_value}."
+                  pa_data_entry = pa_datas.find do |pa_data|
+                    pa_data.type == Rex::Proto::Kerberos::Model::PreAuthType::KERB_SUPERSEDED_BY_USER
+                  end
+
+                  if pa_data_entry
+                    error_code = "#{error_code}. This account has been superseded by #{pa_data_entry.decoded_value}."
                   end
                 end
               end

lib/rex/proto/kerberos/model/krb_error.rb

@@ -72,30 +72,24 @@ def encode
             raise ::NotImplementedError, 'KrbError encoding not supported'
           end
 
-          # Decodes the e_data field as an Array<PreAuthDataEntry>
+          # Decodes the e_data field as an Array<PreAuthDataEntry>.
           #
           # @return [Array<Rex::Proto::Kerberos::Model::PreAuthDataEntry>]
           def e_data_as_pa_data
+            return [] unless self.e_data
+
             pre_auth = []
             decoded = OpenSSL::ASN1.decode(self.e_data)
-            decoded.each do |pre_auth_data|
-              pre_auth << Rex::Proto::Kerberos::Model::PreAuthDataEntry.decode(pre_auth_data)
-            end
-
-            pre_auth
-          end
 
-          # Decodes the e_data field as a PreAuthData
-          #
-          # @return [Rex::Proto::Kerberos::Model::PreAuthData]
-          def e_data_as_pa_data_entry
-            if self.e_data
-              decoded = OpenSSL::ASN1.decode(self.e_data)
-              Rex::Proto::Kerberos::Model::PreAuthDataEntry.decode(decoded)
+            if decoded.first.tag_class == :UNIVERSAL && decoded.first.tag == 16
+              decoded.each do |pre_auth_data|
+                pre_auth << Rex::Proto::Kerberos::Model::PreAuthDataEntry.decode(pre_auth_data)
+              end
             else
-              # This is implementation-defined, so may be different in some cases
-              nil
+              pre_auth << Rex::Proto::Kerberos::Model::PreAuthDataEntry.decode(decoded)
             end
+
+            pre_auth
           end
 
           private