`IRP_MJ_DIRECTORY_CONTROL` #12870

ibeckermayer · 2022-05-24T15:18:45Z

Adds support for IRP_MJ_DIRECTORY_CONTROL, adding support for SharedDirectoryListRequest and SharedDirectoryListResponse in the process.

…and cliprdr::Client's have the vchan::Client as a field.

…esponse

…o trigger it by right-clicking, however it isn't working. One reason is that the vchannel PDU header isn't being added (see rdpdr::encode_message for how that's added to other messages). Noticing that made me notice that there is another cliprdr function for breaking outgoing messages into chunks that should be refactored into vchan to do that work + add the necessary vchan headers. This is a checkpoint commit while I go attend to that.

…ction

…lient parses the DeviceCreateRequest that's immediately sent back

…_CLOSE

…ugh in most cases, no need to neurotically add every bit of the documentation text to the code itself

… to a plain NTSTATUS::STATUS_UNSUCCESSFUL

…ssed

lib/srv/desktop/rdp/rdpclient/src/rdpdr/mod.rs

lib/srv/desktop/rdp/rdpclient/client.go

xacrimon · 2022-06-01T08:27:22Z

lib/srv/desktop/rdp/rdpclient/src/lib.rs

+/// each res.fso_list[i].path MUST be a non-null pointer to a C-style null terminated string.
+///
+/// This function MUST NOT hang on to any of the pointers passed in to it after it returns.
+/// All passed data that needs to persist after this function MUST be copied into Rust-owned memory.


Why do we have an advisory that on relates to the inner function itself on the function docstring?

I think I see your point. The general idea was to add this advisory on all these functions at the FFI, in order to be very explicit about what's expected of them in terms of memory safety. Are you suggesting that this would more appropriately located at SharedDirectoryListResponse::from?

Yep. My thought process is that notes on the FFI functions are advisories to the caller mostly. For impl notes about something I usually lean on placing it as a non docstring inside the body since the caller doesn't care and thus the docs shouldn't either.

See c07f3e5

…-SUPERSEDE

…factor

…tory-control

…sefully pedantic copies to Rust owned memory in relevant FFI functions

zmb3 · 2022-06-28T19:30:32Z

lib/srv/desktop/rdp/rdpclient/client.go

+					// fsoList is memory handled by Go, and will be freed
+					// by the garbage collector automatically sometime after
+					// this code block ends.


I'm curious the reasoning behind this comment. Any time you make a slice in Go the memory is handled by Go. Was there some reason we felt the need to clarify that here?

This was the first time I'd needed to pass something non-trivial across the FFI; this is basically me baby-talking myself through the memory management logic. It can be removed from the production code.

lib/srv/desktop/rdp/rdpclient/src/rdpdr/mod.rs

zmb3 · 2022-06-28T19:35:59Z

lib/srv/desktop/rdp/rdpclient/src/rdpdr/mod.rs

@@ -2164,6 +2496,7 @@ impl ClientDriveQueryDirectoryResponse {
        })
    }

+    // TODO(isaiah): make this logic more sane


I'm fine with leaving a TODO, but make it something more actionable. We're not likely to remember what this even means.

I address it a couple of PR's from this one: https://github.com/gravitational/teleport/pull/12912/files#diff-983433889acb8e2c478d0a04b26bb477bd81271f29515f1ee6c56fa881e2880aL2560

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

…irectory-control

