Difference between revisions of "Ksmbd-review"

From SambaWiki
m
Line 19: Line 19:
 
|-
 
|-
 
|StructureSize field check for 19 SMB3 commands
 
|StructureSize field check for 19 SMB3 commands
|
 
 
| already implemented
 
| already implemented
 
| smb2_req_struct_sizes and smb2_get_data_area_len in smb2misc.c
 
| smb2_req_struct_sizes and smb2_get_data_area_len in smb2misc.c
 
|
 
|-
 
|-
 
|check for overflow of SET_INFO command
 
|check for overflow of SET_INFO command
|
 
 
| patches submitted, partially reviewed
 
| patches submitted, partially reviewed
 
| "ksmbd: add request buffer validation in smb2_set_info"
 
| "ksmbd: add request buffer validation in smb2_set_info"
 
|
 
|-
 
|-
 
|check for overflow of FSCTL command
 
|check for overflow of FSCTL command
|
 
 
| patch submitted, partially reviewed
 
| patch submitted, partially reviewed
 
| "ksmbd: add validation in smb2_ioctl", smb2pdu.c
 
| "ksmbd: add validation in smb2_ioctl", smb2pdu.c
 
|
 
|-
 
|-
 
|Generic check for packet overflow
 
|Generic check for packet overflow
Line 39: Line 39:
 
|-
 
|-
 
|Packet Signing Check (for packet corruption and man-in-the-middle attacks)
 
|Packet Signing Check (for packet corruption and man-in-the-middle attacks)
|
 
 
| already implemented
 
| already implemented
 
| check_sign_req function in smb2pdu.c
 
| check_sign_req function in smb2pdu.c
 
|
 
|-
 
|-
 
|Check for open context overflow
 
|Check for open context overflow
|
 
 
| patch submitted, partially reviewed
 
| patch submitted, partially reviewed
 
| See "ksmbd: add buffer validation for SMB2_CREATE_CONTEXT", oplock.c smb2pdu.c
 
| See "ksmbd: add buffer validation for SMB2_CREATE_CONTEXT", oplock.c smb2pdu.c
 
|
 
|-
 
|-
 
|Check for overflow of ACL
 
|Check for overflow of ACL
|
 
 
| patch submitted, partially reviewed
 
| patch submitted, partially reviewed
 
| See "ksmbd: add buffer validation for SMB2_CREATE_CONTEXT", smbacl.c (parse_dacl and parse_sec_desc functions)
 
| See "ksmbd: add buffer validation for SMB2_CREATE_CONTEXT", smbacl.c (parse_dacl and parse_sec_desc functions)
 
|
 
|-
 
|-
 
|Check for negotiate context overflow
 
|Check for negotiate context overflow
Line 69: Line 69:
 
|-
 
|-
 
|checks for .. escaping the share
 
|checks for .. escaping the share
|
 
 
| patch submitted, reviewed
 
| patch submitted, reviewed
 
| "ksmbd: prevent out of share access"
 
| "ksmbd: prevent out of share access"
 
|
 
|-
 
|-
 
|checks for symlinks in path components
 
|checks for symlinks in path components
|
 
 
| patch submitted, partially reviewed
 
| patch submitted, partially reviewed
 
| "ksmbd: use LOOKUP_NO_SYMLINKS flags for default lookup" (see ksmbd_conv_path_to_unix function in misc.c and its use in smb2pdu.c)
 
| "ksmbd: use LOOKUP_NO_SYMLINKS flags for default lookup" (see ksmbd_conv_path_to_unix function in misc.c and its use in smb2pdu.c)
 
|
 
|-
 
|-
 
|checks for resolved path never being / or \ (outside the share)
 
|checks for resolved path never being / or \ (outside the share)

Revision as of 18:24, 20 September 2021

There are various checks that are especially important for SMB3 servers. They fall into multiple categories:

  • packet processing checks to make sure buffers do not overflow, allowing access to data outside the SMB request
  • path processing checks to make sure access to files outside the share is not permitted, these include checks for ".." or "../.." to make sure they do not go outside the share, and checks for symlinks in paths and checks to ensure paths do not allow a processed path e.g. with "\" or "/" to go outside the share
  • denial of service and resource checks (e.g. ensure that no single client can exhaust the server by opening millions of files, or use so many credits that they submit thousands of simultaneous requests starving other clients)
Packet processing checks in ksmbd
Type of check Status (submitted, reviewed) Function(s) including the check (and patch name if not included yet) Additional work if any
SMB3 header validation
StructureSize field check for 19 SMB3 commands already implemented smb2_req_struct_sizes and smb2_get_data_area_len in smb2misc.c
check for overflow of SET_INFO command patches submitted, partially reviewed "ksmbd: add request buffer validation in smb2_set_info"
check for overflow of FSCTL command patch submitted, partially reviewed "ksmbd: add validation in smb2_ioctl", smb2pdu.c
Generic check for packet overflow
Packet Signing Check (for packet corruption and man-in-the-middle attacks) already implemented check_sign_req function in smb2pdu.c
Check for open context overflow patch submitted, partially reviewed See "ksmbd: add buffer validation for SMB2_CREATE_CONTEXT", oplock.c smb2pdu.c
Check for overflow of ACL patch submitted, partially reviewed See "ksmbd: add buffer validation for SMB2_CREATE_CONTEXT", smbacl.c (parse_dacl and parse_sec_desc functions)
Check for negotiate context overflow
Path processing checks in ksmbd
Type of check Status (submitted, reviewed) Function(s) including the check (and patch name if not included yet) Additional work if any
checks for .. escaping the share patch submitted, reviewed "ksmbd: prevent out of share access"
checks for symlinks in path components patch submitted, partially reviewed "ksmbd: use LOOKUP_NO_SYMLINKS flags for default lookup" (see ksmbd_conv_path_to_unix function in misc.c and its use in smb2pdu.c)
checks for resolved path never being / or \ (outside the share)


Denial of Service checks
Type of check Status (submitted, reviewed) Function(s) including the check (and patch name if not included yet) Additional work if any
checks for opening too many files
checks for allowing too many requests (crediting)