teleport
teleport copied to clipboard
Adds `DesktopSharedDirectoryStart` audit event
Overview
A shared directory is initialized by the TDP client generating a DirectoryID
and sending a SharedDirectoryAnnounce
followed by the TDP server sending back a SharedDirectoryAcknowledge
type SharedDirectoryAnnounce struct {
DirectoryID uint32
Name string
}
type SharedDirectoryAcknowledge struct {
ErrCode uint32
DirectoryID uint32
}
SharedDirectoryAnnounce
is the only TDP message which sends us the name of the directory being shared (Name
), after which the shared directory is always identified by its DirectoryID
in other TDP message types.
As a matter of convenience, we want to show the shared directory's name in the audit event logs, which means that we need to keep an internal cache mapping DirectoryID
s to directory names. This is why I've added a sharedDirectoryNameMap
to the WindowsService
https://github.com/gravitational/teleport/blob/200f16c471805262fa4a64fc3941a70334ca1dd3/lib/srv/desktop/windows_server.go#L96-L147
(Because we currently only allow for the sharing of a single directory, the sharedDirectoryNameMap
currently maps sessionID
to a directory's name. In the future, with multi-directory sharing enabled, we'd need a hash of the sessionID
+ DirectoryID
per name).
A DesktopSharedDirectoryStart
audit event is emitted via:
- We receive an incoming
SharedDirectoryAnnounce
- We add an entry with that directory's name to the
sharedDirectoryNameMap
- Later we receive a
SharedDirectoryAcknowledge
- We grab the directory's name from the
sharedDirectoryNameMap
and use it to emit aDesktopSharedDirectoryStart
audit event.
For discussion:
One point of note is that as currently designed, its technically possible that a SharedDirectoryAnnounce
event is sent, and the server never responds, thus no audit event ever shows up. We could ameliorate this by adding a separate DesktopSharedDirectoryAnnounce
(DesktopSharedDirectoryAttempt
?) event which is emitted upon that initial announcement.
I think we should add it, because the fact that a user attempted to share a directory is conceivably relevant to a security team.
Other changes
The other changes are primarily a refactoring of the tests (as well as adding a couple of tests for this event), see the commit message for 200f16c471805262fa4a64fc3941a70334ca1dd3