Pick the mechanism from the server's preference, not the client's

lib/ruby_smb/gss/provider/multi.rb

@@ -81,15 +81,33 @@ def process(request_buffer=nil)
             end
 
             if negotiation_init?(gss_api)
-              # a NegTokenInit names the mechanism the client chose, so this is where routing is decided
-              mech_type = Gss.asn1dig(gss_api, 1, 0, 0, 0, 0)
-              authenticator = authenticator_for(mech_type)
-              if authenticator.nil?
-                logger.warn("Client selected an unsupported GSS mechanism (#{mech_type&.oid || 'unknown'})")
+              # a NegTokenInit carries the client's full mechTypeList. Server preference wins the
+              # routing: the server picks its most-preferred advertised mechanism that the client
+              # also offers, independent of the client's own ordering. This prevents a client (or
+              # an on-path attacker rewriting the mechTypeList before signing is in effect) from
+              # forcing the server to a weaker sub-provider by listing it first.
+              client_oids = client_mech_oids(gss_api)
+              if client_oids.empty?
+                logger.warn('NegTokenInit carried no mechTypeList')
                 return
               end
 
-              @selected = authenticator
+              chosen_mech = @provider.mech_types.find { |m| client_oids.include?(m.value) }
+              if chosen_mech.nil?
+                logger.warn("Client offered no mechanism the server supports (client_oids=#{client_oids})")
+                return
+              end
+
+              @selected = authenticator_for(chosen_mech)
+
+              # if the client listed a different mechanism first, its optimistic mechToken is for
+              # the wrong mechanism. RFC 4178 section 4.2.2 says to reply with a NegTokenResp
+              # carrying accept-incomplete and supportedMech so the client resends a token for the
+              # mechanism the server selected
+              if client_oids.first != chosen_mech.value
+                logger.info("SPNEGO: client listed #{client_oids.first} first; server prefers #{chosen_mech.value}, requesting a token for it")
+                return Result.new(build_accept_incomplete(chosen_mech), WindowsError::NTStatus::STATUS_MORE_PROCESSING_REQUIRED)
+              end
             elsif @selected.nil?
               # a NegTokenResp carries no mechanism OID, so it can only be interpreted as a continuation of a
               # negotiation that has already selected one
@@ -122,6 +140,29 @@ def authenticator_for(mech_type)
 
             @authenticators[provider] ||= provider.new_authenticator(@server_client)
           end
+
+          # The OIDs the client listed in the NegTokenInit mechTypeList, in the client's own order.
+          #
+          # The ASN.1 path mirrors the one NTLM uses to reach a single mechTypeList entry
+          # (gss_api, 1, 0, 0, 0, 0): one level less reaches the Sequence that holds every entry.
+          def client_mech_oids(gss_api)
+            seq = Gss.asn1dig(gss_api, 1, 0, 0, 0)
+            return [] unless seq.respond_to?(:value) && seq.value.is_a?(Array)
+
+            seq.value.map { |item| item.respond_to?(:value) ? item.value : nil }.compact
+          end
+
+          # A NegTokenResp carrying negResult = accept-incomplete and supportedMech, per RFC 4178
+          # section 4.2.2, used to request a mechToken for the mechanism the server selected when
+          # the client's optimistic mechToken was for a different mechanism.
+          def build_accept_incomplete(supported_mech)
+            OpenSSL::ASN1::ASN1Data.new([
+              OpenSSL::ASN1::Sequence.new([
+                OpenSSL::ASN1::ASN1Data.new([OpenSSL::ASN1::Enumerated.new(OpenSSL::BN.new(1))], 0, :CONTEXT_SPECIFIC),
+                OpenSSL::ASN1::ASN1Data.new([supported_mech], 1, :CONTEXT_SPECIFIC)
+              ])
+            ], 1, :CONTEXT_SPECIFIC).to_der
+          end
         end
       end
     end

spec/lib/ruby_smb/gss/provider/multi_spec.rb

@@ -109,6 +109,55 @@
       it 'returns nil for a malformed request' do
         expect(authenticator.process('not asn1 at all')).to be_nil
       end