* `IRP_MJ_CREATE` (#12665) * `IRP_MJ_QUERY_INFORMATION` (#12717) * `IRP_MJ_CLOSE` (#12729) * Refactor rdpdr client (#12750) * Adding logic for `FILE_SUPERSEDE` (#12829) * Improve `process_irp_create` (#12830) * adds return statements that got lost in a merge * `IRP_MJ_DIRECTORY_CONTROL` (#12870) * `FileFullDirectoryInformation` (#12908) * Improve `ClientDriveQueryDirectoryResponse.encode()` (#12912) * `IRP_MJ_QUERY_VOLUME_INFORMATION` (#13071) * Fix Shared Directory Request handling when feature is disabled (#13439) * IRP_MJ_READ, IRP_MJ_WRITE, and IRP_MJ_SET_INFORMATION (#13995) * Adds constants for sizing calculations (#14051) Co-authored-by: Łukasz Kozłowski <lukasz.kozlowski@goteleport.com> Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

…ring) (#15770) * Windows Desktop Directory Sharing (#13630) * `IRP_MJ_CREATE` (#12665) * `IRP_MJ_QUERY_INFORMATION` (#12717) * `IRP_MJ_CLOSE` (#12729) * Refactor rdpdr client (#12750) * Adding logic for `FILE_SUPERSEDE` (#12829) * Improve `process_irp_create` (#12830) * adds return statements that got lost in a merge * `IRP_MJ_DIRECTORY_CONTROL` (#12870) * `FileFullDirectoryInformation` (#12908) * Improve `ClientDriveQueryDirectoryResponse.encode()` (#12912) * `IRP_MJ_QUERY_VOLUME_INFORMATION` (#13071) * Fix Shared Directory Request handling when feature is disabled (#13439) * IRP_MJ_READ, IRP_MJ_WRITE, and IRP_MJ_SET_INFORMATION (#13995) * Adds constants for sizing calculations (#14051) Co-authored-by: Łukasz Kozłowski <lukasz.kozlowski@goteleport.com> Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com> * `UnixPath` and `WindowsPath` (#14267) * `SharedDirectoryMoveRequest` and `SharedDirectoryMoveResponse` (#14959) * `SharedDirectoryCreateResponse` update (#15289) * Fix `process_irp_set_information` (#15364) * Sanitize Rust Debug Logs (#15743) * updates rdp-rs ref to include licensing changes * Updates rdp-rs ref and fixes Cargo Co-authored-by: Łukasz Kozłowski <lukasz.kozlowski@goteleport.com> Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

Isaiah Becker-Mayer added 30 commits March 31, 2022 15:35

Creates a general vchan Client that can read chunked messages. rdpdr …

24c1a90

…and cliprdr::Client's have the vchan::Client as a field.

Tidying up

41ac367

Merge branch 'master' into isaiah/reuse-vchan-read

58cac52

small fixes

fa3531f

Merge branch 'master' into isaiah/reuse-vchan-read

ab4d66d

fixes linting errors

aaa8369

Merge branch 'master' into isaiah/reuse-vchan-read

54b7e3d

Merge branch 'master' into isaiah/reuse-vchan-read

071620b

Merge branch 'master' into isaiah/reuse-vchan-read

52a3d5d

Adds drive redirection capability set to the Client Core Capability R…

ceddf84

…esponse

Adds to_unicode util

9036618

refactor of vchan and rdpdr to use it

770f6e7

refactors cliprdr to use generalized vchan header and chunking

17a35e8

nits

c946b52

Merge branch 'master' into isaiah/reuse-vchan-write

787b67e

small fixes from CR

6cd9ac2

Merge branch 'master' into isaiah/reuse-vchan-write

997b7d6

Merge branch 'isaiah/reuse-vchan-write' into isaiah/mock-drive-redire…

eb8ed1a

…ction

A right click sends an ClientDeviceListAnnounce message and the rdp c…

54b53d0

…lient parses the DeviceCreateRequest that's immediately sent back

adds prototype logic for handling IRP_MJ_QUERY_INFORMATION and IRP_MJ…

8f1ead3

…_CLOSE

removes some un needed comments. the link to the documentation is eno…

9a88738

…ugh in most cases, no need to neurotically add every bit of the documentation text to the code itself

Successfully mocks a drive for redirection

fb60ebb

adding important todo

51f83e5

removes unneeded pub decl

b4192a6

refactors rdpdr into its own self-contained folder module

6cf694a

moving scard into rdpdr

fa0d555

allowing dead code

4b9c330

stubs out request_file_info

66eb895

fixes mismatched declaration

687b84c

Isaiah Becker-Mayer added 3 commits May 24, 2022 12:58

changing a mistaken cli.prep_file_cache_fail_drive_query_dir_response…

7427ea9

… to a plain NTSTATUS::STATUS_UNSUCCESSFUL

Adds explanatory comment

5b8a8e6

fixing fsoList cgo/ffi passing so that empty lists can be properly pa…

7b97bcf

…ssed

LKozlowski reviewed May 30, 2022

View reviewed changes

lib/srv/desktop/rdp/rdpclient/src/rdpdr/mod.rs Outdated Show resolved Hide resolved

lib/srv/desktop/rdp/rdpclient/client.go Outdated Show resolved Hide resolved

xacrimon reviewed Jun 1, 2022

View reviewed changes

Isaiah Becker-Mayer added 10 commits June 1, 2022 09:05

small changes from code review

ea608cc

noticed missing to_windows_time

d491636

Adds docstrings to new TDP types

8046597

Merge branch 'windows-desktop-directory-sharing' into isaiah/add-FILE…

9a9eb22

…-SUPERSEDE

Merge branch 'isaiah/add-FILE-SUPERSEDE' into isaiah/irp-mj-create-re…

686633c

…factor

early return

7718818

reintroducing match statement

aa1af3a

updating Cargo to latest rdp-rs head

37c3fbb

Merge branch 'isaiah/irp-mj-create-refactor' into isaiah/irp-mj-direc…

f48195c

…tory-control

Moves internal safety comments inside the function bodies, adds purpo…

c07f3e5

…sefully pedantic copies to Rust owned memory in relevant FFI functions

ibeckermayer requested review from LKozlowski and xacrimon June 28, 2022 16:05

zmb3 approved these changes Jun 28, 2022

View reviewed changes

Isaiah Becker-Mayer and others added 3 commits June 28, 2022 16:24

remove incorrect apostrophes

405e2dc

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

removes handholding comments

7275216

removes comment that was converted to issue

3d60d06

ibeckermayer changed the base branch from isaiah/irp-mj-create-refactor to windows-desktop-directory-sharing June 28, 2022 20:50

Merge branch 'windows-desktop-directory-sharing' into isaiah/irp-mj-d…

1fba706

…irectory-control

ibeckermayer merged commit 05b7574 into windows-desktop-directory-sharing Jun 28, 2022

ibeckermayer pushed a commit that referenced this pull request Jul 6, 2022

IRP_MJ_DIRECTORY_CONTROL (#12870)

1ad2f69

ibeckermayer pushed a commit that referenced this pull request Jul 13, 2022

IRP_MJ_DIRECTORY_CONTROL (#12870)

c09ecb5

zmb3 deleted the isaiah/irp-mj-directory-control branch September 9, 2022 18:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`IRP_MJ_DIRECTORY_CONTROL` #12870

`IRP_MJ_DIRECTORY_CONTROL` #12870

ibeckermayer commented May 24, 2022

xacrimon Jun 1, 2022

ibeckermayer Jun 1, 2022

xacrimon Jun 1, 2022 •

edited

Loading

ibeckermayer Jun 28, 2022

zmb3 Jun 28, 2022

ibeckermayer Jun 28, 2022

zmb3 Jun 28, 2022

ibeckermayer Jun 28, 2022

IRP_MJ_DIRECTORY_CONTROL #12870

IRP_MJ_DIRECTORY_CONTROL #12870

Conversation

ibeckermayer commented May 24, 2022

xacrimon Jun 1, 2022

Choose a reason for hiding this comment

ibeckermayer Jun 1, 2022

Choose a reason for hiding this comment

xacrimon Jun 1, 2022 • edited Loading

Choose a reason for hiding this comment

ibeckermayer Jun 28, 2022

Choose a reason for hiding this comment

zmb3 Jun 28, 2022

Choose a reason for hiding this comment

ibeckermayer Jun 28, 2022

Choose a reason for hiding this comment

zmb3 Jun 28, 2022

Choose a reason for hiding this comment

ibeckermayer Jun 28, 2022

Choose a reason for hiding this comment

`IRP_MJ_DIRECTORY_CONTROL` #12870

`IRP_MJ_DIRECTORY_CONTROL` #12870

xacrimon Jun 1, 2022 •

edited

Loading