From 039d7a307f61f3ed57d846a5c4c71c1577deff71 Mon Sep 17 00:00:00 2001 From: Zac Bergquist Date: Wed, 30 Oct 2024 15:28:54 -0600 Subject: [PATCH] [v14] Fix the RDP licensing flow (#47544) * Fix the RDP licensing flow on v14 Licenses are stored in-memory, so if the agent restarts it will be forced to go through the "new license" flow for the first session. * Store RDP licenses in a LRU cache instead of a HashMap This ensures memory doesn't grow unbounded if we end up caching a large number of licenses. --- Cargo.lock | 59 ++++++++--- lib/srv/desktop/rdp/rdpclient/Cargo.toml | 8 +- lib/srv/desktop/rdp/rdpclient/client.go | 9 +- .../desktop/rdp/rdpclient/client_common.go | 6 +- lib/srv/desktop/rdp/rdpclient/src/cliprdr.rs | 1 + lib/srv/desktop/rdp/rdpclient/src/lib.rs | 97 ++++++++++++++++++- lib/srv/desktop/windows_server.go | 1 + 7 files changed, 159 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7148f803bba23..0261bb969c4ed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -17,6 +17,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "allocator-api2" +version = "0.2.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c6cb57a04249c6480766f7f7cef5467412af1490f8d1e243141daddada3264f" + [[package]] name = "asn1-rs" version = "0.5.1" @@ -452,6 +458,12 @@ dependencies = [ "termcolor", ] +[[package]] +name = "equivalent" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" + [[package]] name = "errno" version = "0.3.1" @@ -504,6 +516,12 @@ dependencies = [ "miniz_oxide 0.6.2", ] +[[package]] +name = "foldhash" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f81ec6369c545a7d40e4589b5597581fa1c441fe1cce96dd1de43159910a36a2" + [[package]] name = "foreign-types" version = "0.5.0" @@ -566,16 +584,6 @@ dependencies = [ "version_check", ] -[[package]] -name = "gethostname" -version = "0.2.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c1ebd34e35c46e00bb73e81363248d627782724609fe1b6396f553f68fe3862e" -dependencies = [ - "libc", - "winapi", -] - [[package]] name = "getrandom" version = "0.1.16" @@ -619,6 +627,17 @@ version = "0.12.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8a9ee70c43aaf417c914396645a0fa852624801b24ebb7ae78fe8272889ac888" +[[package]] +name = "hashbrown" +version = "0.15.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e087f84d4f86bf4b218b927129862374b72199ae7d8657835f1e89000eea4fb" +dependencies = [ + "allocator-api2", + "equivalent", + "foldhash", +] + [[package]] name = "heapless" version = "0.7.16" @@ -685,7 +704,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1885e79c1fc4b10f0e172c475f458b7f7b93061064d98c3293e98c5ba0c8b399" dependencies = [ "autocfg", - "hashbrown", + "hashbrown 0.12.3", ] [[package]] @@ -819,6 +838,15 @@ version = "0.4.20" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b5e6163cb8c49088c2c36f57875e58ccd8c87c7427f7fbd50ea6710b2f3f2e8f" +[[package]] +name = "lru" +version = "0.12.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "234cf4f4a04dc1f57e24b96cc0cd600cf2af460d4161ac5ecdd0af8e1f3b2a38" +dependencies = [ + "hashbrown 0.15.0", +] + [[package]] name = "md-5" version = "0.8.0" @@ -1218,6 +1246,7 @@ dependencies = [ "iso7816-tlv", "libc", "log", + "lru", "num-derive", "num-traits", "png", @@ -1233,12 +1262,11 @@ dependencies = [ [[package]] name = "rdp-rs" version = "0.1.0" -source = "git+https://github.com/gravitational/rdp-rs?rev=edfb5330a11d11eaf36d65e4300555368b4c6b02#edfb5330a11d11eaf36d65e4300555368b4c6b02" +source = "git+https://github.com/gravitational/rdp-rs?rev=2b0d99cc60c7b6474a1e2224a0bd6b2beca56b63#2b0d99cc60c7b6474a1e2224a0bd6b2beca56b63" dependencies = [ "boring", "bufstream", "byteorder", - "gethostname", "hmac", "indexmap", "md-5", @@ -1251,6 +1279,7 @@ dependencies = [ "ring", "rsa 0.6.1", "rustls", + "uuid", "x509-parser", "yasna", ] @@ -1706,9 +1735,9 @@ dependencies = [ [[package]] name = "uuid" -version = "1.4.1" +version = "1.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "79daa5ed5740825c40b389c5e50312b9c86df53fccd33f281df655642b43869d" +checksum = "81dfa00651efa65069b0b6b651f4aaa31ba9e3c3ce0137aaad053604ee7e0314" dependencies = [ "getrandom 0.2.8", ] diff --git a/lib/srv/desktop/rdp/rdpclient/Cargo.toml b/lib/srv/desktop/rdp/rdpclient/Cargo.toml index 64d155318516d..d7f696f1be9f7 100644 --- a/lib/srv/desktop/rdp/rdpclient/Cargo.toml +++ b/lib/srv/desktop/rdp/rdpclient/Cargo.toml @@ -1,7 +1,10 @@ [package] name = "rdp-client" version = "0.1.0" -authors = ["Andrew Lytvynov ", "Zac Bergquist "] +authors = [ + "Andrew Lytvynov ", + "Zac Bergquist ", +] edition = "2018" [lib] @@ -20,10 +23,11 @@ num-traits = "0.2.16" rand = { version = "0.8.5", features = ["getrandom"] } rand_chacha = "0.3.1" rsa = "0.9.2" -rdp-rs = { git = "https://github.com/gravitational/rdp-rs", rev = "edfb5330a11d11eaf36d65e4300555368b4c6b02" } +rdp-rs = { git = "https://github.com/gravitational/rdp-rs", rev = "2b0d99cc60c7b6474a1e2224a0bd6b2beca56b63" } uuid = { version = "1.4.1", features = ["v4"] } utf16string = "0.2.0" png = "0.17.10" +lru = "0.12.5" [build-dependencies] cbindgen = "0.25.0" diff --git a/lib/srv/desktop/rdp/rdpclient/client.go b/lib/srv/desktop/rdp/rdpclient/client.go index 8e4fc0b551af6..7c0bb25556c23 100644 --- a/lib/srv/desktop/rdp/rdpclient/client.go +++ b/lib/srv/desktop/rdp/rdpclient/client.go @@ -237,12 +237,14 @@ func (c *Client) connect(ctx context.Context) error { return trace.Wrap(err) } - // Addr and username strings only need to be valid for the duration of + // These strings only need to be valid for the duration of // C.connect_rdp. They are copied on the Rust side and can be freed here. addr := C.CString(c.cfg.Addr) defer C.free(unsafe.Pointer(addr)) username := C.CString(c.username) defer C.free(unsafe.Pointer(username)) + hostID := C.CString(c.cfg.HostID) + defer C.free(unsafe.Pointer(hostID)) cert_der, err := utils.UnsafeSliceData(userCertDER) if err != nil { @@ -261,8 +263,9 @@ func (c *Client) connect(ctx context.Context) error { res := C.connect_rdp( C.uintptr_t(c.handle), C.CGOConnectParams{ - go_addr: addr, - go_username: username, + go_addr: addr, + go_username: username, + go_client_id: hostID, // cert length and bytes. cert_der_len: C.uint32_t(len(userCertDER)), cert_der: (*C.uint8_t)(cert_der), diff --git a/lib/srv/desktop/rdp/rdpclient/client_common.go b/lib/srv/desktop/rdp/rdpclient/client_common.go index 5763598aa81d4..582d5e798faa4 100644 --- a/lib/srv/desktop/rdp/rdpclient/client_common.go +++ b/lib/srv/desktop/rdp/rdpclient/client_common.go @@ -32,10 +32,14 @@ import ( type Config struct { // Addr is the network address of the RDP server, in the form host:port. Addr string - // UserCertGenerator generates user certificates for RDP authentication. + + // GenerateUserCert generates user certificates for RDP authentication. GenerateUserCert GenerateUserCertFn CertTTL time.Duration + // HostID uniquely identifies the Teleport agent running the RDP client. + HostID string + // AuthorizeFn is called to authorize a user connecting to a Windows desktop. AuthorizeFn func(login string) error diff --git a/lib/srv/desktop/rdp/rdpclient/src/cliprdr.rs b/lib/srv/desktop/rdp/rdpclient/src/cliprdr.rs index 7bbb592d74834..2e5990adcc43c 100644 --- a/lib/srv/desktop/rdp/rdpclient/src/cliprdr.rs +++ b/lib/srv/desktop/rdp/rdpclient/src/cliprdr.rs @@ -749,6 +749,7 @@ enum ClipboardFormat { /// Sent as a reply to the format list PDU - used to indicate whether /// the format list PDU was processed succesfully. #[derive(Debug)] +#[allow(dead_code)] struct FormatListResponsePDU { // empty, the only information needed is the flags in the header } diff --git a/lib/srv/desktop/rdp/rdpclient/src/lib.rs b/lib/srv/desktop/rdp/rdpclient/src/lib.rs index d3e30365266db..7f8e0052c3deb 100644 --- a/lib/srv/desktop/rdp/rdpclient/src/lib.rs +++ b/lib/srv/desktop/rdp/rdpclient/src/lib.rs @@ -50,6 +50,7 @@ extern crate log; extern crate num_derive; use errors::try_error; +use lru::LruCache; use rand::Rng; use rand::SeedableRng; use rdp::core::event::*; @@ -60,6 +61,7 @@ use rdp::core::mcs; use rdp::core::sec; use rdp::core::tpkt; use rdp::core::x224; +use rdp::core::LicenseStore; use rdp::model::error::{Error as RdpError, RdpError as RdpProtocolError, RdpErrorKind, RdpResult}; use rdp::model::link::{Link, Stream}; use rdpdr::path::UnixPath; @@ -72,8 +74,10 @@ use std::io::ErrorKind; use std::io::{Cursor, Read, Write}; use std::net; use std::net::{TcpStream, ToSocketAddrs}; +use std::num::NonZeroUsize; use std::os::raw::c_char; use std::os::unix::io::AsRawFd; +use std::sync::OnceLock; use std::sync::{Arc, Mutex}; use std::{mem, ptr, slice, time}; @@ -182,6 +186,7 @@ pub unsafe extern "C" fn connect_rdp(go_ref: usize, params: CGOConnectParams) -> // Convert from C to Rust types. let addr = from_c_string(params.go_addr); let username = from_c_string(params.go_username); + let client_id = from_c_string(params.go_client_id); let cert_der = from_go_array(params.cert_der, params.cert_der_len); let key_der = from_go_array(params.key_der, params.key_der_len); @@ -190,6 +195,7 @@ pub unsafe extern "C" fn connect_rdp(go_ref: usize, params: CGOConnectParams) -> ConnectParams { addr, username, + client_id, cert_der, key_der, screen_width: params.screen_width, @@ -230,6 +236,7 @@ const RDPSND_CHANNEL_NAME: &str = "rdpsnd"; pub struct CGOConnectParams { go_addr: *const c_char, go_username: *const c_char, + go_client_id: *const c_char, cert_der_len: u32, cert_der: *mut u8, key_der_len: u32, @@ -244,6 +251,7 @@ pub struct CGOConnectParams { struct ConnectParams { addr: String, username: String, + client_id: String, cert_der: Vec, key_der: Vec, screen_width: u16, @@ -253,6 +261,8 @@ struct ConnectParams { show_desktop_wallpaper: bool, } +static LICENSE_STORE: OnceLock = OnceLock::new(); + fn connect_rdp_inner(go_ref: usize, params: ConnectParams) -> Result { // Connect and authenticate. let addr = params @@ -288,7 +298,7 @@ fn connect_rdp_inner(go_ref: usize, params: ConnectParams) -> Result Result Result>>, +} + +impl LruLicenseStore { + pub fn new() -> Self { + Self { + licenses: Mutex::new(LruCache::new( + NonZeroUsize::new(MAX_SAVED_LICENSES).unwrap(), + )), + } + } +} + +impl LicenseStore for &LruLicenseStore { + fn write_license( + &mut self, + major: u16, + minor: u16, + company: &str, + issuer: &str, + product_id: &str, + license: &[u8], + ) { + info!("Saving {major}.{minor} license from {issuer}"); + self.licenses.lock().unwrap().put( + LicenseStoreKey { + major, + minor, + company: company.to_owned(), + issuer: issuer.to_owned(), + product_id: product_id.to_owned(), + }, + license.to_vec(), + ); + } + + fn read_license( + &self, + major: u16, + minor: u16, + company: &str, + issuer: &str, + product_id: &str, + ) -> Option> { + let license = self + .licenses + .lock() + .unwrap() + .get(&LicenseStoreKey { + major, + minor, + company: company.to_owned(), + issuer: issuer.to_owned(), + product_id: product_id.to_owned(), + }) + .cloned(); + + if license.is_some() { + info!("Found existing {major}.{minor} license from {issuer}"); + } else { + info!("No existing {major}.{minor} license from {issuer}"); + } + + license + } +} + +#[derive(PartialEq, Eq, Hash)] +struct LicenseStoreKey { + major: u16, + minor: u16, + company: String, + issuer: String, + product_id: String, +} + // These functions are defined on the Go side. Look for functions with '//export funcname' // comments. extern "C" { diff --git a/lib/srv/desktop/windows_server.go b/lib/srv/desktop/windows_server.go index 07d2cc7ebfd8b..803df2a81a644 100644 --- a/lib/srv/desktop/windows_server.go +++ b/lib/srv/desktop/windows_server.go @@ -877,6 +877,7 @@ func (s *WindowsService) connectRDP(ctx context.Context, log logrus.FieldLogger, }, CertTTL: windows.CertTTL, Addr: addr.String(), + HostID: s.cfg.Heartbeat.HostUUID, Conn: tdpConn, AuthorizeFn: authorize, AllowClipboard: authCtx.Checker.DesktopClipboard(),