Merge pull request #1284 from TheManticoreProject/bugfix-trans2-setfsinfo-status

Refuse TRANS2_SET_FS_INFORMATION with the mandated status (Fixes #1215)

Author: p0dalirius

network/smb/smb_v10/server/errors.go

@@ -144,6 +144,13 @@ var dosErrors = map[nt_status.NT_STATUS]SMBStatus{
 	nt_status.NT_STATUS_NONEXISTENT_SECTOR:    {ERRHRD, 0x001B}, // ERRbadsector
 	nt_status.NT_STATUS_WRONG_VOLUME:          {ERRHRD, 0x0022}, // ERRwrongdisk
 	nt_status.NT_STATUS_DISK_FULL:             {ERRHRD, 0x0027}, // ERRdiskfull
+
+	// NT_STATUS_SMB_NO_SUPPORT is tabulated rather than decomposed. It is a
+	// [MS-CIFS] composite like the others, but its ErrorCode is 0xFFFF, so its
+	// top byte is 0xFF and the decomposition below — which requires a zero top
+	// byte to tell a composite from a real [MS-ERREF] NTSTATUS — cannot claim
+	// it. Without this entry it would fall through to ERRSRV/ERRsrverror.
+	nt_status.NT_STATUS_SMB_NO_SUPPORT: {ERRSRV, 0xFFFF}, // ERRnosupport
 }
 
 // unmappedError is the pair sent for an NTSTATUS with no tabulated SMBSTATUS

network/smb/smb_v10/server/errors_test.go

@@ -121,6 +121,32 @@ func TestDOSErrorDecomposesCIFSStatus(t *testing.T) {
 	}
 }
 
+// TestDOSErrorTabulatesSMBNoSupport covers the one [MS-CIFS] composite that
+// cannot be decomposed. NT_STATUS_SMB_NO_SUPPORT is ERRSRV/ERRnosupport, but its
+// ErrorCode is 0xFFFF, so the value's top byte is 0xFF and the decomposition in
+// DOSError — which requires a zero top byte to tell a composite apart from a
+// real [MS-ERREF] NTSTATUS — cannot claim it. It is tabulated instead, and
+// without that entry a client which did not negotiate NT status codes would be
+// told ERRSRV/ERRsrverror for a subcommand the specification says to refuse with
+// ERRnosupport.
+func TestDOSErrorTabulatesSMBNoSupport(t *testing.T) {
+	got, ok := DOSError(nt_status.NT_STATUS_SMB_NO_SUPPORT)
+	if !ok {
+		t.Fatal("DOSError(NT_STATUS_SMB_NO_SUPPORT) reported no mapping")
+	}
+	if want := (SMBStatus{ERRSRV, 0xFFFF}); got != want {
+		t.Fatalf("DOSError(NT_STATUS_SMB_NO_SUPPORT) = %s/0x%04X, want %s/0x%04X",
+			got.Class, got.Code, want.Class, want.Code)
+	}
+	if got == unmappedError {
+		t.Fatal("it fell through to the generic ERRSRV/ERRsrverror")
+	}
+	// It is still wire-identical to the NTSTATUS, like every other composite.
+	if encoded := got.Encode(); encoded != uint32(nt_status.NT_STATUS_SMB_NO_SUPPORT) {
+		t.Fatalf("re-encoding gives 0x%08X, want 0x%08X", encoded, uint32(nt_status.NT_STATUS_SMB_NO_SUPPORT))
+	}
+}
+
 // TestDOSErrorUnmapped asserts an NTSTATUS with no legacy equivalent reports
 // that fact and still yields something valid to send.
 func TestDOSErrorUnmapped(t *testing.T) {

network/smb/smb_v10/server/handler_remaining_subcommands.go

@@ -142,17 +142,17 @@ func handleTrans2CreateDirectory(
 // than left to fall through to STATUS_NOT_IMPLEMENTED because the specification
 // names a different status for it than for its neighbours.
 //
-// STATUS_SMB_NO_SUPPORT has no constant in windows/errors/nt_status, so the closest
-// available is used: NT_STATUS_NOT_SUPPORTED, which carries the same meaning and
-// maps to a not-supported legacy code. Nothing sends this subcommand — it is
-// reserved — so the difference is a conformance detail rather than a live one.
+// The status is NT_STATUS_SMB_NO_SUPPORT, the [MS-CIFS] extension value that is
+// wire-identical to the ERRSRV/ERRnosupport pair the specification names. Nothing
+// sends this subcommand — it is reserved — so the refusal is a conformance
+// detail rather than a live path.
 func handleTrans2SetFsInformation(
 	conn *Connection,
 	_ *message.Message,
 	_ *transactionReassembly,
 ) ([]byte, []byte, nt_status.NT_STATUS) {
 	logger.Debugf("SMB1 server: %s sent TRANS2_SET_FS_INFORMATION, which is reserved and refused", conn.Remote)
-	return nil, nil, nt_status.NT_STATUS_NOT_SUPPORTED
+	return nil, nil, nt_status.NT_STATUS_SMB_NO_SUPPORT
 }
 
 // handleNtTransactCreate answers NT_TRANSACT_CREATE, the NT_TRANSACT open.

network/smb/smb_v10/server/remaining_subcommands_test.go

@@ -210,13 +210,12 @@ func TestReservedSubcommandsUseTheirMandatedStatus(t *testing.T) {
 	_, client := fileServer(t, fs, false)
 
 	t.Run("TRANS2_SET_FS_INFORMATION", func(t *testing.T) {
-		// STATUS_SMB_NO_SUPPORT has no constant in windows/errors/nt_status, so the
-		// closest available is used; what matters here is that it is not the
-		// generic STATUS_NOT_IMPLEMENTED.
+		// [MS-CIFS] 2.2.6.5 mandates STATUS_SMB_NO_SUPPORT for this subcommand,
+		// not the generic refusal its neighbours receive.
 		_, _, status := sendTrans2(t, client, subcommands.TRANS2_SET_FS_INFORMATION, make([]byte, 4), nil)
-		if status != uint32(nt_status.NT_STATUS_NOT_SUPPORTED) {
-			t.Errorf("it reported 0x%08X, want STATUS_NOT_SUPPORTED (0x%08X)",
-				status, uint32(nt_status.NT_STATUS_NOT_SUPPORTED))
+		if status != uint32(nt_status.NT_STATUS_SMB_NO_SUPPORT) {
+			t.Errorf("it reported 0x%08X, want STATUS_SMB_NO_SUPPORT (0x%08X)",
+				status, uint32(nt_status.NT_STATUS_SMB_NO_SUPPORT))
 		}
 		if status == uint32(nt_status.NT_STATUS_NOT_IMPLEMENTED) {
 			t.Error("it fell through to the table's generic refusal")