diff --git a/lib/srv/desktop/rdp/rdpclient/client.go b/lib/srv/desktop/rdp/rdpclient/client.go index e8a99e0369eed..088728627c4e4 100644 --- a/lib/srv/desktop/rdp/rdpclient/client.go +++ b/lib/srv/desktop/rdp/rdpclient/client.go @@ -142,7 +142,7 @@ type Client struct { } // New creates and connects a new Client based on cfg. -func New(ctx context.Context, cfg Config) (*Client, error) { +func New(cfg Config) (*Client, error) { if err := cfg.checkAndSetDefaults(); err != nil { return nil, err } diff --git a/lib/srv/desktop/rdp/rdpclient/client_nop.go b/lib/srv/desktop/rdp/rdpclient/client_nop.go index d32fd4adfccf6..0a341ad9f34af 100644 --- a/lib/srv/desktop/rdp/rdpclient/client_nop.go +++ b/lib/srv/desktop/rdp/rdpclient/client_nop.go @@ -34,7 +34,7 @@ type Client struct { } // New creates and connects a new Client based on opts. -func New(ctx context.Context, cfg Config) (*Client, error) { +func New(cfg Config) (*Client, error) { return &Client{}, errors.New("the real rdpclient.Client implementation was not included in this build") } diff --git a/lib/srv/desktop/rdp/rdpclient/src/lib.rs b/lib/srv/desktop/rdp/rdpclient/src/lib.rs index 0e74e0852ef0a..71a65ba197471 100644 --- a/lib/srv/desktop/rdp/rdpclient/src/lib.rs +++ b/lib/srv/desktop/rdp/rdpclient/src/lib.rs @@ -37,9 +37,10 @@ use rdp::core::tpkt; use rdp::core::x224; use rdp::model::error::{Error as RdpError, RdpError as RdpProtocolError, RdpErrorKind, RdpResult}; use rdp::model::link::{Link, Stream}; +use rdpdr::path::UnixPath; use rdpdr::ServerCreateDriveRequest; use std::convert::TryFrom; -use std::ffi::{CStr, CString}; +use std::ffi::CStr; use std::io::Error as IoError; use std::io::ErrorKind; use std::io::{Cursor, Read, Write}; @@ -306,7 +307,7 @@ fn connect_rdp_inner( let tdp_sd_info_request = Box::new(move |req: SharedDirectoryInfoRequest| -> RdpResult<()> { debug!("sending TDP SharedDirectoryInfoRequest: {:?}", req); // Create C compatible string from req.path - match CString::new(req.path.clone()) { + match req.path.to_cstring() { Ok(c_string) => { unsafe { let err = tdp_sd_info_request( @@ -328,7 +329,7 @@ fn connect_rdp_inner( Err(_) => { // TODO(isaiah): change TryError to TeleportError for a generic error caused by Teleport specific code. return Err(RdpError::TryError(format!( - "path contained characters that couldn't be converted to a C string: {}", + "path contained characters that couldn't be converted to a C string: {:?}", req.path ))); } @@ -339,7 +340,7 @@ fn connect_rdp_inner( Box::new(move |req: SharedDirectoryCreateRequest| -> RdpResult<()> { debug!("sending TDP SharedDirectoryCreateRequest: {:?}", req); // Create C compatible string from req.path - match CString::new(req.path.clone()) { + match req.path.to_cstring() { Ok(c_string) => { unsafe { let err = tdp_sd_create_request( @@ -362,7 +363,7 @@ fn connect_rdp_inner( Err(_) => { // TODO(isaiah): change TryError to TeleportError for a generic error caused by Teleport specific code. return Err(RdpError::TryError(format!( - "path contained characters that couldn't be converted to a C string: {}", + "path contained characters that couldn't be converted to a C string: {:?}", req.path ))); } @@ -373,7 +374,7 @@ fn connect_rdp_inner( Box::new(move |req: SharedDirectoryDeleteRequest| -> RdpResult<()> { debug!("sending TDP SharedDirectoryDeleteRequest: {:?}", req); // Create C compatible string from req.path - match CString::new(req.path.clone()) { + match req.path.to_cstring() { Ok(c_string) => { unsafe { let err = tdp_sd_delete_request( @@ -395,7 +396,7 @@ fn connect_rdp_inner( Err(_) => { // TODO(isaiah): change TryError to TeleportError for a generic error caused by Teleport specific code. return Err(RdpError::TryError(format!( - "path contained characters that couldn't be converted to a C string: {}", + "path contained characters that couldn't be converted to a C string: {:?}", req.path ))); } @@ -405,7 +406,7 @@ fn connect_rdp_inner( let tdp_sd_list_request = Box::new(move |req: SharedDirectoryListRequest| -> RdpResult<()> { debug!("sending TDP SharedDirectoryListRequest: {:?}", req); // Create C compatible string from req.path - match CString::new(req.path.clone()) { + match req.path.to_cstring() { Ok(c_string) => { unsafe { let err = tdp_sd_list_request( @@ -427,7 +428,7 @@ fn connect_rdp_inner( Err(_) => { // TODO(isaiah): change TryError to TeleportError for a generic error caused by Teleport specific code. return Err(RdpError::TryError(format!( - "path contained characters that couldn't be converted to a C string: {}", + "path contained characters that couldn't be converted to a C string: {:?}", req.path ))); } @@ -435,8 +436,8 @@ fn connect_rdp_inner( }); let tdp_sd_read_request = Box::new(move |req: SharedDirectoryReadRequest| -> RdpResult<()> { - debug!("sending: {:?}", req); - match CString::new(req.path.clone()) { + debug!("sending TDP SharedDirectoryReadRequest: {:?}", req); + match req.path.to_cstring() { Ok(c_string) => { unsafe { let err = tdp_sd_read_request( @@ -445,7 +446,7 @@ fn connect_rdp_inner( completion_id: req.completion_id, directory_id: req.directory_id, path: c_string.as_ptr(), - path_length: req.path.len() as u32, + path_length: req.path.len(), offset: req.offset, length: req.length, }, @@ -461,7 +462,7 @@ fn connect_rdp_inner( } Err(_) => { return Err(RdpError::TryError(format!( - "path contained characters that couldn't be converted to a C string: {}", + "path contained characters that couldn't be converted to a C string: {:?}", req.path ))); } @@ -469,8 +470,8 @@ fn connect_rdp_inner( }); let tdp_sd_write_request = Box::new(move |req: SharedDirectoryWriteRequest| -> RdpResult<()> { - debug!("sending: {:?}", req); - match CString::new(req.path.clone()) { + debug!("sending TDP SharedDirectoryWriteRequest: {:?}", req); + match req.path.to_cstring() { Ok(c_string) => { unsafe { let err = tdp_sd_write_request( @@ -480,7 +481,7 @@ fn connect_rdp_inner( directory_id: req.directory_id, offset: req.offset, path: c_string.as_ptr(), - path_length: req.path.len() as u32, + path_length: req.path.len(), write_data_length: req.write_data.len() as u32, write_data: req.write_data.as_ptr() as *mut u8, }, @@ -496,7 +497,7 @@ fn connect_rdp_inner( } Err(_) => { return Err(RdpError::TryError(format!( - "path contained characters that couldn't be converted to a C string: {}", + "path contained characters that couldn't be converted to a C string: {:?}", req.path ))); } @@ -1365,7 +1366,7 @@ pub type CGOSharedDirectoryAcknowledge = SharedDirectoryAcknowledge; pub struct SharedDirectoryInfoRequest { completion_id: u32, directory_id: u32, - path: String, + path: UnixPath, } #[repr(C)] @@ -1380,7 +1381,7 @@ impl From for SharedDirectoryInfoRequest { SharedDirectoryInfoRequest { completion_id: req.device_io_request.completion_id, directory_id: req.device_io_request.device_id, - path: req.path, + path: UnixPath::from(&req.path), } } } @@ -1423,12 +1424,12 @@ pub struct FileSystemObject { last_modified: u64, size: u64, file_type: FileType, - path: String, + path: UnixPath, } impl FileSystemObject { fn name(&self) -> RdpResult { - if let Some(name) = self.path.split('/').last() { + if let Some(name) = self.path.last() { Ok(name.to_string()) } else { Err(try_error(&format!( @@ -1460,7 +1461,7 @@ impl From for FileSystemObject { last_modified: cgo_fso.last_modified, size: cgo_fso.size, file_type: cgo_fso.file_type, - path: from_go_string(cgo_fso.path), + path: UnixPath::from(from_go_string(cgo_fso.path)), } } } @@ -1493,7 +1494,7 @@ pub struct SharedDirectoryWriteRequest { completion_id: u32, directory_id: u32, offset: u64, - path: String, + path: UnixPath, write_data: Vec, } @@ -1515,7 +1516,7 @@ pub struct CGOSharedDirectoryWriteRequest { pub struct SharedDirectoryReadRequest { completion_id: u32, directory_id: u32, - path: String, + path: UnixPath, offset: u64, length: u32, } @@ -1580,7 +1581,7 @@ pub struct SharedDirectoryCreateRequest { completion_id: u32, directory_id: u32, file_type: FileType, - path: String, + path: UnixPath, } #[repr(C)] diff --git a/lib/srv/desktop/rdp/rdpclient/src/rdpdr/mod.rs b/lib/srv/desktop/rdp/rdpclient/src/rdpdr/mod.rs index 1872f461b008f..20db61cf7032b 100644 --- a/lib/srv/desktop/rdp/rdpclient/src/rdpdr/mod.rs +++ b/lib/srv/desktop/rdp/rdpclient/src/rdpdr/mod.rs @@ -14,8 +14,10 @@ mod consts; mod flags; +pub(crate) mod path; mod scard; +use self::path::{UnixPath, WindowsPath}; use crate::errors::{ invalid_data_error, not_implemented_error, rejected_by_server_error, try_error, NTSTATUS_OK, SPECIAL_NO_RESPONSE, @@ -462,7 +464,7 @@ impl Client { let file_id = cli.generate_file_id(); cli.file_cache.insert( file_id, - FileCacheObject::new(rdp_req.path.clone(), res.fso), + FileCacheObject::new(UnixPath::from(&rdp_req.path), res.fso), ); return cli.prep_device_create_response( &rdp_req, @@ -495,7 +497,7 @@ impl Client { let file_id = cli.generate_file_id(); cli.file_cache.insert( file_id, - FileCacheObject::new(rdp_req.path.clone(), res.fso), + FileCacheObject::new(UnixPath::from(&rdp_req.path), res.fso), ); return cli.prep_device_create_response( &rdp_req, @@ -1144,7 +1146,7 @@ impl Client { completion_id: rdp_req.device_io_request.completion_id, directory_id: rdp_req.device_io_request.device_id, file_type, - path: rdp_req.path.clone(), + path: UnixPath::from(&rdp_req.path), }; (self.tdp_sd_create_request)(tdp_req)?; @@ -1163,8 +1165,10 @@ impl Client { } let file_id = cli.generate_file_id(); - cli.file_cache - .insert(file_id, FileCacheObject::new(rdp_req.path.clone(), fso)); + cli.file_cache.insert( + file_id, + FileCacheObject::new(UnixPath::from(&rdp_req.path), fso), + ); cli.prep_device_create_response(&rdp_req, NTSTATUS::STATUS_SUCCESS, file_id) }, ), @@ -1183,7 +1187,7 @@ impl Client { let tdp_req = SharedDirectoryDeleteRequest { completion_id: rdp_req.device_io_request.completion_id, directory_id: rdp_req.device_io_request.device_id, - path: rdp_req.path.clone(), + path: UnixPath::from(&rdp_req.path), }; (self.tdp_sd_delete_request)(tdp_req)?; self.pending_sd_delete_resp_handlers.insert( @@ -1369,7 +1373,7 @@ impl Client { /// | -------- | ------------- | ---------------------------------------------------------| #[derive(Debug)] struct FileCacheObject { - path: String, + path: UnixPath, delete_pending: bool, /// The FileSystemObject pertaining to the file or directory at path. fso: FileSystemObject, @@ -1385,7 +1389,7 @@ struct FileCacheObject { } impl FileCacheObject { - fn new(path: String, fso: FileSystemObject) -> Self { + fn new(path: UnixPath, fso: FileSystemObject) -> Self { Self { path, delete_pending: false, @@ -1427,7 +1431,7 @@ impl Iterator for FileCacheObject { last_modified: self.fso.last_modified, size: self.fso.size, file_type: self.fso.file_type, - path: ".".to_string(), + path: UnixPath::from(".".to_string()), }) } else if !self.dotdot_sent { // On the second call to next, return the ".." directory @@ -1436,7 +1440,7 @@ impl Iterator for FileCacheObject { last_modified: self.fso.last_modified, size: 0, file_type: FileType::Directory, - path: "..".to_string(), + path: UnixPath::from("..".to_string()), }) } else { // "." and ".." have been sent, now start iterating through @@ -2089,7 +2093,7 @@ pub struct DeviceCreateRequest { create_disposition: flags::CreateDisposition, create_options: flags::CreateOptions, path_length: u32, - pub path: String, + pub path: WindowsPath, } impl DeviceCreateRequest { @@ -2114,7 +2118,7 @@ impl DeviceCreateRequest { // for a u32 will never panic on the machines that run teleport. let mut path = vec![0u8; path_length.try_into().unwrap()]; payload.read_exact(&mut path)?; - let path = util::from_unicode(path)?; + let path = WindowsPath::from(util::from_unicode(path)?); Ok(Self { device_io_request, @@ -3451,7 +3455,7 @@ struct ServerDriveQueryDirectoryRequest { /// A variable-length array of Unicode characters (we will store this as a regular rust String) that specifies the directory /// on which this operation will be performed. The Path field MUST be null-terminated. If the value of the InitialQuery field /// is zero, then the contents of the Path field MUST be ignored, irrespective of the value specified in the PathLength field. - path: String, + path: WindowsPath, } impl ServerDriveQueryDirectoryRequest { @@ -3478,7 +3482,7 @@ impl ServerDriveQueryDirectoryRequest { let initial_query = payload.read_u8()?; let mut path_length: u32 = 0; - let mut path = String::from(""); + let mut path = WindowsPath::from("".to_string()); let mut padding: [u8; 23] = [0; 23]; if initial_query != 0 { path_length = payload.read_u32::()?; @@ -3489,7 +3493,7 @@ impl ServerDriveQueryDirectoryRequest { // TODO(isaiah): make a from_unicode_exact let mut path_as_vec = vec![0u8; path_length.try_into().unwrap()]; payload.read_exact(&mut path_as_vec)?; - path = util::from_unicode(path_as_vec)?; + path = WindowsPath::from(util::from_unicode(path_as_vec)?); } Ok(Self { diff --git a/lib/srv/desktop/rdp/rdpclient/src/rdpdr/path.rs b/lib/srv/desktop/rdp/rdpclient/src/rdpdr/path.rs new file mode 100644 index 0000000000000..2c512263df6eb --- /dev/null +++ b/lib/srv/desktop/rdp/rdpclient/src/rdpdr/path.rs @@ -0,0 +1,120 @@ +// Copyright 2022 Gravitational, Inc +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +use std::ffi::{CString, NulError}; + +/// WindowsPath is a String that we assume to be in the form +/// of a traditional DOS path: +/// +/// https://docs.microsoft.com/en-us/dotnet/standard/io/file-path-formats +/// +/// Because RDP device redirection is limited in the paths it uses, we can +/// further assume that it is in one of the following forms: +/// +/// r"\Program Files\Custom Utilities\StringFinder.exe": An absolute path from the root of the current drive. +/// +/// r"2018\January.xlsx": A relative path to a file in a subdirectory of the current directory. +#[derive(Debug, Clone)] +pub struct WindowsPath { + path: String, +} + +impl From for WindowsPath { + fn from(path: String) -> WindowsPath { + Self { path } + } +} + +/// UnixPath is a String that we assume to be in the form of a +/// Unix Path, qualified by the qualifications laid out in RFD 0067 +/// +/// https://github.com/gravitational/teleport/blob/master/rfd/0067-desktop-access-file-system-sharing.md +#[derive(Debug, Clone)] +pub struct UnixPath { + pub path: String, +} + +impl UnixPath { + /// This function will create a CString from a UnixPath. + /// + /// # Errors + /// + /// This function will return an error if the UnixPath contains + /// any characters that can't be handled by CString::new(). + pub fn to_cstring(&self) -> Result { + CString::new(self.path.clone()) + } + + pub fn len(&self) -> u32 { + self.path.len() as u32 + } + + pub fn last(&self) -> Option<&str> { + self.path.split('/').last() + } +} + +impl From<&WindowsPath> for UnixPath { + fn from(p: &WindowsPath) -> UnixPath { + Self::from(to_unix_path(&p.path)) + } +} + +impl From for UnixPath { + fn from(path: String) -> UnixPath { + Self { path } + } +} + +/// Converts a String from the type of path that's sent to us by RDP +/// into a unix-style path, as specified in Teleport RFD 0067: +/// +/// https://github.com/gravitational/teleport/blob/master/rfd/0067-desktop-access-file-system-sharing.md +fn to_unix_path(rdp_path: &str) -> String { + // Convert r"\" to "/" + let mut cleaned = rdp_path.replace('\\', "/"); + + // If the string started with r"\", just remove it + if cleaned.starts_with('/') { + crop_first_n_letters(&mut cleaned, 1); + } + + cleaned +} + +/// Crops the first n letters off of a String (in-place). +fn crop_first_n_letters(s: &mut String, n: usize) { + match s.char_indices().nth(n) { + Some((pos, _)) => { + s.drain(..pos); + } + None => { + s.clear(); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_to_unix_path() { + assert_eq!(to_unix_path(r"\"), ""); + assert_eq!(to_unix_path(r"\desktop.ini"), "desktop.ini"); + assert_eq!( + to_unix_path(r"\test_directory\desktop.ini"), + "test_directory/desktop.ini" + ); + } +} diff --git a/lib/srv/desktop/windows_server.go b/lib/srv/desktop/windows_server.go index b2d9c12474fae..1834ec38f4d1e 100644 --- a/lib/srv/desktop/windows_server.go +++ b/lib/srv/desktop/windows_server.go @@ -834,7 +834,7 @@ func (s *WindowsService) connectRDP(ctx context.Context, log logrus.FieldLogger, tdpConn.OnRecv = s.makeTDPReceiveHandler(ctx, sw, delay, &identity, string(sessionID), desktop.GetAddr()) sessionStartTime := s.cfg.Clock.Now().UTC().Round(time.Millisecond) - rdpc, err := rdpclient.New(ctx, rdpclient.Config{ + rdpc, err := rdpclient.New(rdpclient.Config{ Log: log, GenerateUserCert: func(ctx context.Context, username string, ttl time.Duration) (certDER, keyDER []byte, err error) { return s.generateCredentials(ctx, username, desktop.GetDomain(), ttl)