+
+      # Server preference is [Kerberos, NTLM] (other_provider ordered first in the subject).
+      # Each row names a client-supplied mechTypeList and the outcome the server should produce.
+      context 'when a client offers more than one mechanism' do
+        it '[Kerberos, NTLM] routes to Kerberos directly (first-mech matches server preference)' do
+          expect(other_authenticator).to receive(:process)
+          authenticator.process(gss_init_list([RubySMB::Gss::OID_KERBEROS_5, RubySMB::Gss::OID_NTLMSSP]))
+        end
+
+        it '[NTLM, Kerberos] rejects the client ordering and replies accept-incomplete for Kerberos' do
+          expect(other_authenticator).not_to receive(:process)
+          result = authenticator.process(gss_init_list([RubySMB::Gss::OID_NTLMSSP, RubySMB::Gss::OID_KERBEROS_5]))
+          expect(result).to be_a(RubySMB::Gss::Provider::Result)
+          expect(result.nt_status).to eq(WindowsError::NTStatus::STATUS_MORE_PROCESSING_REQUIRED)
+          supported_mech = RubySMB::Gss.asn1dig(OpenSSL::ASN1.decode(result.buffer), 0, 1, 0)
+          expect(supported_mech.value).to eq(RubySMB::Gss::OID_KERBEROS_5.value)
+        end
+
+        it '[NTLM] only still routes to NTLM when it is the only overlap' do
+          type1 = Net::NTLM::Message::Type1.new.tap { |msg| msg.domain = domain }
+          result = authenticator.process(RubySMB::Gss.gss_type1(type1.serialize))
+          expect(result.nt_status).to eq(WindowsError::NTStatus::STATUS_MORE_PROCESSING_REQUIRED)
+        end
+
+        it '[Kerberos] only routes to Kerberos' do
+          expect(other_authenticator).to receive(:process)
+          authenticator.process(gss_init_list([RubySMB::Gss::OID_KERBEROS_5]))
+        end
+
+        it 'rejects a client with no overlapping mechanism' do
+          expect(authenticator.process(gss_init_list([RubySMB::Gss::OID_NEGOEX]))).to be_nil
+        end
+
+        it 'rejects a NegTokenInit with an empty mechTypeList' do
+          expect(authenticator.process(gss_init_list([]))).to be_nil
+        end
+      end
+
+      context 'after replying accept-incomplete to a reordered client' do
+        it 'routes the client\'s next NegTokenResp (carrying the server-chosen mechanism token) to that provider' do
+          # leg 1: client lists [NTLM, Kerberos] - server picks Kerberos and asks for its token.
+          first = authenticator.process(gss_init_list([RubySMB::Gss::OID_NTLMSSP, RubySMB::Gss::OID_KERBEROS_5]))
+          expect(first.nt_status).to eq(WindowsError::NTStatus::STATUS_MORE_PROCESSING_REQUIRED)
+
+          # leg 2: client resends with a Kerberos token wrapped as a NegTokenResp.
+          expect(other_authenticator).to receive(:process)
+          authenticator.process(RubySMB::Gss.gss_type3('kerberos-ap-req-bytes'))
+        end
+      end
     end
 
     describe 'a complete NTLM exchange' do
@@ -143,14 +192,20 @@
 
   # Build a NegTokenInit that selects the specified mechanism, with an empty mechToken.
   def gss_init(mech_type)
+    gss_init_list([mech_type])
+  end
+
+  # Build a NegTokenInit whose mechTypeList is the given list of OIDs, with an empty mechToken.
+  # Used by the matrix tests to exercise multi-mechanism client offers.
+  def gss_init_list(mech_types)
     OpenSSL::ASN1::ASN1Data.new(
       [
         RubySMB::Gss::OID_SPNEGO,
         OpenSSL::ASN1::ASN1Data.new(
           [
             OpenSSL::ASN1::Sequence.new(
               [
-                OpenSSL::ASN1::ASN1Data.new([OpenSSL::ASN1::Sequence.new([mech_type])], 0, :CONTEXT_SPECIFIC),
+                OpenSSL::ASN1::ASN1Data.new([OpenSSL::ASN1::Sequence.new(mech_types)], 0, :CONTEXT_SPECIFIC),
                 OpenSSL::ASN1::ASN1Data.new([OpenSSL::ASN1::OctetString.new('')], 2, :CONTEXT_SPECIFIC)
               ]
             )