Add #set_unix_link to the SMB1 tree#297
Conversation
There was a problem hiding this comment.
Pull request overview
Adds SMB1 support for creating UNIX symlinks on Samba shares by implementing the CIFS UNIX Extensions handshake (QUERY/SET_CIFS_UNIX_INFO) and issuing a Trans2 SET_PATH_INFORMATION request at the SMB_SET_FILE_UNIX_LINK info level.
Changes:
- Add
RubySMB::SMB1::Tree#set_unix_linkplus CIFS UNIX Extensions handshake helpers (enable_cifs_unix_extensions, QUERY/SET helpers). - Introduce new Trans2 request/response packet classes and new info-level constants for CIFS UNIX Extensions.
- Expand/update specs to cover the new Tree behavior and packet shapes/encodings.
Impact Analysis:
- Blast radius: medium (new public Tree methods and new packet classes/constants; consumers using
RubySMB::SMB1::Treeor SMB1 Trans2 packet namespace). - Data and contract effects: adds new public API surface (
set_unix_link,enable_cifs_unix_extensions) and new protocol encodings; no storage/schema changes. - Rollback and test focus: rollback is straightforward (remove new methods/packets); focus testing on (1) handshake success/failure paths, (2) correct non-Unicode encoding on Samba, and (3) decoding/encoding of the new packet classes.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/lib/ruby_smb/smb1/tree_spec.rb | Adds behavioral specs for #set_unix_link and #enable_cifs_unix_extensions. |
| spec/lib/ruby_smb/smb1/packet/trans2/set_path_information_response_spec.rb | New spec coverage for SetPathInformationResponse structure. |
| spec/lib/ruby_smb/smb1/packet/trans2/set_path_information_request_spec.rb | New spec coverage for SetPathInformationRequest fields and encoding. |
| spec/lib/ruby_smb/smb1/packet/trans2/set_fs_information_response_spec.rb | New spec coverage for SetFsInformationResponse. |
| spec/lib/ruby_smb/smb1/packet/trans2/set_fs_information_request_spec.rb | New spec coverage for SetFsInformationRequest structure. |
| spec/lib/ruby_smb/smb1/packet/trans2/query_fs_information_request_spec.rb | Updates expectations to reflect a trans2_data placeholder. |
| lib/ruby_smb/smb1/tree.rb | Implements symlink creation and CIFS UNIX Extensions handshake logic. |
| lib/ruby_smb/smb1/packet/trans2/set_path_information_response.rb | Adds Trans2 SET_PATH_INFORMATION response packet. |
| lib/ruby_smb/smb1/packet/trans2/set_path_information_request.rb | Adds Trans2 SET_PATH_INFORMATION request packet. |
| lib/ruby_smb/smb1/packet/trans2/set_information_level.rb | Defines CIFS UNIX Extensions SET info levels (e.g., UNIX_LINK). |
| lib/ruby_smb/smb1/packet/trans2/set_fs_information_response.rb | Adds Trans2 SET_FS_INFORMATION response packet. |
| lib/ruby_smb/smb1/packet/trans2/set_fs_information_request.rb | Adds Trans2 SET_FS_INFORMATION request packet. |
| lib/ruby_smb/smb1/packet/trans2/set_fs_information_level.rb | Defines SET_FS info level for CIFS UNIX Extensions negotiation. |
| lib/ruby_smb/smb1/packet/trans2/query_fs_information_request.rb | Adds a zero-length trans2_data placeholder field to support request encoding helpers. |
| lib/ruby_smb/smb1/packet/trans2/query_fs_information_level/query_fs_cifs_unix_info.rb | Adds the CIFS UNIX info structure (major/minor/capabilities). |
| lib/ruby_smb/smb1/packet/trans2/query_fs_information_level.rb | Registers SMB_QUERY_CIFS_UNIX_INFO and requires the new structure. |
| lib/ruby_smb/smb1/packet/trans2.rb | Requires the new Trans2 packet/types modules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| request.parameter_block.max_data_count = 560 | ||
|
|
||
| raw_response = client.send_recv(request) | ||
| response = RubySMB::SMB1::Packet::Trans2::QueryFsInformationResponse.read(raw_response) |
There was a problem hiding this comment.
Important: Problem: query_cifs_unix_info parses the response and checks only status_code without verifying response.valid?. Impact: malformed/non-Trans2 replies can be misinterpreted or raise later while still reporting an NTSTATUS. Fix: mirror other Tree Trans2 call sites by validating the packet (valid?) and raising RubySMB::Error::InvalidPacket before inspecting status_code.
| response = RubySMB::SMB1::Packet::Trans2::QueryFsInformationResponse.read(raw_response) | |
| response = RubySMB::SMB1::Packet::Trans2::QueryFsInformationResponse.read(raw_response) | |
| unless response.valid? | |
| raise RubySMB::Error::InvalidPacket | |
| end |
| request.parameter_block.max_data_count = 0 | ||
|
|
||
| raw_response = client.send_recv(request) | ||
| response = RubySMB::SMB1::Packet::Trans2::SetFsInformationResponse.read(raw_response) |
There was a problem hiding this comment.
Important: Problem: set_cifs_unix_info checks status_code but does not verify response.valid? after SetFsInformationResponse.read. Impact: an invalid/partial packet can surface as misleading status handling or downstream parsing errors. Fix: add a valid? check and raise RubySMB::Error::InvalidPacket (consistent with other Tree request/response handlers) before checking status_code.
| response = RubySMB::SMB1::Packet::Trans2::SetFsInformationResponse.read(raw_response) | |
| response = RubySMB::SMB1::Packet::Trans2::SetFsInformationResponse.read(raw_response) | |
| raise RubySMB::Error::InvalidPacket, 'SetFsInformationResponse' unless response.valid? |
| # target info level. SMB_SET_FILE_UNIX_LINK (0x0201) for example | ||
| # carries the symlink target as a null-terminated string. | ||
| class SetPathInformationRequestTrans2Data < BinData::Record | ||
| string :buffer, read_length: -> { parent.buffer_read_length } |
There was a problem hiding this comment.
Critical: Problem: read_length: -> { parent.buffer_read_length } calls buffer_read_length as a private method on an explicit receiver, which Ruby forbids. Impact: decoding a SetPathInformationRequest from bytes (or any path that triggers this read_length) can raise NoMethodError instead of parsing. Fix: use the established Trans2 pattern (read_length: :buffer_read_length or equivalent that evaluates via send) so it can access the DataBlock helper.
| string :buffer, read_length: -> { parent.buffer_read_length } | |
| string :buffer, read_length: :buffer_read_length |
| # and implemented in | ||
| # [source3/smbd/smb1_trans2.c:1706-1915 (`call_trans2setfsinfo`)](https://github.com/samba-team/samba/blob/33f516c06756e12a9d11f50e2bf309171cdf5c88/source3/smbd/smb1_trans2.c#L1706-L1915). | ||
| class SetFsInformationRequestTrans2Data < BinData::Record | ||
| string :buffer, read_length: -> { parent.buffer_read_length } |
There was a problem hiding this comment.
Critical: Problem: read_length: -> { parent.buffer_read_length } invokes buffer_read_length as a private method on an explicit receiver. Impact: parsing SetFsInformationRequest from bytes can fail with NoMethodError, breaking any code that attempts to decode captured traffic or secondary request flows. Fix: switch to read_length: :buffer_read_length (or an approach that uses send) to match the existing Trans2 buffer parsing behavior.
| string :buffer, read_length: -> { parent.buffer_read_length } | |
| string :buffer, read_length: :buffer_read_length |
…mlink helper
SMB1::Tree#set_unix_link builds a Trans2 SET_PATH_INFORMATION request
with the CIFS UNIX Extensions SMB_SET_FILE_UNIX_LINK (0x0201) info
level and sends it against the currently-mounted share.
Samba gates UNIX info levels behind a per-session CIFS UNIX Extensions
handshake: SMB_QUERY_CIFS_UNIX_INFO to learn the server's supported
extensions, then SMB_SET_CIFS_UNIX_INFO to echo the capability bits
back. Without this handshake Samba returns STATUS_INVALID_LEVEL even
when 'unix extensions = yes' is configured. set_unix_link now performs
that handshake automatically via enable_cifs_unix_extensions (also
exposed as a public helper).
Trans2 surface area added:
- SET_PATH_INFORMATION request/response + SetInformationLevel
(SMB_SET_FILE_UNIX_LINK = 0x0201)
- SET_FS_INFORMATION request/response + SetFsInformationLevel
(SMB_SET_CIFS_UNIX_INFO = 0x0200)
- QueryFsCifsUnixInfo record (major/minor/capabilities)
- SMB_QUERY_CIFS_UNIX_INFO (0x0200) added to QueryFsInformationLevel
Also fixes a latent issue in QueryFsInformationRequestDataBlock: the
class omitted a :trans2_data field that the parent DataBlock padding
helpers need, which crashed the class as soon as something tried to
serialize or introspect it. A zero-length placeholder restores
symmetry with the other Trans2 request data blocks.
Path strings for SMB_SET_FILE_UNIX_LINK are transmitted as raw byte
strings per the CIFS UNIX Extensions spec; set_unix_link clears
flags2.unicode for the request regardless of session state.
Attach MS-CIFS (and where applicable, Samba source) references to every
BinData-derived class and information-level module touched by the
symlink work so reviewers can cross-check the packet layouts against
the canonical specifications.
References added:
- TRANS2_SET_PATH_INFORMATION (0x0006) subcommand [2.2.6.7] and its
Request [2.2.6.7.1] / Response [2.2.6.7.2] sub-sections. Replaces
the 2.2.6.8.x links previously in set_path_information_*.rb — the
subcommand is section 2.2.6.7 in the current MS-CIFS revision and
the old UUIDs now return 404.
- TRANS2_QUERY_FS_INFORMATION (0x0003) subcommand [2.2.6.4] and its
Request [2.2.6.4.1] sub-section.
- TRANS2_SET_FS_INFORMATION (0x0004) subcommand [2.2.6.5] — marked
"reserved but not implemented" in MS-CIFS; the CIFS UNIX
Extensions repurpose this subcommand. Files pair the MS-CIFS link
with a pointer to Samba's source3/smbd/smb1_trans2.c so the
actual wire format is discoverable.
- SET Information Level Codes [2.2.2.3.4] for SetInformationLevel.
- Information Level Codes parent [2.2.2.3] and Samba source for
SetFsInformationLevel (UNIX levels live in the 0x0200–0x02FF
third-party range reserved by that section).
- QueryFsCifsUnixInfo gets a pointer to TRANS2_QUERY_FS_INFORMATION
plus the Samba source implementing the record layout.
Every URL added in this commit was confirmed to return HTTP 200 OK at
commit time.
Replaces every `blob/master/...smb1_trans2.c` reference added in the
previous commit with a permalink to commit 33f516c0 plus a specific
line range so the reader lands on exactly the handler or struct being
cited, and the link cannot silently drift when upstream master moves.
Mapping:
- SetInformationLevel -> `smb_set_file_unix_link` handler
(smb1_trans2.c#L3643-L3712)
- SetFsInformationLevel and every Set_Fs_Information_* class ->
`call_trans2setfsinfo` handler
(smb1_trans2.c#L1706-L1915)
- QueryFsCifsUnixInfo -> `unix_info` struct in globals.h plus
`call_trans2qfsinfo` handler
(globals.h#L419-L424 and smb1_trans2.c#L1633-L1703); also fixes
a mismatched label/URL pair left by an earlier edit where the
label claimed smb1_trans2.c while the href pointed to globals.h.
Every URL in this commit was confirmed HTTP 200 at commit time.
349a11c to
81aff03
Compare
jheysel-r7
left a comment
There was a problem hiding this comment.
Thanks for the PR @zeroSteiner. Testing on the framework side was as expected.
TIL about Transaction, Transaction2 and their subcommands. Also that they only exist in SMB1 and not SMB2 or 3.
This appears to be implement what's needed on the framework side 👍
This closes a gap with the Rex SMB client. There will be an accompanying PR to Metasploit that leverages it by updating the
auxiliary/smb/samba_symlink_traversalmodule.