[PATCH] ksmbd: require DELETE access for delete-on-close opens

[PATCH] ksmbd: require DELETE access for delete-on-close opens

[Ian Bridges]

smb2_open() accepts FILE_DELETE_ON_CLOSE on an existing file without
DELETE in DesiredAccess. When the request asks only for
FILE_READ_ATTRIBUTES or READ_CONTROL, it also skips inode_permission()
and the parent directory MAY_WRITE | MAY_EXEC check that delete-on-close
is meant to get. The file is then marked for deletion by a caller who
could not delete it.

The unlink happens later, in __ksmbd_inode_close(), when the last handle
on the inode is closed, and it runs with the credentials of that handle.
If the caller closes last, the unlink fails. If another user who can
write the parent directory, such as the file's owner, still has the file
open, the file is deleted when that user closes it. Until then new opens
of the file fail with STATUS_DELETE_PENDING. So a user with only search
permission on a directory can delete a file in it, or block opens of it,
while the owner has it open.

A MAXIMUM_ALLOWED open that also requests DELETE has the same problem.
The requested access is replaced with the maximal access, which does not
include DELETE when the caller cannot write the parent directory, and the
later permission checks are skipped as already done.

MS-FSA 2.1.5.1 requires an open with FILE_DELETE_ON_CLOSE and without
DELETE in DesiredAccess to fail with STATUS_INVALID_PARAMETER, and Samba
does the same. Add that check to the CreateOptions validation, using
the generic-mapped DesiredAccess so that GENERIC_ALL still qualifies.
This also refuses MAXIMUM_ALLOWED with FILE_DELETE_ON_CLOSE but no
explicit DELETE, as MS-FSA and Samba do. When MAXIMUM_ALLOWED is
requested with DELETE but the maximal access does not include DELETE,
fail with STATUS_ACCESS_DENIED. With DELETE requested, delete-on-close
opens go through the existing parent directory check.

Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Signed-off-by: Ian Bridges <icb@fastmail.org>
---

Notes:
    Reachable by any authenticated user on a writable share, with no
    acl_xattr or other special configuration. The caller needs search
    permission on the directory, and the file must be open by a user who can
    write the directory when the caller closes. Impact is unauthorized
    deletion of files that are in use, and denial of new opens while the
    delete is pending. It does not expose file contents and is not a
    privilege escalation.
    
    Reproduced on ksmbd-for-next 206fc30f71b0. tester2 owns a 0755 directory
    and a 0600 file in it, and holds the file open with ShareAccess
    READ | WRITE. tester is denied open(DELETE) and open(GENERIC_READ), but
    opens the file with FILE_READ_ATTRIBUTES, or with MAXIMUM_ALLOWED |
    DELETE, plus FILE_DELETE_ON_CLOSE, and closes it. tester2's next open
    fails with STATUS_DELETE_PENDING and the file is unlinked when tester2
    closes. With this change those opens fail with STATUS_INVALID_PARAMETER
    and STATUS_ACCESS_DENIED and the file survives. Delete-on-close with
    DELETE, GENERIC_ALL or MAXIMUM_ALLOWED | DELETE on existing files, new
    files and directories still works, as do smbclient rm, rmdir, rename and
    deltree. The Linux client always requests DELETE with delete-on-close.
    
    A reproducer is available on request.
    
    This fix was developed with LLM assistance.

 fs/smb/server/smb2pdu.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index be226664b8af..c626cb3bece1 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -4606,6 +4606,14 @@ int smb2_open(struct ksmbd_work *work)
 				goto err_out2;
 			}
 		}
+
+		/* MS-FSA 2.1.5.1: delete-on-close requires DELETE access */
+		if (req->CreateOptions & FILE_DELETE_ON_CLOSE_LE &&
+		    !(smb_map_generic_desired_access(req->DesiredAccess) &
+		      FILE_DELETE_LE)) {
+			rc = -EINVAL;
+			goto err_out2;
+		}
 	}
 
 	if (le32_to_cpu(req->CreateDisposition) >
@@ -4875,6 +4883,13 @@ int smb2_open(struct ksmbd_work *work)
 		maximal_access = daccess;
 	}
 
+	/* MAXIMUM_ALLOWED may not have granted the requested DELETE */
+	if (req->CreateOptions & FILE_DELETE_ON_CLOSE_LE &&
+	    !(daccess & FILE_DELETE_LE)) {
+		rc = -EACCES;
+		goto err_out;
+	}
+
 	open_flags = smb2_create_open_flags(file_present, daccess,
 					    req->CreateDisposition,
 					    &may_flags,

base-commit: 206fc30f71b005156085fd8523ccc84a2c67791b
-- 
2.47.3