From 294ac3bea78e96d85e88136eb1b095a24c7c49aa Mon Sep 17 00:00:00 2001 From: Allan Zhang <6740989+allan2@users.noreply.github.com> Date: Tue, 20 May 2025 12:56:04 -0400 Subject: [PATCH 1/7] feat(pedm): account tracking This commit adds tracking of user accounts and domain accounts. Changes to accounts on the system such as name change, creation, removal, and even SID change are captured. Accounts now use an internally generated stable ID. This stable ID can be used to build policies in a robust manner. The functionality has been tested with real account creation, deletion, and removal on Windows for both DB backends. This commit also includes query helpers for parametrization and bulk insertion. A basic endpoint, `/accounts`, is added for displaying info about existing accounts in a fashion similar to `Get-LocalUser`. --- Cargo.lock | 197 ++-- crates/devolutions-pedm/Cargo.toml | 7 + crates/devolutions-pedm/build.rs | 19 + crates/devolutions-pedm/schema/libsql.sql | 76 +- crates/devolutions-pedm/schema/pg.sql | 57 ++ crates/devolutions-pedm/src/account.rs | 1006 ++++++++++++++++++++ crates/devolutions-pedm/src/api/account.rs | 34 + crates/devolutions-pedm/src/api/launch.rs | 4 - crates/devolutions-pedm/src/api/mod.rs | 22 + crates/devolutions-pedm/src/db/err.rs | 2 +- crates/devolutions-pedm/src/db/libsql.rs | 237 ++++- crates/devolutions-pedm/src/db/mod.rs | 12 + crates/devolutions-pedm/src/db/pg.rs | 251 ++++- crates/devolutions-pedm/src/db/util.rs | 92 ++ crates/devolutions-pedm/src/lib.rs | 1 + crates/devolutions-pedm/src/policy.rs | 16 +- 16 files changed, 1942 insertions(+), 91 deletions(-) create mode 100644 crates/devolutions-pedm/build.rs create mode 100644 crates/devolutions-pedm/src/account.rs create mode 100644 crates/devolutions-pedm/src/api/account.rs create mode 100644 crates/devolutions-pedm/src/db/util.rs diff --git a/Cargo.lock b/Cargo.lock index 6e008e138..b9a404cbd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 4 +version = 3 [[package]] name = "addr2line" @@ -554,7 +554,7 @@ version = "0.66.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f2b84e06fc203107bfbad243f4aba2af864eb7db3b1cf46ea0a023b0b433d2a7" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", "cexpr", "clang-sys", "lazy_static", @@ -577,7 +577,7 @@ version = "0.69.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "271383c67ccabffb7381723dea0672a673f292304fcb45c01cc648c7a8d58088" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", "cexpr", "clang-sys", "itertools", @@ -623,9 +623,9 @@ checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" [[package]] name = "bitflags" -version = "2.9.0" +version = "2.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5c8214115b7bf84099f1309324e63141d4c5d7cc26862f97a0a857dbefe165bd" +checksum = "1b8e56985ec62d17e9c1001dc89c88ecd7dc08e47eba5ec7c29c7b5eeecde967" [[package]] name = "bitvec" @@ -731,9 +731,9 @@ dependencies = [ [[package]] name = "cc" -version = "1.2.22" +version = "1.2.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "32db95edf998450acc7881c932f94cd9b05c87b4b2599e8bab064753da4acfd1" +checksum = "5f4ac86a9e5bc1e2b3449ab9d7d3a6a405e3d1bb28d7b9be8614f55846ae3766" dependencies = [ "jobserver", "libc", @@ -1213,7 +1213,7 @@ dependencies = [ "parking_lot", "pcap-file", "picky", - "picky-krb 0.9.4", + "picky-krb 0.9.5", "pin-project-lite 0.2.16", "portpicker", "proptest", @@ -1319,6 +1319,8 @@ dependencies = [ "tracing", "uuid", "win-api-wrappers", + "windows 0.61.1", + "windows-result 0.3.4", ] [[package]] @@ -1625,9 +1627,9 @@ checksum = "877a4ace8713b0bcf2a4e7eec82529c029f1d0619886d18145fea96c3ffe5c0f" [[package]] name = "errno" -version = "0.3.11" +version = "0.3.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "976dd42dc7e85965fe702eb8164f21f450704bdde31faefd6471dba214cb594e" +checksum = "cea14ef9355e3beab063703aa9dab15afd25f0667c341310c1e5274bb1d0da18" dependencies = [ "libc", "windows-sys 0.59.0", @@ -1878,15 +1880,16 @@ dependencies = [ [[package]] name = "generator" -version = "0.8.4" +version = "0.8.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cc6bd114ceda131d3b1d665eba35788690ad37f5916457286b32ab6fd3c438dd" +checksum = "d18470a76cb7f8ff746cf1f7470914f900252ec36bbc40b569d74b1258446827" dependencies = [ + "cc", "cfg-if", "libc", "log", "rustversion", - "windows 0.58.0", + "windows 0.61.1", ] [[package]] @@ -2302,9 +2305,9 @@ dependencies = [ [[package]] name = "hyper-util" -version = "0.1.11" +version = "0.1.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "497bbc33a26fdd4af9ed9c70d63f61cf56a938375fbb32df34db9b1cd6d643f2" +checksum = "cf9f1e950e0d9d1d3c47184416723cf29c0d1f93bd8cccf37e4beb6b44f31710" dependencies = [ "bytes 1.10.1", "futures-channel", @@ -2332,7 +2335,7 @@ dependencies = [ "js-sys", "log", "wasm-bindgen", - "windows-core 0.61.0", + "windows-core 0.61.2", ] [[package]] @@ -2393,9 +2396,9 @@ checksum = "00210d6893afc98edb752b664b8890f0ef174c8adbb8d0be9710fa66fbbf72d3" [[package]] name = "icu_properties" -version = "2.0.0" +version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2549ca8c7241c82f59c80ba2a6f415d931c5b58d24fb8412caa1a1f02c49139a" +checksum = "016c619c1eeb94efb86809b015c58f479963de65bdb6253345c1a1276f22e32b" dependencies = [ "displaydoc", "icu_collections", @@ -2409,9 +2412,9 @@ dependencies = [ [[package]] name = "icu_properties_data" -version = "2.0.0" +version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8197e866e47b68f8f7d95249e172903bec06004b18b2937f1095d40a0c57de04" +checksum = "298459143998310acd25ffe6810ed544932242d3f07083eee1084d83a71bd632" [[package]] name = "icu_provider" @@ -2565,7 +2568,7 @@ name = "ironrdp-ainput" version = "0.1.0" source = "git+https://github.com/Devolutions/IronRDP?rev=2e1a9ac88e38e7d92d893007bc25d0a05c365861#2e1a9ac88e38e7d92d893007bc25d0a05c365861" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", "ironrdp-dvc", "ironrdp-pdu 0.1.0 (git+https://github.com/Devolutions/IronRDP?rev=2e1a9ac88e38e7d92d893007bc25d0a05c365861)", "num-derive", @@ -2588,7 +2591,7 @@ name = "ironrdp-cliprdr" version = "0.1.0" source = "git+https://github.com/Devolutions/IronRDP?rev=2e1a9ac88e38e7d92d893007bc25d0a05c365861#2e1a9ac88e38e7d92d893007bc25d0a05c365861" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", "ironrdp-pdu 0.1.0 (git+https://github.com/Devolutions/IronRDP?rev=2e1a9ac88e38e7d92d893007bc25d0a05c365861)", "ironrdp-svc", "thiserror 1.0.69", @@ -2671,7 +2674,7 @@ version = "0.1.0" source = "git+https://github.com/Devolutions/IronRDP?rev=2e1a9ac88e38e7d92d893007bc25d0a05c365861#2e1a9ac88e38e7d92d893007bc25d0a05c365861" dependencies = [ "bit_field", - "bitflags 2.9.0", + "bitflags 2.9.1", "bitvec", "byteorder", "ironrdp-error 0.1.0 (git+https://github.com/Devolutions/IronRDP?rev=2e1a9ac88e38e7d92d893007bc25d0a05c365861)", @@ -2688,7 +2691,7 @@ version = "0.1.0" source = "git+https://github.com/Devolutions/IronRDP?rev=2e1a9ac88e38e7d92d893007bc25d0a05c365861#2e1a9ac88e38e7d92d893007bc25d0a05c365861" dependencies = [ "bit_field", - "bitflags 2.9.0", + "bitflags 2.9.1", "byteorder", "der-parser", "ironrdp-error 0.1.0 (git+https://github.com/Devolutions/IronRDP?rev=2e1a9ac88e38e7d92d893007bc25d0a05c365861)", @@ -2710,7 +2713,7 @@ version = "0.1.0" source = "git+https://github.com/Devolutions/IronRDP?rev=7c268d863048d0a9182b3f7bf778668de8db4ccf#7c268d863048d0a9182b3f7bf778668de8db4ccf" dependencies = [ "bit_field", - "bitflags 2.9.0", + "bitflags 2.9.1", "byteorder", "der-parser", "ironrdp-core 0.1.0", @@ -2740,7 +2743,7 @@ name = "ironrdp-rdpsnd" version = "0.1.0" source = "git+https://github.com/Devolutions/IronRDP?rev=2e1a9ac88e38e7d92d893007bc25d0a05c365861#2e1a9ac88e38e7d92d893007bc25d0a05c365861" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", "ironrdp-pdu 0.1.0 (git+https://github.com/Devolutions/IronRDP?rev=2e1a9ac88e38e7d92d893007bc25d0a05c365861)", "ironrdp-svc", "tracing", @@ -2773,7 +2776,7 @@ name = "ironrdp-svc" version = "0.1.0" source = "git+https://github.com/Devolutions/IronRDP?rev=2e1a9ac88e38e7d92d893007bc25d0a05c365861#2e1a9ac88e38e7d92d893007bc25d0a05c365861" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", "ironrdp-pdu 0.1.0 (git+https://github.com/Devolutions/IronRDP?rev=2e1a9ac88e38e7d92d893007bc25d0a05c365861)", ] @@ -3075,23 +3078,23 @@ version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c0ff37bd590ca25063e35af745c343cb7a0271906fb7b37e4813e8f79f00268d" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", "libc", "redox_syscall", ] [[package]] name = "libsql" -version = "0.9.6" +version = "0.9.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "096b80492b42179396a551078a24dbc8d50c135e80098e10cf46e1ce82b86e09" +checksum = "ac8482f800f3f515e1d96af82bbb47d57d1edbc585642316eebc045f72a1930e" dependencies = [ "anyhow", "async-stream", "async-trait", "base64 0.21.7", "bincode", - "bitflags 2.9.0", + "bitflags 2.9.1", "bytes 1.10.1", "chrono", "crc32fast", @@ -3121,9 +3124,9 @@ dependencies = [ [[package]] name = "libsql-ffi" -version = "0.9.6" +version = "0.9.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dfaffc5d1d4e990961c48989b04252c22dd6628c1e0c66a4724d3918932cc96e" +checksum = "1f78ac450ec98334db75ffda145c9f17377f6aa0c5a2529d233844b7b7841a48" dependencies = [ "bindgen 0.66.1", "cc", @@ -3132,9 +3135,9 @@ dependencies = [ [[package]] name = "libsql-hrana" -version = "0.9.6" +version = "0.9.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "af3522399868e2302c47ae31902017e2eccbf51c50e6d7ad78eb7e96a640dd43" +checksum = "7df6ae47bfedf165fb22d1aad3ca983b0aa27782fea48f98b5142f36c02b459d" dependencies = [ "base64 0.21.7", "bytes 1.10.1", @@ -3144,11 +3147,11 @@ dependencies = [ [[package]] name = "libsql-rusqlite" -version = "0.9.6" +version = "0.9.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4487c5a1ee674aae0f300cb5507af03ddb9989159ff32928f3379db19cfee4a7" +checksum = "9a462d9e10715139636d7f17351ba28c4b5fbc439110282ef3aa53770c3a8ba5" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", "fallible-iterator 0.2.0", "fallible-streaming-iterator", "hashlink", @@ -3162,7 +3165,7 @@ version = "0.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "15a90128c708356af8f7d767c9ac2946692c9112b4f74f07b99a01a60680e413" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", "cc", "fallible-iterator 0.3.0", "indexmap 2.9.0", @@ -3176,9 +3179,9 @@ dependencies = [ [[package]] name = "libsql-sys" -version = "0.9.6" +version = "0.9.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "506f309248a5ddca722b5337b9f6e0e1dbe35d829b8016f0bd409d8792d3b04d" +checksum = "76ca92a20f9b274aecfa2861e0a447d43f2b4c48dc6c8b63b6702d22c51e15eb" dependencies = [ "bytes 1.10.1", "libsql-ffi", @@ -3190,9 +3193,9 @@ dependencies = [ [[package]] name = "libsql_replication" -version = "0.9.6" +version = "0.9.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bc11901506831b1f0574a69c5f22a0e466feedeb6fcc26868a424992f8d8e44b" +checksum = "0e7f478774f05f272943a6d6b22c401c14178070c9919a8c678205b69bb70d16" dependencies = [ "aes", "async-stream", @@ -3631,7 +3634,7 @@ version = "0.27.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2eb04e9c688eff1c89d72b407f168cf79bb9e867a9d3323ed6c01519eb9cc053" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", "cfg-if", "libc", ] @@ -3642,7 +3645,7 @@ version = "0.30.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "74523f3a35e05aba87a1d978330aef40f67b0304ac79c1c00b294c9830543db6" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", "cfg-if", "cfg_aliases", "libc", @@ -3670,7 +3673,7 @@ version = "6.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6205bd8bb1e454ad2e27422015fb5e4f2bcc7e08fa8f27058670d208324a4d2d" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", "crossbeam-channel", "filetime", "fsevent-sys", @@ -3700,7 +3703,7 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "063dca685ea8efa62d1a3566332b08be0198922f1d8aced1ead413c9f02fd89e" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", "ironrdp-core 0.1.4", "ironrdp-error 0.1.2", ] @@ -3854,7 +3857,7 @@ version = "0.10.72" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fedfea7d58a1f73118430a55da6a286e7b044961736ce96a16a17068ea25e5da" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", "cfg-if", "foreign-types", "libc", @@ -4068,9 +4071,9 @@ dependencies = [ [[package]] name = "picky" -version = "7.0.0-rc.13" +version = "7.0.0-rc.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "68ff00229e235526557077232ea50a8f619bbba9f3d37c7f63d1d41502546bec" +checksum = "162c4c060a9813dcae344eba16ea9281d1ca022ad733f13142d27b5b495eebc3" dependencies = [ "aes", "aes-gcm", @@ -4209,9 +4212,9 @@ dependencies = [ [[package]] name = "picky-krb" -version = "0.9.4" +version = "0.9.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "322b38c31330193b1d428df5b88f1b0d3d9e91548d302844c4f221dd839b1a7d" +checksum = "28b1e729dbb3da7f9ed4b66be4bba2ed0da8414db74b6cbe8c4ceae27af797d8" dependencies = [ "aes", "byteorder", @@ -4501,7 +4504,7 @@ checksum = "14cae93065090804185d3b75f0bf93b8eeda30c7a9b4a33d3bdb3988d6229e50" dependencies = [ "bit-set", "bit-vec", - "bitflags 2.9.0", + "bitflags 2.9.1", "lazy_static", "num-traits", "rand 0.8.5", @@ -4797,7 +4800,7 @@ version = "0.5.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "928fca9cf2aa042393a8325b9ead81d2f0df4cb12e1e24cef072922ccd99c5af" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", ] [[package]] @@ -5083,7 +5086,7 @@ version = "0.38.44" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fdb5bc1ae2baa591800df16c9ca78619bf65c0488b41b96ccec5d11220d8c154" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", "errno", "libc", "linux-raw-sys 0.4.15", @@ -5096,7 +5099,7 @@ version = "1.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c71e83d6afe7ff64890ec6b71d6a69bb8a610ab78ce364b3352876bb4c801266" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", "errno", "libc", "linux-raw-sys 0.9.4", @@ -5317,7 +5320,7 @@ version = "2.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "897b2245f0b511c87893af39b033e5ca9cce68824c4d7e7630b5a1d339658d02" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", "core-foundation 0.9.4", "core-foundation-sys", "libc", @@ -5330,7 +5333,7 @@ version = "3.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "271720403f46ca04f7ba6f55d438f8bd878d6b8ca0a1046e8228c4145bcbb316" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", "core-foundation 0.10.0", "core-foundation-sys", "libc", @@ -5639,7 +5642,7 @@ checksum = "18d31fab47d9290be28a8d027c8428756826f1d4fe1e5ba0f51d24f52c568e21" dependencies = [ "async-dnssd", "async-recursion", - "bitflags 2.9.0", + "bitflags 2.9.1", "byteorder", "cfg-if", "crypto-mac", @@ -6310,7 +6313,7 @@ version = "0.4.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "61c5bb1d698276a2443e5ecfabc1008bf15a36c12e6a7176e7bf089ea9131140" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", "bytes 1.10.1", "futures-core", "futures-util", @@ -6330,7 +6333,7 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1e9cd434a998747dd2c4276bc96ee2e0c7a2eadf3cae88e52be55a05fa9053f5" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", "bytes 1.10.1", "futures-util", "http 1.3.1", @@ -7083,6 +7086,28 @@ dependencies = [ "windows-targets 0.52.6", ] +[[package]] +name = "windows" +version = "0.61.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c5ee8f3d025738cb02bad7868bbb5f8a6327501e870bf51f1b455b0a2454a419" +dependencies = [ + "windows-collections", + "windows-core 0.61.2", + "windows-future", + "windows-link", + "windows-numerics", +] + +[[package]] +name = "windows-collections" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3beeceb5e5cfd9eb1d76b381630e82c4241ccd0d27f1a39ed41b2760b255c5e8" +dependencies = [ + "windows-core 0.61.2", +] + [[package]] name = "windows-core" version = "0.51.1" @@ -7116,15 +7141,26 @@ dependencies = [ [[package]] name = "windows-core" -version = "0.61.0" +version = "0.61.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4763c1de310c86d75a878046489e2e5ba02c649d185f21c67d4cf8a56d098980" +checksum = "c0fdd3ddb90610c7638aa2b3a3ab2904fb9e5cdbecc643ddb3647212781c4ae3" dependencies = [ "windows-implement 0.60.0", "windows-interface 0.59.1", "windows-link", - "windows-result 0.3.2", - "windows-strings 0.4.0", + "windows-result 0.3.4", + "windows-strings 0.4.2", +] + +[[package]] +name = "windows-future" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fc6a41e98427b19fe4b73c550f060b59fa592d7d686537eebf9385621bfbad8e" +dependencies = [ + "windows-core 0.61.2", + "windows-link", + "windows-threading", ] [[package]] @@ -7177,6 +7213,16 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "76840935b766e1b0a05c0066835fb9ec80071d4c09a16f6bd5f7e655e3c14c38" +[[package]] +name = "windows-numerics" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9150af68066c4c5c07ddc0ce30421554771e528bde427614c61038bc2c92c2b1" +dependencies = [ + "windows-core 0.61.2", + "windows-link", +] + [[package]] name = "windows-registry" version = "0.2.0" @@ -7205,7 +7251,7 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4286ad90ddb45071efd1a66dfa43eb02dd0dfbae1545ad6cc3c51cf34d7e8ba3" dependencies = [ - "windows-result 0.3.2", + "windows-result 0.3.4", "windows-strings 0.3.1", "windows-targets 0.53.0", ] @@ -7221,9 +7267,9 @@ dependencies = [ [[package]] name = "windows-result" -version = "0.3.2" +version = "0.3.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c64fd11a4fd95df68efcfee5f44a294fe71b8bc6a91993e2791938abcc712252" +checksum = "56f42bd332cc6c8eac5af113fc0c1fd6a8fd2aa08a0119358686e5160d0586c6" dependencies = [ "windows-link", ] @@ -7258,9 +7304,9 @@ dependencies = [ [[package]] name = "windows-strings" -version = "0.4.0" +version = "0.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7a2ba9642430ee452d5a7aa78d72907ebe8cfda358e8cb7918a2050581322f97" +checksum = "56e6c93f3a0c3b36176cb1327a4958a0353d5d166c2a35cb268ace15e91d3b57" dependencies = [ "windows-link", ] @@ -7363,6 +7409,15 @@ dependencies = [ "windows_x86_64_msvc 0.53.0", ] +[[package]] +name = "windows-threading" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b66463ad2e0ea3bbf808b7f1d371311c80e115c0b71d60efc142cafbcfb057a6" +dependencies = [ + "windows-link", +] + [[package]] name = "windows_aarch64_gnullvm" version = "0.42.2" @@ -7597,7 +7652,7 @@ version = "0.39.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6f42320e61fe2cfd34354ecb597f86f413484a798ba44a8ca1165c58d42da6c1" dependencies = [ - "bitflags 2.9.0", + "bitflags 2.9.1", ] [[package]] diff --git a/crates/devolutions-pedm/Cargo.toml b/crates/devolutions-pedm/Cargo.toml index 6f3a2a4a7..7efd2fc2c 100644 --- a/crates/devolutions-pedm/Cargo.toml +++ b/crates/devolutions-pedm/Cargo.toml @@ -45,6 +45,13 @@ tokio-postgres = { version = "0.7", optional = true, features = ["with-chrono-0_ bb8 = { version = "0.9", optional = true } bb8-postgres = { version = "0.9", optional = true } +[target.'cfg(windows)'.dependencies] +windows = { version = "0.61", features = [ + "Win32_NetworkManagement_NetManagement", + "Win32_Security_Authorization", +] } +windows-result = "0.3" + [features] default = ["libsql"] libsql = ["dep:libsql"] diff --git a/crates/devolutions-pedm/build.rs b/crates/devolutions-pedm/build.rs new file mode 100644 index 000000000..32abd9420 --- /dev/null +++ b/crates/devolutions-pedm/build.rs @@ -0,0 +1,19 @@ +use std::path::Path; +use std::{env, fs}; + +fn main() -> Result<(), Box> { + let manifest_dir = env::var("CARGO_MANIFEST_DIR")?; + // This expects the PEDM crate to be in repo_root/crates/devolutions-pedm. + let repo_root = Path::new(&manifest_dir) + .parent() + .expect("crates dir missing") + .parent() + .expect("repo root missing"); + + let version = fs::read_to_string(repo_root.join("VERSION")) + .expect("failed to read VERSION file") + .trim_end() + .to_string(); + println!("cargo:rustc-env=VERSION={}", version); + Ok(()) +} diff --git a/crates/devolutions-pedm/schema/libsql.sql b/crates/devolutions-pedm/schema/libsql.sql index 739d00881..bebe47b21 100644 --- a/crates/devolutions-pedm/schema/libsql.sql +++ b/crates/devolutions-pedm/schema/libsql.sql @@ -1,12 +1,15 @@ /* In SQLite, we store time as an 8-byte integer (i64) with microsecond precision. This matches TIMESTAMPTZ in Postgres. Use `chrono::DateTime::timestamp_micros` when inserting or fetching timestamps in Rust. + + `valid_from` and `valid_to` are used in place of a temporal interval type. + Since the special infinity value does not exist in SQLite, we use NULL. This allows for easy checking of a row validity. A row is presently valid if `valid_to` is NULL. */ CREATE TABLE IF NOT EXISTS version ( version integer PRIMARY KEY, updated_at integer NOT NULL DEFAULT ( - CAST(strftime('%s', 'now') AS integer) * 1000000 + CAST(strftime('%f', 'now') * 1000000 AS integer) % 1000000 + cast(strftime('%s', 'now') AS integer) * 1000000 + cast(strftime('%f', 'now') * 1000000 AS integer) % 1000000 ) ); @@ -14,7 +17,7 @@ CREATE TABLE IF NOT EXISTS run ( id integer PRIMARY KEY, start_time integer NOT NULL DEFAULT ( - CAST(strftime('%s', 'now') AS integer) * 1000000 + CAST(strftime('%f', 'now') * 1000000 AS integer) % 1000000 + cast(strftime('%s', 'now') AS integer) * 1000000 + cast(strftime('%f', 'now') * 1000000 AS integer) % 1000000 ), pipe_name text NOT NULL ); @@ -23,7 +26,7 @@ CREATE TABLE IF NOT EXISTS http_request ( id integer PRIMARY KEY, at integer NOT NULL DEFAULT ( - CAST(strftime('%s', 'now') AS integer) * 1000000 + CAST(strftime('%f', 'now') * 1000000 AS integer) % 1000000 + cast(strftime('%s', 'now') AS integer) * 1000000 + cast(strftime('%f', 'now') * 1000000 AS integer) % 1000000 ), method text NOT NULL, path text NOT NULL, @@ -69,4 +72,71 @@ CREATE TABLE IF NOT EXISTS jit_elevation_result FOREIGN KEY (target_user_id) REFERENCES user(id) ); +CREATE TABLE IF NOT EXISTS account_diff_request +( + id integer PRIMARY KEY, + at integer NOT NULL DEFAULT ( + cast(strftime('%s', 'now') AS integer) * 1000000 + + cast(strftime('%f', 'now') * 1000000 AS integer) % 1000000 + ) +); + +CREATE TABLE IF NOT EXISTS domain +( + id integer PRIMARY KEY AUTOINCREMENT, + subauth1 integer NOT NULL, + subauth2 integer NOT NULL, + subauth3 integer NOT NULL, + subauth4 integer NOT NULL, + CONSTRAINT unique_domain UNIQUE (subauth1, subauth2, subauth3, subauth4) +); + +CREATE TABLE IF NOT EXISTS sid +( + id integer PRIMARY KEY AUTOINCREMENT, + domain_id integer NOT NULL REFERENCES domain (id), + relative_id integer NOT NULL, + CONSTRAINT unique_sid UNIQUE (domain_id, relative_id) +); + +CREATE TABLE IF NOT EXISTS account +( + id integer PRIMARY KEY +); + +CREATE TABLE IF NOT EXISTS account_name +( + id integer NOT NULL REFERENCES account (id), + name text NOT NULL, + valid_from integer NOT NULL DEFAULT ( + cast(strftime('%s', 'now') AS integer) * 1000000 + + cast(strftime('%f', 'now') * 1000000 AS integer) % 1000000 + ), + valid_to integer DEFAULT NULL, + PRIMARY KEY (id, valid_from) +); + +CREATE TABLE IF NOT EXISTS account_removed +( + id integer NOT NULL REFERENCES account (id), + valid_from integer NOT NULL DEFAULT ( + cast(strftime('%s', 'now') AS integer) * 1000000 + + cast(strftime('%f', 'now') * 1000000 AS integer) % 1000000 + ), + valid_to integer DEFAULT NULL, + PRIMARY KEY (id, valid_from) +); + +CREATE TABLE IF NOT EXISTS account_sid +( + account_id integer NOT NULL REFERENCES account (id), + sid_id integer NOT NULL REFERENCES sid (id), + valid_from integer NOT NULL DEFAULT ( + cast(strftime('%s', 'now') AS integer) * 1000000 + + cast(strftime('%f', 'now') * 1000000 AS integer) % 1000000 + ), + valid_to integer DEFAULT NULL, + PRIMARY KEY (account_id, sid_id, valid_from) +); + INSERT INTO version (version) VALUES (0) ON CONFLICT DO NOTHING; \ No newline at end of file diff --git a/crates/devolutions-pedm/schema/pg.sql b/crates/devolutions-pedm/schema/pg.sql index 393df9ca4..dcc228eff 100644 --- a/crates/devolutions-pedm/schema/pg.sql +++ b/crates/devolutions-pedm/schema/pg.sql @@ -1,3 +1,5 @@ +CREATE EXTENSION IF NOT EXISTS btree_gist; + CREATE TABLE IF NOT EXISTS version ( version smallint PRIMARY KEY, @@ -28,4 +30,59 @@ CREATE TABLE IF NOT EXISTS elevate_tmp_request seconds int NOT NULL ); +CREATE TABLE IF NOT EXISTS account_diff_request +( + id int PRIMARY KEY GENERATED ALWAYS AS IDENTITY, + at timestamptz NOT NULL DEFAULT now() +); + +CREATE TABLE IF NOT EXISTS domain +( + id smallint PRIMARY KEY GENERATED ALWAYS AS IDENTITY, + subauth1 smallint NOT NULL, + subauth2 bigint NOT NULL, + subauth3 bigint NOT NULL, + subauth4 bigint NOT NULL, + CONSTRAINT unique_domain UNIQUE (subauth1, subauth2, subauth3, subauth4) +); + +CREATE TABLE IF NOT EXISTS sid +( + id smallint PRIMARY KEY GENERATED ALWAYS AS IDENTITY, + domain_id smallint NOT NULL REFERENCES domain (id), + relative_id smallint NOT NULL, + CONSTRAINT unique_sid UNIQUE (domain_id, relative_id) +); + +CREATE TABLE IF NOT EXISTS account +( + id smallint PRIMARY KEY GENERATED ALWAYS AS IDENTITY +); + +CREATE TABLE IF NOT EXISTS account_name +( + id smallint NOT NULL REFERENCES account (id), + name text NOT NULL, + during tstzrange NOT NULL DEFAULT tstzrange(now(), 'infinity'), + PRIMARY KEY (id, during), + EXCLUDE USING gist (id WITH =, during WITH &&) +); + +CREATE TABLE IF NOT EXISTS account_removed +( + id smallint NOT NULL REFERENCES account (id), + during tstzrange NOT NULL DEFAULT tstzrange(now(), 'infinity'), + PRIMARY KEY (id, during), + EXCLUDE USING gist (id WITH =, during WITH &&) +); + +CREATE TABLE IF NOT EXISTS account_sid +( + account_id smallint NOT NULL REFERENCES account (id), + sid_id smallint NOT NULL REFERENCES sid (id), + during tstzrange NOT NULL DEFAULT tstzrange(now(), 'infinity'), + PRIMARY KEY (account_id, sid_id, during), + EXCLUDE USING gist (account_id WITH =, sid_id WITH =, during WITH &&) +); + INSERT INTO version (version) VALUES (0) ON CONFLICT DO NOTHING; \ No newline at end of file diff --git a/crates/devolutions-pedm/src/account.rs b/crates/devolutions-pedm/src/account.rs new file mode 100644 index 000000000..cf1c2fc6a --- /dev/null +++ b/crates/devolutions-pedm/src/account.rs @@ -0,0 +1,1006 @@ +use core::{error, fmt}; +use std::collections::{HashMap, HashSet}; +use std::num::ParseIntError; +use std::str::FromStr; +use std::string::FromUtf16Error; + +/// A domain for accounts. +/// +/// Since we are dealing with regular accounts and domain accounts, the identifier authority is `5 - NT AUTHORITY`. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub(crate) struct DomainId { + /// The first subauthority. + /// + /// This is part of [well-known SID structures](https://learn.microsoft.com/en-ca/openspecs/windows_protocols/ms-dtyp/81d92bba-d22b-4a8c-908a-554ab29148ab). + /// + /// For example, `21` is `SECURITY_NT_NON_UNIQUE`. + pub(crate) subauth1: u8, + /// The second subauthority, a 32-bit random number. + pub(crate) subauth2: u32, + /// The third subauthority, a 32-bit random number. + pub(crate) subauth3: u32, + /// The fourth subauthority, a 32-bit random number. + pub(crate) subauth4: u32, +} + +impl FromStr for DomainId { + type Err = ParseDomainIdError; + + fn from_str(s: &str) -> Result { + let parts: Vec<&str> = s.split('-').collect(); + if parts.len() != 4 { + return Err(ParseDomainIdError::InvalidFormat); + } + + let map_e = |e, field, value| ParseDomainIdError::ParseInt { + source: e, + field, + value, + }; + Ok(Self { + subauth1: parts[0] + .parse() + .map_err(|e| map_e(e, "subauth1", parts[0].to_owned()))?, + subauth2: parts[1] + .parse() + .map_err(|e| map_e(e, "subauth2", parts[1].to_owned()))?, + subauth3: parts[2] + .parse() + .map_err(|e| map_e(e, "subauth3", parts[2].to_owned()))?, + subauth4: parts[3] + .parse() + .map_err(|e| map_e(e, "subauth4", parts[3].to_owned()))?, + }) + } +} + +impl fmt::Display for DomainId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "{}-{}-{}-{}", + self.subauth1, self.subauth2, self.subauth3, self.subauth4 + ) + } +} + +#[derive(Debug)] +pub enum ParseDomainIdError { + InvalidFormat, + ParseInt { + source: ParseIntError, + field: &'static str, + value: String, + }, +} + +impl error::Error for ParseDomainIdError { + fn source(&self) -> Option<&(dyn error::Error + 'static)> { + match self { + Self::InvalidFormat => None, + Self::ParseInt { source, .. } => Some(source), + } + } +} + +impl fmt::Display for ParseDomainIdError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::InvalidFormat => write!(f, "invalid format'"), + Self::ParseInt { source, field, value } => { + write!(f, "failed to parse field {} with value {}: {}", field, value, source) + } + } + } +} + +/// A security identifier. +/// +/// The SID structure is described in the [Microsoft docs](https://learn.microsoft.com/en-ca/windows-server/identity/ad-ds/manage/understand-security-identifiers). +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub(crate) struct Sid { + pub(crate) domain_id: DomainId, + /// The relative ID, or RID, indicates a unique object ID within the domain. + /// + /// The max is 15000 according to the [Microsoft docs](https://learn.microsoft.com/en-ca/windows-server/identity/ad-ds/manage/managing-rid-issuance). + pub(crate) relative_id: i16, +} + +impl FromStr for Sid { + type Err = ParseSidError; + + fn from_str(s: &str) -> Result { + let Some(s) = s.strip_prefix("S-1-5-") else { + return Err(ParseSidError::MissingPrefix); + }; + let Some((domain_id, rid)) = s.rsplit_once('-') else { + return Err(ParseSidError::InvalidFormat); + }; + let domain_id = DomainId::from_str(domain_id)?; + let rid = rid.parse().map_err(|e| ParseSidError::ParseInt { + source: e, + field: "relative_id", + value: rid.to_owned(), + })?; + Ok(Self { + domain_id, + relative_id: rid, + }) + } +} + +impl fmt::Display for Sid { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "S-1-5-{}-{}", self.domain_id, self.relative_id) + } +} + +/// Error type for parsing a SID or domain ID. +#[derive(Debug)] +pub enum ParseSidError { + InvalidFormat, + MissingPrefix, + ParseInt { + source: ParseIntError, + field: &'static str, + value: String, + }, + DomainId(ParseDomainIdError), +} + +impl error::Error for ParseSidError { + fn source(&self) -> Option<&(dyn error::Error + 'static)> { + match self { + Self::InvalidFormat | Self::MissingPrefix => None, + Self::ParseInt { source, .. } => Some(source), + Self::DomainId(e) => Some(e), + } + } +} + +impl fmt::Display for ParseSidError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::InvalidFormat => write!(f, "invalid format'"), + Self::MissingPrefix => write!(f, "missing prefix"), + Self::ParseInt { source, field, value } => { + write!(f, "failed to parse field {} with value {}: {}", field, value, source) + } + Self::DomainId(e) => e.fmt(f), + } + } +} + +impl From for ParseSidError { + fn from(e: ParseDomainIdError) -> Self { + Self::DomainId(e) + } +} + +/// A user account. +/// +/// This contains the information that is retrieved from WinAPI. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct Account { + pub(crate) name: String, + pub(crate) sid: Sid, +} + +/// A user account with a unique ID. +/// +/// This is the type to use when referring to accounts. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct AccountWithId { + /// The numeric ID for the account ID that was generated by the database. + pub(crate) id: i16, + pub(crate) name: String, + /// The numeric ID for the domain that was generated by the database. + pub(crate) internal_domain_id: i16, + pub(crate) sid: Sid, +} + +impl PartialEq for AccountWithId { + fn eq(&self, other: &Account) -> bool { + self.name == other.name && self.sid == other.sid + } +} + +/// Get the list of usernames and corresponding SIDs. +/// +/// `LookupAccountNameW` must be called to enable `ConvertSidToStringSidW` to work. +#[cfg(target_os = "windows")] +#[allow(clippy::multiple_unsafe_ops_per_block)] +#[allow(clippy::cast_possible_truncation)] +pub(crate) unsafe fn list_accounts() -> Result, ListAccountsError> { + use windows::core::PWSTR; + use windows::Win32::NetworkManagement::NetManagement::{ + NERR_Success, NetApiBufferFree, NetUserEnum, FILTER_NORMAL_ACCOUNT, MAX_PREFERRED_LENGTH, USER_INFO_0, + }; + use windows::Win32::Security::Authorization::ConvertSidToStringSidW; + use windows::Win32::Security::{LookupAccountNameW, PSID, SECURITY_MAX_SID_SIZE, SID_NAME_USE}; + + // SAFETY: uses `NetUserEnum` and `LookupAccountNameW` from `windows` + unsafe { + let mut buf: *mut u8 = std::ptr::null_mut(); + let mut entries_read = 0; + let mut total_entries = 0; + + // Get the list of user accounts. + let status = NetUserEnum( + None, + 0, + FILTER_NORMAL_ACCOUNT, + &mut buf, + MAX_PREFERRED_LENGTH, + &mut entries_read, + &mut total_entries, + None, + ); + + if status != NERR_Success { + return Err(ListAccountsError::NetUserEnumFail(status)); + } + + #[allow(clippy::cast_ptr_alignment)] + let users = std::slice::from_raw_parts(buf as *const USER_INFO_0, entries_read as usize); + + let mut accounts = Vec::with_capacity(users.len()); + for user in users { + let username = user.usri0_name.display(); + let mut sid = [0u8; SECURITY_MAX_SID_SIZE as usize]; + let mut sid_size = sid.len() as u32; + let mut domain_name = [0u16; 256]; + let mut domain_size = domain_name.len() as u32; + let mut sid_type = SID_NAME_USE(0); + let sid = PSID(sid.as_mut_ptr().cast()); + + LookupAccountNameW( + None, + user.usri0_name, + Some(sid), + &mut sid_size, + Some(PWSTR(domain_name.as_mut_ptr())), + &mut domain_size, + &mut sid_type, + )?; + + let mut sid_str: PWSTR = PWSTR::null(); + ConvertSidToStringSidW(sid, &mut sid_str)?; + + // convert to string + let s = sid_str.to_string()?; + accounts.push(Account { + name: username.to_string(), + sid: Sid::from_str(&s)?, + }) + } + NetApiBufferFree(Some(buf as *mut _)); + Ok(accounts) + } +} + +/// A diff of changes between two lists of accounts. +#[derive(Debug, PartialEq, Eq, Default)] +pub(crate) struct AccountsDiff { + /// An account in this list has a new name and a new SID. + pub(crate) added: Vec, + /// A list of account IDs that have been removed. + pub(crate) removed: Vec, + /// A list of accounts with changed names. + /// + /// The elements are the account ID and the new name. + pub(crate) changed_name: Vec<(i16, String)>, + /// A list of accounts that are either new, or have new SIDs. + /// + /// These accounts have names that were previously known, but the SIDs have changed. + /// The elements are the account ID of the previous account with that name and the new account details. + /// + /// We treat accounts with changed SID as a new account for the purposes of PEDM policy. The anomaly can be traced in the logs by querying the removal time of a previous account with the shared name and the add time of the new account. This maybe relevant for [Active Directory migration](https://learn.microsoft.com/en-ca/previous-versions/windows/it-pro/windows-server-2008-R2-and-2008/cc974384(v=ws.10)?redirectedfrom=MSDN). + pub(crate) added_or_changed_sid: Vec<(i16, Account)>, + /// Domain ID mappings that are known. + /// + /// This is taken from the list of accounts retrieved in the database. + /// It is useful to determine if a domain ID needs to be added to the database when adding a new account. + pub(crate) known_domain_ids: HashMap, +} + +impl AccountsDiff { + pub(crate) fn is_empty(&self) -> bool { + self.added.is_empty() + && self.removed.is_empty() + && self.changed_name.is_empty() + && self.added_or_changed_sid.is_empty() + } + + /// Returns the combined list of added accounts and accounts that are either added or have changed SID. + pub(crate) fn added_all(&self) -> Vec<&Account> { + let mut v = self.added.iter().collect::>(); + let added_or_changed = self.added_or_changed_sid.iter().map(|(_, a)| a).collect::>(); + v.extend(added_or_changed.iter().copied()); + v + } + + pub(crate) fn potentially_new_domains(&self) -> Vec { + let mut ids = HashSet::new(); + for a in self.added_all() { + if !self.known_domain_ids.contains_key(&a.sid.domain_id) { + ids.insert(a.sid.domain_id.clone()); + } + } + ids.into_iter().collect() + } +} + +/// Compares two lists of accounts and returns the diff. +/// +/// `old` and `new` are both is sorted by name. +/// `new` is sorted by name because it is how `NetUserEnum` returns the list. +/// We choose to sort `old` by name when retrieving from the database to match the order. +pub(crate) fn diff_accounts(old: &[AccountWithId], new: &[Account]) -> AccountsDiff { + let mut added = Vec::new(); + let mut changed_name = Vec::new(); + let mut added_or_changed_sid = Vec::new(); + + // Use SID as the key. + let old_map = old.iter().map(|a| (&a.sid, a)).collect::>(); + + let mut matched = HashSet::new(); + let old_names = old.iter().map(|a| (a.name.as_str(), a.id)).collect::>(); + for i in new { + // Check for a match by SID. + if let Some(j) = old_map.get(&i.sid) { + // Match found. Check for name change. + if j.name != i.name { + changed_name.push((j.id, i.name.clone())); + } + matched.insert(j.id); + } else { + // Check if the name is an old name. + if let Ok(pos) = old_names.binary_search_by_key(&i.name.as_str(), |&(n, _)| n) { + let old_id = old_names[pos].1; + added_or_changed_sid.push((old_id, i.clone())); + matched.insert(old_id); + } else { + added.push(i.clone()); + } + } + } + + // Unmatched accounts are removed. + let mut removed = Vec::new(); + for i in old { + if !matched.contains(&i.id) { + removed.push(i.id); + } + } + + AccountsDiff { + added, + removed, + changed_name, + added_or_changed_sid, + known_domain_ids: old.iter().fold(HashMap::new(), |mut acc, a| { + acc.insert(a.sid.domain_id.clone(), a.internal_domain_id); + acc + }), + } +} + +#[derive(Debug)] +pub enum ListAccountsError { + FromUtf16(FromUtf16Error), + ParseSid(ParseSidError), + #[cfg(target_os = "windows")] + Windows(windows_result::Error), + /// Contains `nStatus`. + NetUserEnumFail(u32), +} + +impl error::Error for ListAccountsError { + fn source(&self) -> Option<&(dyn error::Error + 'static)> { + match self { + Self::FromUtf16(e) => Some(e), + Self::ParseSid(e) => Some(e), + #[cfg(target_os = "windows")] + Self::Windows(e) => Some(e), + Self::NetUserEnumFail(_) => None, + } + } +} + +impl fmt::Display for ListAccountsError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::FromUtf16(e) => e.fmt(f), + Self::ParseSid(e) => e.fmt(f), + #[cfg(target_os = "windows")] + Self::Windows(e) => e.fmt(f), + Self::NetUserEnumFail(n) => { + write!(f, "NetUserEnum failed with nStatus: {n}") + } + } + } +} + +impl From for ListAccountsError { + fn from(e: FromUtf16Error) -> Self { + Self::FromUtf16(e) + } +} +impl From for ListAccountsError { + fn from(e: ParseSidError) -> Self { + Self::ParseSid(e) + } +} +#[cfg(target_os = "windows")] +impl From for ListAccountsError { + fn from(e: windows_result::Error) -> Self { + Self::Windows(e) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_domain_id() { + assert_eq!( + DomainId::from_str("21-1-2-3").unwrap(), + DomainId { + subauth1: 21, + subauth2: 1, + subauth3: 2, + subauth4: 3, + } + ); + } + + #[test] + fn parse_sid() { + assert_eq!( + Sid::from_str("S-1-5-21-1-2-3-500").unwrap(), + Sid { + domain_id: DomainId { + subauth1: 21, + subauth2: 1, + subauth3: 2, + subauth4: 3, + }, + relative_id: 500, + } + ); + } + + #[test] + fn domain_id_to_string() { + assert_eq!( + DomainId { + subauth1: 21, + subauth2: 1, + subauth3: 2, + subauth4: 3, + } + .to_string(), + "21-1-2-3".to_owned() + ); + } + + #[test] + fn sid_to_string() { + assert_eq!( + Sid { + domain_id: DomainId { + subauth1: 21, + subauth2: 1, + subauth3: 2, + subauth4: 3, + }, + relative_id: 500, + } + .to_string(), + "S-1-5-21-1-2-3-500".to_owned() + ); + } + + #[test] + fn diff_accounts_no_change_one() { + let diff = diff_accounts( + &[AccountWithId { + id: 1, + name: "A".into(), + internal_domain_id: 1, + sid: Sid::from_str("S-1-5-21-1-2-3-1").unwrap(), + }], + &[Account { + name: "A".into(), + sid: Sid::from_str("S-1-5-21-1-2-3-1").unwrap(), + }], + ); + assert!(diff.is_empty()); + } + + #[test] + fn diff_accounts_no_change_two() { + let diff = diff_accounts( + &[ + AccountWithId { + id: 1, + name: "A".into(), + internal_domain_id: 1, + sid: Sid::from_str("S-1-5-21-1-2-3-1").unwrap(), + }, + AccountWithId { + id: 2, + name: "B".into(), + internal_domain_id: 1, + sid: Sid::from_str("S-1-5-21-1-2-3-2").unwrap(), + }, + ], + &[ + Account { + name: "A".into(), + sid: Sid::from_str("S-1-5-21-1-2-3-1").unwrap(), + }, + Account { + name: "B".into(), + sid: Sid::from_str("S-1-5-21-1-2-3-2").unwrap(), + }, + ], + ); + assert!(diff.is_empty()); + } + + #[test] + fn diff_accounts_add_one() { + let diff = diff_accounts( + &[], + &[Account { + name: "A".into(), + sid: Sid::from_str("S-1-5-21-1-2-3-1").unwrap(), + }], + ); + assert_eq!( + diff.added, + vec![Account { + name: "A".into(), + sid: Sid::from_str("S-1-5-21-1-2-3-1").unwrap(), + }] + ); + assert!(diff.removed.is_empty()); + assert!(diff.changed_name.is_empty()); + assert!(diff.added_or_changed_sid.is_empty()); + assert!(diff.known_domain_ids.is_empty()); + } + + #[test] + fn diff_accounts_add_two() { + let diff = diff_accounts( + &[], + &[ + Account { + name: "A".into(), + sid: Sid::from_str("S-1-5-21-1-2-3-1").unwrap(), + }, + Account { + name: "B".into(), + sid: Sid::from_str("S-1-5-21-1-2-3-2").unwrap(), + }, + ], + ); + assert_eq!( + diff.added, + vec![ + Account { + name: "A".into(), + sid: Sid::from_str("S-1-5-21-1-2-3-1").unwrap(), + }, + Account { + name: "B".into(), + sid: Sid::from_str("S-1-5-21-1-2-3-2").unwrap(), + }, + ] + ); + assert!(diff.removed.is_empty()); + assert!(diff.changed_name.is_empty()); + assert!(diff.added_or_changed_sid.is_empty()); + assert!(diff.known_domain_ids.is_empty()); + } + + #[test] + fn diff_accounts_remove_one() { + let diff = diff_accounts( + &[AccountWithId { + id: 1, + name: "Foo".into(), + internal_domain_id: 1, + sid: Sid::from_str("S-1-5-21-1-2-3-1").unwrap(), + }], + &[], + ); + assert!(diff.added.is_empty()); + assert_eq!(diff.removed, vec![1]); + assert!(diff.changed_name.is_empty()); + assert!(diff.added_or_changed_sid.is_empty()); + assert_eq!( + diff.known_domain_ids, + HashMap::from([( + DomainId { + subauth1: 21, + subauth2: 1, + subauth3: 2, + subauth4: 3, + }, + 1 + )]) + ); + } + + #[test] + fn diff_accounts_remove_two() { + let diff = diff_accounts( + &[ + AccountWithId { + id: 1, + name: "A".into(), + internal_domain_id: 1, + sid: Sid::from_str("S-1-5-21-1-2-3-1").unwrap(), + }, + AccountWithId { + id: 2, + name: "B".into(), + internal_domain_id: 1, + sid: Sid::from_str("S-1-5-21-1-2-3-2").unwrap(), + }, + ], + &[], + ); + assert!(diff.added.is_empty()); + assert_eq!(diff.removed, vec![1, 2]); + assert!(diff.changed_name.is_empty()); + assert!(diff.added_or_changed_sid.is_empty()); + assert_eq!( + diff.known_domain_ids, + HashMap::from([( + DomainId { + subauth1: 21, + subauth2: 1, + subauth3: 2, + subauth4: 3, + }, + 1 + )]) + ); + } + + #[test] + fn diff_accounts_changed_name_one() { + let diff = diff_accounts( + &[AccountWithId { + id: 1, + name: "A".into(), + internal_domain_id: 1, + sid: Sid::from_str("S-1-5-21-1-2-3-1").unwrap(), + }], + &[Account { + name: "AA".into(), + sid: Sid::from_str("S-1-5-21-1-2-3-1").unwrap(), + }], + ); + + assert!(diff.added.is_empty()); + assert!(diff.removed.is_empty()); + assert_eq!(diff.changed_name, vec![(1, "AA".into())]); + assert!(diff.added_or_changed_sid.is_empty()); + assert_eq!( + diff.known_domain_ids, + HashMap::from([( + DomainId { + subauth1: 21, + subauth2: 1, + subauth3: 2, + subauth4: 3, + }, + 1 + )]) + ); + } + + #[test] + fn diff_accounts_changed_name_two() { + let diff = diff_accounts( + &[ + AccountWithId { + id: 1, + name: "A".into(), + internal_domain_id: 1, + sid: Sid::from_str("S-1-5-21-1-2-3-1").unwrap(), + }, + AccountWithId { + id: 2, + name: "B".into(), + internal_domain_id: 2, + sid: Sid::from_str("S-1-5-21-7-8-9-2").unwrap(), + }, + ], + &[ + Account { + name: "AA".into(), + sid: Sid::from_str("S-1-5-21-1-2-3-1").unwrap(), + }, + Account { + name: "BB".into(), + sid: Sid::from_str("S-1-5-21-7-8-9-2").unwrap(), + }, + ], + ); + + assert!(diff.added.is_empty()); + assert!(diff.removed.is_empty()); + assert_eq!(diff.changed_name, vec![(1, "AA".into()), (2, "BB".into())]); + assert!(diff.added_or_changed_sid.is_empty()); + assert_eq!( + diff.known_domain_ids, + HashMap::from([ + ( + DomainId { + subauth1: 21, + subauth2: 1, + subauth3: 2, + subauth4: 3, + }, + 1 + ), + ( + DomainId { + subauth1: 21, + subauth2: 7, + subauth3: 8, + subauth4: 9, + }, + 2 + ) + ]) + ); + } + + #[test] + fn test_added_or_changed_sid_one() { + let diff = diff_accounts( + &[AccountWithId { + id: 1, + name: "A".into(), + internal_domain_id: 1, + sid: Sid::from_str("S-1-5-21-1-2-3-1").unwrap(), + }], + &[Account { + name: "A".into(), + sid: Sid::from_str("S-1-5-21-7-8-9-11").unwrap(), + }], + ); + + assert!(diff.added.is_empty()); + assert!(diff.removed.is_empty()); + assert!(diff.changed_name.is_empty()); + assert_eq!( + diff.added_or_changed_sid, + vec![( + 1, + Account { + name: "A".into(), + sid: Sid::from_str("S-1-5-21-7-8-9-11").unwrap() + } + )] + ); + assert_eq!( + diff.known_domain_ids, + HashMap::from([( + DomainId { + subauth1: 21, + subauth2: 1, + subauth3: 2, + subauth4: 3, + }, + 1 + ),]) + ); + } + + #[test] + fn diff_accounts_added_or_changed_sid_two() { + let diff = diff_accounts( + &[ + AccountWithId { + id: 1, + name: "A".into(), + internal_domain_id: 1, + sid: Sid::from_str("S-1-5-21-1-2-3-1").unwrap(), + }, + AccountWithId { + id: 2, + name: "B".into(), + internal_domain_id: 1, + sid: Sid::from_str("S-1-5-21-1-2-3-2").unwrap(), + }, + ], + &[ + Account { + name: "A".into(), + sid: Sid::from_str("S-1-5-21-7-8-9-11").unwrap(), + }, + Account { + name: "B".into(), + sid: Sid::from_str("S-1-5-21-7-8-9-22").unwrap(), + }, + ], + ); + + assert!(diff.added.is_empty()); + assert!(diff.removed.is_empty()); + assert!(diff.changed_name.is_empty()); + assert_eq!( + diff.added_or_changed_sid, + vec![ + ( + 1, + Account { + name: "A".into(), + sid: Sid::from_str("S-1-5-21-7-8-9-11").unwrap() + } + ), + ( + 2, + Account { + name: "B".into(), + sid: Sid::from_str("S-1-5-21-7-8-9-22").unwrap() + } + ) + ] + ); + assert_eq!( + diff.known_domain_ids, + HashMap::from([( + DomainId { + subauth1: 21, + subauth2: 1, + subauth3: 2, + subauth4: 3, + }, + 1 + )]) + ); + } + + #[test] + fn diff_accounts_full() { + let diff = diff_accounts( + &[ + // A will not change. + AccountWithId { + id: 1, + name: "A".into(), + internal_domain_id: 1, + sid: Sid::from_str("S-1-5-21-1-2-3-1").unwrap(), + }, + // B will be removed. + AccountWithId { + id: 2, + name: "B".into(), + internal_domain_id: 1, + sid: Sid::from_str("S-1-5-21-1-2-3-2").unwrap(), + }, + // C will have a name change. + AccountWithId { + id: 3, + name: "C".into(), + internal_domain_id: 1, + sid: Sid::from_str("S-1-5-21-1-2-3-3").unwrap(), + }, + // E will have a new SID. + AccountWithId { + id: 5, + name: "E".into(), + internal_domain_id: 1, + sid: Sid::from_str("S-1-5-21-1-2-3-5").unwrap(), + }, + // F is an account to be deleted, but the system will have a new one created with the same name. + AccountWithId { + id: 6, + name: "F".into(), + internal_domain_id: 1, + sid: Sid::from_str("S-1-5-21-1-2-3-6").unwrap(), + }, + // G is from a different domain. It will not change. + AccountWithId { + id: 7, + name: "G".into(), + internal_domain_id: 2, + sid: Sid::from_str("S-1-5-21-7-8-9-7").unwrap(), + }, + ], + &[ + // A is unchanged. + Account { + name: "A".into(), + sid: Sid::from_str("S-1-5-21-1-2-3-1").unwrap(), + }, + // C has a new name. + Account { + name: "CC".into(), + sid: Sid::from_str("S-1-5-21-1-2-3-3").unwrap(), + }, + // D is a new account. + Account { + name: "D".into(), + sid: Sid::from_str("S-1-5-21-1-2-3-4").unwrap(), + }, + // E has a new SID. + Account { + name: "E".into(), + sid: Sid::from_str("S-1-5-21-7-8-9-55").unwrap(), + }, + // F is a new account with the same name as a previous account. + Account { + name: "F".into(), + sid: Sid::from_str("S-1-5-21-1-2-3-7").unwrap(), + }, + Account { + name: "G".into(), + sid: Sid::from_str("S-1-5-21-7-8-9-7").unwrap(), + }, + ], + ); + + assert_eq!( + diff.added, + vec![Account { + name: "D".into(), + sid: Sid::from_str("S-1-5-21-1-2-3-4").unwrap(), + }] + ); + assert_eq!(diff.removed, vec![2]); + assert_eq!(diff.changed_name, vec![(3, "CC".into())]); + assert_eq!( + diff.added_or_changed_sid, + vec![ + ( + 5, + Account { + name: "E".into(), + sid: Sid::from_str("S-1-5-21-7-8-9-55").unwrap() + } + ), + ( + 6, + Account { + name: "F".into(), + sid: Sid::from_str("S-1-5-21-1-2-3-7").unwrap() + } + ), + ] + ); + assert_eq!( + diff.known_domain_ids, + HashMap::from([ + ( + DomainId { + subauth1: 21, + subauth2: 1, + subauth3: 2, + subauth4: 3, + }, + 1 + ), + ( + DomainId { + subauth1: 21, + subauth2: 7, + subauth3: 8, + subauth4: 9, + }, + 2 + ) + ]) + ); + } +} diff --git a/crates/devolutions-pedm/src/api/account.rs b/crates/devolutions-pedm/src/api/account.rs new file mode 100644 index 000000000..2da59e531 --- /dev/null +++ b/crates/devolutions-pedm/src/api/account.rs @@ -0,0 +1,34 @@ +use aide::NoApi; +use axum::Json; +use schemars::JsonSchema; +use serde::Serialize; + +use crate::account::AccountWithId; +use crate::db::Db; + +use super::err::HandlerError; + +/// An account with its ID, name, and SID. +#[derive(Serialize, JsonSchema)] +pub(crate) struct AccountData { + pub(crate) id: i16, + pub(crate) name: String, + pub(crate) sid: String, +} + +impl From for AccountData { + fn from(account: AccountWithId) -> Self { + Self { + id: account.id, + name: account.name, + sid: account.sid.to_string(), + } + } +} + +/// Gets accounts on the system. +/// +/// Includes info like the account name and SID. +pub(crate) async fn get_accounts(NoApi(Db(db)): NoApi) -> Result>, HandlerError> { + Ok(Json(db.get_accounts().await?.into_iter().map(Into::into).collect())) +} diff --git a/crates/devolutions-pedm/src/api/launch.rs b/crates/devolutions-pedm/src/api/launch.rs index 25ac6ea82..a269ddb06 100644 --- a/crates/devolutions-pedm/src/api/launch.rs +++ b/crates/devolutions-pedm/src/api/launch.rs @@ -1,10 +1,8 @@ use std::path::{Path, PathBuf}; -use std::sync::Arc; use aide::NoApi; use axum::extract::State; use axum::{Extension, Json}; -use parking_lot::RwLock; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use tracing::info; @@ -20,10 +18,8 @@ use win_api_wrappers::token::Token; use win_api_wrappers::utils::{environment_block, expand_environment_path, CommandLine, WideString}; use crate::api::state::AppState; -use crate::db::DbHandle; use crate::elevator; use crate::error::Error; -use crate::policy::Policy; use super::NamedPipeConnectInfo; diff --git a/crates/devolutions-pedm/src/api/mod.rs b/crates/devolutions-pedm/src/api/mod.rs index 369950286..c438c820b 100644 --- a/crates/devolutions-pedm/src/api/mod.rs +++ b/crates/devolutions-pedm/src/api/mod.rs @@ -33,12 +33,14 @@ use win_api_wrappers::token::Token; use win_api_wrappers::undoc::PIPE_ACCESS_FULL_CONTROL; use win_api_wrappers::utils::Pipe; +use crate::account::{diff_accounts, list_accounts, ListAccountsError}; use crate::config::Config; use crate::db::{Db, DbAsyncBridgeTask, DbError, InitSchemaError}; use crate::error::{Error, ErrorResponse}; use crate::utils::AccountExt; mod about; +mod account; mod elevate_session; mod elevate_temporary; mod err; @@ -50,6 +52,7 @@ pub(crate) mod state; mod status; use self::about::about; +use self::account::get_accounts; use self::elevate_session::elevate_session; use self::elevate_temporary::elevate_temporary; use self::launch::post_launch; @@ -137,6 +140,7 @@ fn create_pipe(pipe_name: &str) -> anyhow::Result { pub(crate) fn api_router() -> ApiRouter { ApiRouter::new() .api_route("/about", aide::axum::routing::get(about)) + .api_route("/accounts", aide::axum::routing::get(get_accounts)) .api_route("/elevate/temporary", aide::axum::routing::post(elevate_temporary)) .api_route("/elevate/session", aide::axum::routing::post(elevate_session)) .api_route("/launch", aide::axum::routing::post(post_launch)) @@ -176,6 +180,16 @@ pub async fn serve(config: Config, shutdown_signal: ShutdownSignal) -> Result<() let db = Db::new(&config).await?; db.setup().await?; + // Update the list of accounts in the database. + + // SAFETY: uses `NetUserEnum` and `LookupAccountNameW` from `windows` + let accounts = unsafe { list_accounts()? }; + + let db_accounts = db.get_accounts().await?; + info!("Accounts retrieved successfully"); + let diff = diff_accounts(&db_accounts, &accounts); + db.update_accounts(&diff).await?; + let (db_handle, db_async_bridge_task) = DbAsyncBridgeTask::new(db.clone()); let _db_async_bridge_task = devolutions_gateway_task::spawn_task(db_async_bridge_task, shutdown_signal); @@ -233,6 +247,7 @@ pub enum ServeError { AppState(AppStateError), Db(DbError), InitSchema(InitSchemaError), + ListAccounts(ListAccountsError), Other(anyhow::Error), } @@ -243,6 +258,7 @@ impl core::error::Error for ServeError { Self::AppState(e) => Some(e), Self::Db(e) => Some(e), Self::InitSchema(e) => Some(e), + Self::ListAccounts(e) => Some(e), Self::Other(e) => Some(e.as_ref()), } } @@ -255,6 +271,7 @@ impl fmt::Display for ServeError { Self::AppState(e) => e.fmt(f), Self::Db(e) => e.fmt(f), Self::InitSchema(e) => e.fmt(f), + Self::ListAccounts(e) => e.fmt(f), Self::Other(e) => e.fmt(f), } } @@ -280,6 +297,11 @@ impl From for ServeError { Self::InitSchema(e) } } +impl From for ServeError { + fn from(e: ListAccountsError) -> Self { + Self::ListAccounts(e) + } +} impl From for ServeError { fn from(e: anyhow::Error) -> Self { Self::Other(e) diff --git a/crates/devolutions-pedm/src/db/err.rs b/crates/devolutions-pedm/src/db/err.rs index 7400a972d..99b943806 100644 --- a/crates/devolutions-pedm/src/db/err.rs +++ b/crates/devolutions-pedm/src/db/err.rs @@ -153,4 +153,4 @@ impl fmt::Display for InvalidEnumError { } } -impl std::error::Error for InvalidEnumError {} +impl Error for InvalidEnumError {} diff --git a/crates/devolutions-pedm/src/db/libsql.rs b/crates/devolutions-pedm/src/db/libsql.rs index eae9e6f1a..147d7950b 100644 --- a/crates/devolutions-pedm/src/db/libsql.rs +++ b/crates/devolutions-pedm/src/db/libsql.rs @@ -1,14 +1,25 @@ +//! Concrete implementation of the `Database` trait for libSQL. +//! +//! It is important to note that Transaction` is owned and `Transaction::clone` returns a connection. +//! For these reasons, we run database operations serially. + +use std::collections::HashMap; use std::ops::Deref; use async_trait::async_trait; use chrono::{DateTime, Utc}; -use devolutions_pedm_shared::policy::{AuthenticodeSignatureStatus, ElevationResult, Hash, Signature, Signer, User}; +use futures_util::{StreamExt, TryStreamExt}; use libsql::params::IntoParams; use libsql::{params, Row, Transaction, Value}; +use devolutions_pedm_shared::policy::{AuthenticodeSignatureStatus, ElevationResult, Hash, Signature, Signer, User}; +use tracing::info; + +use crate::account::{AccountWithId, AccountsDiff, DomainId, Sid}; use crate::log::{JitElevationLogPage, JitElevationLogQueryOptions, JitElevationLogRow}; use super::err::{InvalidEnumError, ParseTimestampError}; +use super::util::{bulk_insert_statement_generic, query_args_inline_generic, query_args_single_generic}; use super::{Database, DbError}; pub(crate) struct LibsqlConn(libsql::Connection); @@ -140,6 +151,204 @@ impl Database for LibsqlConn { Ok(()) } + async fn get_accounts(&self) -> Result, DbError> { + let statement = "SELECT a.id, + n.name, + d.id, + d.subauth1, + d.subauth2, + d.subauth3, + d.subauth4, + s.relative_id +FROM account a + JOIN account_name n ON a.id = n.id + AND n.valid_to IS NULL + JOIN account_sid sa ON a.id = sa.account_id + AND sa.valid_to IS NULL + JOIN sid s ON sa.sid_id = s.id + JOIN domain d ON s.domain_id = d.id + LEFT JOIN account_removed r ON a.id = r.id + AND r.valid_to IS NULL +WHERE r.id IS NULL +ORDER BY n.name"; + + self.query(statement, ()) + .await? + .into_stream() + .then(|row| async move { + let row = row?; + Ok(AccountWithId { + id: i16::try_from(row.get::(0)?)?, + name: row.get(1)?, + internal_domain_id: i16::try_from(row.get::(2)?)?, + sid: Sid { + domain_id: DomainId { + subauth1: u8::try_from(row.get::(3)?)?, + subauth2: u32::try_from(row.get::(4)?)?, + subauth3: u32::try_from(row.get::(5)?)?, + subauth4: u32::try_from(row.get::(6)?)?, + }, + relative_id: i16::try_from(row.get::(7)?)?, + }, + }) + }) + .try_collect() + .await + } + + async fn update_accounts(&self, diff: &AccountsDiff) -> Result<(), DbError> { + let tx = self.transaction().await?; + tx.execute("INSERT INTO account_diff_request DEFAULT VALUES", ()) + .await?; + + if diff.is_empty() { + tx.commit().await?; + return Ok(()); + } + + // Add new accounts. + if !(diff.added.is_empty() && diff.added_or_changed_sid.is_empty()) { + let accounts = diff.added_all(); + + // Insert SIDs into the `sid` table and get the DB-generated SID IDs. + + let new_domains = diff.potentially_new_domains(); + let mut domain_map = diff.known_domain_ids.clone(); + + // Add potentially new domain IDs to the database and update the map with the DB-generated IDs. + if !new_domains.is_empty() { + let params = new_domains + .iter() + .flat_map(|id| { + [ + id.subauth1.into(), + id.subauth2.into(), + id.subauth3.into(), + id.subauth4.into(), + ] + }) + .collect::>(); + let mut statement = bulk_insert_statement( + "domain", + &["subauth1", "subauth2", "subauth3", "subauth4"], + new_domains.len(), + ); + // The domain ID we are inserting may already exist. If it does exist, retrieve the existing ID. + statement.push_str(" ON CONFLICT (subauth1, subauth2, subauth3, subauth4) DO UPDATE SET subauth1 = EXCLUDED.subauth1 RETURNING id"); + let db_domain_ids = tx + .query(&statement, params) + .await? + .into_stream() + .then(|row| async { + let val: i64 = row?.get(0)?; + Ok::<_, DbError>(i16::try_from(val)?) + }) + .try_collect::>() + .await?; + let new_map = new_domains + .iter() + .cloned() + .zip(db_domain_ids.into_iter()) + .collect::>(); + // Add to the existing map. + domain_map.extend(new_map); + } + + // Insert SIDs. + let params = accounts + .iter() + .flat_map(|a| { + let domain_id = &domain_map[&a.sid.domain_id]; + [(*domain_id), a.sid.relative_id] + }) + .collect::>(); + let mut statement = bulk_insert_statement("sid", &["domain_id", "relative_id"], accounts.len()); + statement.push_str(" RETURNING id"); + + // Return the DB-generated SID IDs. + let rows = tx.query(&statement, params).await?; + let sid_ids = rows + .into_stream() + .then(|row| async { row?.get::(0) }) + .try_collect::>() + .await?; + + // Create accounts by inserting into the `account` table and get the account IDs. + // This is a hacky way to insert multiple rows with default values in one statement. + // Unfortunately, it doesn't support `RETURNING`. + let mut statement = String::from("INSERT INTO account SELECT NULL"); + for _ in 0..accounts.len() { + statement.push_str(" UNION ALL SELECT NULL"); + } + #[allow(clippy::cast_possible_wrap)] + let len = accounts.len() as i64; + tx.execute(&statement, [len]).await?; + + // Get the newly created account IDs. + let account_ids = tx + .query("SELECT id FROM account ORDER BY id DESC LIMIT ?", [len]) + .await? + .into_stream() + .then(|row| async { row?.get::(0) }) + .try_collect::>() + .await?; + + // Insert account name. + let params = account_ids + .iter() + .zip(&accounts) + .flat_map(|(&id, a)| [id.into(), a.name.clone().into()]) + .collect::>(); + let statement = bulk_insert_statement("account_name", &["id", "name"], accounts.len()); + tx.execute(&statement, params).await?; + + // Insert account SID. + let params = account_ids + .iter() + .zip(&sid_ids) + .flat_map(|(account_id, sid)| [(*account_id).into(), (*sid).into()]) + .collect::>(); + let statement = bulk_insert_statement("account_sid", &["account_id", "sid_id"], accounts.len()); + tx.execute(&statement, params).await?; + } + + // Remove accounts. + if !diff.removed.is_empty() { + info!("Accounts to remove"); + let records = &diff.removed; + let params = records.iter().map(|id| Value::from(*id)).collect::>(); + let statement = format!( + "UPDATE account_removed SET valid_to = {NOW} WHERE id IN {} AND valid_to IS NULL", + query_args_inline(records.len()) + ); + tx.execute(&statement, params.clone()).await?; + let statement = format!( + "INSERT INTO account_removed (id) VALUES {}", + query_args_single(records.len()) + ); + tx.execute(&statement, params).await?; + }; + + // Update accounts with changed names. + if !diff.changed_name.is_empty() { + info!("accounts to update"); + let records = &diff.changed_name; + let params = records + .iter() + .flat_map(|(id, name)| [(*id).into(), name.clone().into()]) + .collect::>(); + let statement = format!( + "UPDATE account_name SET valid_to = {NOW} WHERE id IN {} AND valid_to IS NULL", + query_args_single(records.len()) + ); + tx.execute(&statement, params.clone()).await?; + let statement = bulk_insert_statement("account_name", &["id", "name"], records.len()); + tx.execute(&statement, params).await?; + }; + tx.commit().await?; + Ok(()) + } + async fn insert_elevate_tmp_request(&self, req_id: i32, seconds: i32) -> Result<(), DbError> { self.execute( "INSERT INTO elevate_tmp_request (req_id, seconds) VALUES (?1, ?2)", @@ -398,12 +607,36 @@ fn parse_micros(micros: i64) -> Result, ParseTimestampError> { } } +/// Constructs query args like `(?1), (?2), (?3)`. +fn query_args_single(num_records: usize) -> String { + query_args_single_generic(num_records, '?') +} + +/// Constructs n query args like `(?1, ?2, ?3)`. +/// +/// This is useful for `IN`. +fn query_args_inline(num_records: usize) -> String { + query_args_inline_generic(num_records, '?') +} + +/// Constructs an insert statement for bulk inserts. +/// +/// The output is like `INSERT INTO table_name (col1, col2, col3) VALUES (?1, ?2, ?3), (?4, ?5, ?6)`. +fn bulk_insert_statement(table_name: &str, col_names: &[&str], num_records: usize) -> String { + bulk_insert_statement_generic(table_name, col_names, num_records, '?') +} + +/// The current time in microseconds. +// +// We use this because `libsql` does not support creating scalar functions. +const NOW: &str = "(strftime('%s', 'now') * 1000000 + (strftime('%f', 'now') * 1000000) % 1000000)"; + #[cfg(test)] mod tests { use chrono::{TimeZone, Utc}; - use super::parse_micros; use crate::db::err::ParseTimestampError; + use crate::db::libsql::parse_micros; #[test] fn test_valid_micros() { diff --git a/crates/devolutions-pedm/src/db/mod.rs b/crates/devolutions-pedm/src/db/mod.rs index 79fd5891a..bb70d60d3 100644 --- a/crates/devolutions-pedm/src/db/mod.rs +++ b/crates/devolutions-pedm/src/db/mod.rs @@ -1,3 +1,7 @@ +//! This module defines the database trait, a backend-agnostic interface for databse operations. +//! +//! Trait methods are defined here. A suffix of `_tx` indicates that the method is part of a transaction but not committed. + use core::fmt; use std::ops::Deref; use std::sync::Arc; @@ -9,7 +13,9 @@ use devolutions_pedm_shared::policy::{ElevationResult, User}; use tracing::{info, warn}; mod err; +mod util; +use crate::account::{AccountWithId, AccountsDiff}; use crate::config::DbBackend; use crate::log::{JitElevationLogPage, JitElevationLogQueryOptions, JitElevationLogRow}; use crate::Config; @@ -199,6 +205,12 @@ pub(crate) trait Database: Send + Sync { /// This is used in the `LogLayer` middleware. Note that this query will only be executed after the response is sent. async fn log_http_request(&self, req_id: i32, method: &str, path: &str, status_code: i16) -> Result<(), DbError>; + /// Gets accounts from the database, ordered by name. + async fn get_accounts(&self) -> Result, DbError>; + + /// Updates accounts in the database. + async fn update_accounts(&self, diff: &AccountsDiff) -> Result<(), DbError>; + async fn insert_elevate_tmp_request(&self, req_id: i32, seconds: i32) -> Result<(), DbError>; async fn insert_jit_elevation_result(&self, result: &ElevationResult) -> Result<(), DbError>; diff --git a/crates/devolutions-pedm/src/db/pg.rs b/crates/devolutions-pedm/src/db/pg.rs index 28c969599..9c3fe7b7f 100644 --- a/crates/devolutions-pedm/src/db/pg.rs +++ b/crates/devolutions-pedm/src/db/pg.rs @@ -1,16 +1,28 @@ +#![allow(clippy::cast_possible_truncation)] +#![allow(clippy::cast_possible_wrap)] +#![allow(clippy::cast_sign_loss)] + +use std::collections::HashMap; use std::ops::Deref; use async_trait::async_trait; use bb8::Pool; use bb8_postgres::PostgresConnectionManager; use chrono::{DateTime, Utc}; -use devolutions_pedm_shared::policy::ElevationResult; +use futures_util::try_join; +use tokio_postgres::types::ToSql; use tokio_postgres::NoTls; use crate::log::{JitElevationLogPage, JitElevationLogQueryOptions}; +use devolutions_pedm_shared::policy::ElevationResult; + +use crate::account::{AccountWithId, AccountsDiff, DomainId, Sid}; +use super::util::{bulk_insert_statement_generic, query_args_inline_generic, query_args_single_generic}; use super::{Database, DbError}; +type Params<'a> = Vec<&'a (dyn ToSql + Sync)>; + pub(crate) struct PgPool(Pool>); impl PgPool { @@ -91,6 +103,224 @@ impl Database for PgPool { Ok(()) } + async fn get_accounts(&self) -> Result, DbError> { + Ok(self + .get() + .await? + .query( + "SELECT a.id, + n.name, + d.id, + d.subauth1, + d.subauth2, + d.subauth3, + d.subauth4, + s.relative_id +FROM account a + JOIN account_name n ON a.id = n.id + AND n.during @> now() + JOIN account_sid sa + ON a.id = sa.account_id + AND sa.during @> now() + JOIN sid s ON sa.sid_id = s.id + JOIN domain d ON s.domain_id = d.id + LEFT JOIN account_removed r ON a.id = r.id + AND r.during @> now() +WHERE r.id IS NULL +ORDER BY n.name", + &[], + ) + .await? + .into_iter() + .map(|row| AccountWithId { + id: row.get(0), + name: row.get(1), + internal_domain_id: row.get(2), + sid: Sid { + domain_id: DomainId { + subauth1: row.get::<_, i16>(3) as u8, + subauth2: row.get::<_, i64>(4) as u32, + subauth3: row.get::<_, i64>(5) as u32, + subauth4: row.get::<_, i64>(6) as u32, + }, + relative_id: row.get(7), + }, + }) + .collect()) + } + + async fn update_accounts(&self, diff: &AccountsDiff) -> Result<(), DbError> { + let mut conn = self.get().await?; + let tx = conn.transaction().await?; + tx.execute("INSERT INTO account_diff_request DEFAULT VALUES", &[]) + .await?; + + if diff.is_empty() { + tx.commit().await?; + return Ok(()); + } + + let add_fut = async { + if diff.added.is_empty() && diff.added_or_changed_sid.is_empty() { + return Ok::<_, tokio_postgres::Error>(()); + } + let accounts = diff.added_all(); + + // Insert SIDs into the `sid` table and get the DB-generated SID IDs. + let sid_fut = async { + let new_domains = diff.potentially_new_domains(); + let mut domain_map = diff.known_domain_ids.clone(); + + // Add potentially new domain IDs to the database and update the map with the DB-generated IDs. + if !new_domains.is_empty() { + let mut params: Params<'_> = Vec::with_capacity(new_domains.len() * 4); + // Convert some types. + let parts = new_domains + .iter() + .map(|id| { + ( + i16::from(id.subauth1), + i64::from(id.subauth2), + i64::from(id.subauth3), + i64::from(id.subauth4), + ) + }) + .collect::>(); + for (subauth1, subauth2, subauth3, subauth4) in parts.iter().take(new_domains.len()) { + params.push(subauth1); + params.push(subauth2); + params.push(subauth3); + params.push(subauth4); + } + let mut statement = bulk_insert_statement( + "domain", + &["subauth1", "subauth2", "subauth3", "subauth4"], + new_domains.len(), + ); + // The domain ID we are inserting may already exist. If it does exist, retrieve the existing ID. + statement.push_str(" ON CONFLICT (subauth1, subauth2, subauth3, subauth4) DO UPDATE SET subauth1 = EXCLUDED.subauth1 RETURNING id"); + let db_domain_ids: Vec = tx + .query(&statement, ¶ms) + .await? + .into_iter() + .map(|row| row.get(0)) + .collect::>(); + let new_map = new_domains + .iter() + .cloned() + .zip(db_domain_ids.into_iter()) + .collect::>(); + // Add to the existing map. + domain_map.extend(new_map); + }; + + // Insert SIDs. + let mut params: Params<'_> = Vec::with_capacity(accounts.len() * 2); + for a in &accounts { + // look up the domain ID in map + let domain_id = &domain_map[&a.sid.domain_id]; + params.push(domain_id); + params.push(&a.sid.relative_id); + } + let mut statement = bulk_insert_statement("sid", &["domain_id", "relative_id"], accounts.len()); + statement.push_str(" RETURNING id"); + + // Return the DB-generated SID IDs. + Ok(tx + .query(&statement, ¶ms) + .await? + .into_iter() + .map(|row| row.get(0)) + .collect::>()) + }; + + // Create accounts by inserting into the `account` table and get the account IDs. + let account_fut = async { + Ok(tx + .query( + "INSERT INTO account SELECT FROM generate_series(1, $1) RETURNING id", + &[&(accounts.len() as i32)], + ) + .await? + .into_iter() + .map(|row| row.get(0)) + .collect::>()) + }; + let (sid_ids, account_ids) = try_join!(sid_fut, account_fut)?; + + // Insert account name. + let name_fut = async { + let mut params: Params<'_> = Vec::with_capacity(accounts.len() * 2); + for (i, a) in accounts.iter().enumerate() { + params.push(&account_ids[i]); + params.push(&a.name); + } + let statement = bulk_insert_statement("account_name", &["id", "name"], accounts.len()); + tx.execute(&statement, ¶ms).await?; + Ok(()) + }; + // Insert account SID. + let account_sid_fut = async { + let mut params: Params<'_> = Vec::with_capacity(accounts.len() * 2); + for i in 0..accounts.len() { + params.push(&account_ids[i]); + params.push(&sid_ids[i]); + } + let statement = bulk_insert_statement("account_sid", &["account_id", "sid_id"], accounts.len()); + tx.execute(&statement, ¶ms).await?; + Ok(()) + }; + try_join!(name_fut, account_sid_fut)?; + Ok(()) + }; + + let remove_fut = async { + if diff.removed.is_empty() { + return Ok(()); + } + let records = &diff.removed; + let mut params: Params<'_> = Vec::with_capacity(records.len()); + for id in records.iter() { + params.push(id); + } + let statement = format!( + "UPDATE account_removed SET during = tstzrange(lower(during), now()) WHERE id IN {} AND during @> now()", + query_args_inline(records.len()) + ); + tx.execute(&statement, ¶ms).await?; + let statement = format!( + "INSERT INTO account_removed (id) VALUES {}", + query_args_single(records.len()) + ); + tx.execute(&statement, ¶ms).await?; + Ok(()) + }; + let changed_name_fut = async { + if diff.changed_name.is_empty() { + return Ok(()); + } + let records = &diff.changed_name; + let mut update_params: Params<'_> = Vec::with_capacity(records.len()); + let mut insert_params: Params<'_> = Vec::with_capacity(records.len() * 2); + for (id, name) in records.iter() { + update_params.push(id); + insert_params.push(id); + insert_params.push(name); + } + let statement = format!( + "UPDATE account_name SET during = tstzrange(lower(during), now()) WHERE id IN {} AND during @> now()", + query_args_inline(records.len()) + ); + tx.execute(&statement, &update_params).await?; + let statement = bulk_insert_statement("account_name", &["id", "name"], records.len()); + tx.execute(&statement, &insert_params).await?; + Ok(()) + }; + try_join!(add_fut, remove_fut, changed_name_fut).map_err(DbError::from)?; + tx.commit().await?; + Ok(()) + } + async fn insert_elevate_tmp_request(&self, req_id: i32, seconds: i32) -> Result<(), DbError> { self.get() .await? @@ -121,3 +351,22 @@ impl Database for PgPool { unimplemented!() } } + +/// Constructs query args like `($1), ($2), ($3)`. +fn query_args_single(num_records: usize) -> String { + query_args_single_generic(num_records, '$') +} + +/// Constructs n query args like `($1, $2, $3)`. +/// +/// This is useful for `IN`. +fn query_args_inline(num_records: usize) -> String { + query_args_inline_generic(num_records, '$') +} + +/// Constructs an insert statement for bulk inserts. +/// +/// The output is like `INSERT INTO table_name (col1, col2, col3) VALUES ($1, $2, $3), ($4, $5, $6)`. +fn bulk_insert_statement(table_name: &str, col_names: &[&str], num_records: usize) -> String { + bulk_insert_statement_generic(table_name, col_names, num_records, '$') +} diff --git a/crates/devolutions-pedm/src/db/util.rs b/crates/devolutions-pedm/src/db/util.rs new file mode 100644 index 000000000..779329b6c --- /dev/null +++ b/crates/devolutions-pedm/src/db/util.rs @@ -0,0 +1,92 @@ +use std::fmt::Write; + +/// Constructs query args like `($1, $2), ($3, $4), ($5, $6)`. +fn query_args_generic(num_records: usize, num_fields: usize, c: char) -> String { + // 4 assumes single digits; in reality, there is reallocation because we need bigger + let mut s = String::with_capacity(4 * num_records * num_fields); + for i in 1..=num_records { + s.push('('); + for j in 1..=num_fields { + write!(s, "{}{}, ", c, (i - 1) * num_fields + j).unwrap(); + } + // Remove trailing space + s.pop(); + // Remove trailing comma + s.pop(); + write!(s, "), ").unwrap(); + } + s.pop(); + s.pop(); + s +} + +/// Constructs query args like `($1), ($2), ($3)`. +pub(crate) fn query_args_single_generic(num_records: usize, c: char) -> String { + #[allow(clippy::arithmetic_side_effects)] + let mut s = String::with_capacity(4 * num_records); + for i in 1..=num_records { + write!(s, "({c}{i}), ").unwrap(); + } + s.pop(); + s.pop(); + s +} + +/// Constructs n query args like `($1, $2, $3)`. +/// +/// This is useful for `IN`. +pub(crate) fn query_args_inline_generic(num_records: usize, c: char) -> String { + let mut s = String::with_capacity(4 * num_records); + s.push('('); + for i in 1..=num_records { + #[allow(clippy::unwrap_used)] + write!(s, "{c}{i}, ").unwrap(); + } + // Remove trailing space. + s.pop(); + // Remove trailing comma. + s.pop(); + s.push(')'); + s +} + +/// Constructs an insert statement for bulk inserts. +/// +/// The output is like `INSERT INTO table_name (col1, col2, col3) VALUES ($1, $2, $3), ($4, $5, $6)`. +pub(crate) fn bulk_insert_statement_generic( + table_name: &str, + col_names: &[&str], + num_records: usize, + c: char, +) -> String { + format!( + "INSERT INTO {table_name} ({col_names}) VALUES {values}", + col_names = col_names.join(", "), + values = query_args_generic(num_records, col_names.len(), c) + ) +} + +#[cfg(test)] +mod tests { + use crate::db::util::query_args_single_generic; + + use super::{query_args_generic, query_args_inline_generic}; + + #[test] + fn test_query_args() { + assert_eq!(query_args_generic(2, 2, '$'), "($1, $2), ($3, $4)".to_owned()); + assert_eq!(query_args_generic(3, 2, '$'), "($1, $2), ($3, $4), ($5, $6)".to_owned()); + } + + #[test] + fn test_query_args_single() { + assert_eq!(query_args_single_generic(2, '$'), "($1), ($2)".to_owned()); + assert_eq!(query_args_single_generic(3, '$'), "($1), ($2), ($3)".to_owned()); + } + + #[test] + fn test_query_args_inline() { + assert_eq!(query_args_inline_generic(2, '$'), "(1, 2)".to_owned()); + assert_eq!(query_args_inline_generic(3, '$'), "(1, 2, 3)".to_owned()); + } +} diff --git a/crates/devolutions-pedm/src/lib.rs b/crates/devolutions-pedm/src/lib.rs index 48a74de91..ad4a9b138 100644 --- a/crates/devolutions-pedm/src/lib.rs +++ b/crates/devolutions-pedm/src/lib.rs @@ -3,6 +3,7 @@ use camino::Utf8PathBuf; use devolutions_gateway_task::{ShutdownSignal, Task}; +mod account; mod config; mod db; mod log; diff --git a/crates/devolutions-pedm/src/policy.rs b/crates/devolutions-pedm/src/policy.rs index 16be039bd..51cac00e3 100644 --- a/crates/devolutions-pedm/src/policy.rs +++ b/crates/devolutions-pedm/src/policy.rs @@ -442,25 +442,23 @@ pub(crate) fn application_from_process(pid: u32) -> anyhow::Result pub(crate) fn authenticode_win_to_policy( win_status: win_api_wrappers::security::crypt::AuthenticodeSignatureStatus, -) -> policy::AuthenticodeSignatureStatus { +) -> AuthenticodeSignatureStatus { match win_status { - win_api_wrappers::security::crypt::AuthenticodeSignatureStatus::Valid => { - policy::AuthenticodeSignatureStatus::Valid - } + win_api_wrappers::security::crypt::AuthenticodeSignatureStatus::Valid => AuthenticodeSignatureStatus::Valid, win_api_wrappers::security::crypt::AuthenticodeSignatureStatus::Incompatible => { - policy::AuthenticodeSignatureStatus::Incompatible + AuthenticodeSignatureStatus::Incompatible } win_api_wrappers::security::crypt::AuthenticodeSignatureStatus::NotSigned => { - policy::AuthenticodeSignatureStatus::NotSigned + AuthenticodeSignatureStatus::NotSigned } win_api_wrappers::security::crypt::AuthenticodeSignatureStatus::HashMismatch => { - policy::AuthenticodeSignatureStatus::HashMismatch + AuthenticodeSignatureStatus::HashMismatch } win_api_wrappers::security::crypt::AuthenticodeSignatureStatus::NotSupportedFileFormat => { - policy::AuthenticodeSignatureStatus::NotSupportedFileFormat + AuthenticodeSignatureStatus::NotSupportedFileFormat } win_api_wrappers::security::crypt::AuthenticodeSignatureStatus::NotTrusted => { - policy::AuthenticodeSignatureStatus::NotTrusted + AuthenticodeSignatureStatus::NotTrusted } } } From b9af8cbf10d477c22dfaa62d9d14a18319307cf0 Mon Sep 17 00:00:00 2001 From: Allan Zhang <6740989+allan2@users.noreply.github.com> Date: Fri, 25 Apr 2025 22:13:18 -0400 Subject: [PATCH 2/7] Fix query args test typo The function works as expected. The expected side of the test was wrong. --- crates/devolutions-pedm/src/db/util.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/devolutions-pedm/src/db/util.rs b/crates/devolutions-pedm/src/db/util.rs index 779329b6c..a76464c45 100644 --- a/crates/devolutions-pedm/src/db/util.rs +++ b/crates/devolutions-pedm/src/db/util.rs @@ -86,7 +86,7 @@ mod tests { #[test] fn test_query_args_inline() { - assert_eq!(query_args_inline_generic(2, '$'), "(1, 2)".to_owned()); - assert_eq!(query_args_inline_generic(3, '$'), "(1, 2, 3)".to_owned()); + assert_eq!(query_args_inline_generic(2, '$'), "($1, $2)".to_owned()); + assert_eq!(query_args_inline_generic(3, '$'), "($1, $2, $3)".to_owned()); } } From 08e409acb4d9b711a2d50d24c275a7986aff6c42 Mon Sep 17 00:00:00 2001 From: Allan Zhang <6740989+allan2@users.noreply.github.com> Date: Tue, 13 May 2025 13:40:29 -0400 Subject: [PATCH 3/7] Split unsafe blocks and add safety comments --- crates/devolutions-pedm/src/account.rs | 87 +++++++++++++++----------- 1 file changed, 51 insertions(+), 36 deletions(-) diff --git a/crates/devolutions-pedm/src/account.rs b/crates/devolutions-pedm/src/account.rs index cf1c2fc6a..c23befbb7 100644 --- a/crates/devolutions-pedm/src/account.rs +++ b/crates/devolutions-pedm/src/account.rs @@ -209,7 +209,6 @@ impl PartialEq for AccountWithId { /// /// `LookupAccountNameW` must be called to enable `ConvertSidToStringSidW` to work. #[cfg(target_os = "windows")] -#[allow(clippy::multiple_unsafe_ops_per_block)] #[allow(clippy::cast_possible_truncation)] pub(crate) unsafe fn list_accounts() -> Result, ListAccountsError> { use windows::core::PWSTR; @@ -220,13 +219,16 @@ pub(crate) unsafe fn list_accounts() -> Result, ListAccountsError> use windows::Win32::Security::{LookupAccountNameW, PSID, SECURITY_MAX_SID_SIZE, SID_NAME_USE}; // SAFETY: uses `NetUserEnum` and `LookupAccountNameW` from `windows` - unsafe { - let mut buf: *mut u8 = std::ptr::null_mut(); - let mut entries_read = 0; - let mut total_entries = 0; + let mut buf: *mut u8 = std::ptr::null_mut(); + let mut entries_read = 0; + let mut total_entries = 0; + + // SAFETY: `buf` is a null-initialized out-pointer that NetUserEnum will allocate. + // `entries_read` and `total_entries` are valid pointers to receive output counts. + let status = unsafe { // Get the list of user accounts. - let status = NetUserEnum( + NetUserEnum( None, 0, FILTER_NORMAL_ACCOUNT, @@ -235,48 +237,61 @@ pub(crate) unsafe fn list_accounts() -> Result, ListAccountsError> &mut entries_read, &mut total_entries, None, - ); - - if status != NERR_Success { - return Err(ListAccountsError::NetUserEnumFail(status)); - } - - #[allow(clippy::cast_ptr_alignment)] - let users = std::slice::from_raw_parts(buf as *const USER_INFO_0, entries_read as usize); - - let mut accounts = Vec::with_capacity(users.len()); - for user in users { - let username = user.usri0_name.display(); - let mut sid = [0u8; SECURITY_MAX_SID_SIZE as usize]; - let mut sid_size = sid.len() as u32; - let mut domain_name = [0u16; 256]; - let mut domain_size = domain_name.len() as u32; - let mut sid_type = SID_NAME_USE(0); - let sid = PSID(sid.as_mut_ptr().cast()); + ) + }; + if status != NERR_Success { + return Err(ListAccountsError::NetUserEnumFail(status)); + } + + #[expect(clippy::cast_ptr_alignment)] + // SAFETY: `buf` is guaranteed by `NetUserEnum` to point to an array of `USER_INFO_0` structs. + // We cast it and build a slice with `entries_read` elements, which was returned alongside `buf`. + // We expect the alignment to be correct because `USER_INFO_0` is a `#[repr(C)]` struct with a single field, so it is identical to the alignment of PWSTR. + let users = unsafe { std::slice::from_raw_parts(buf as *const USER_INFO_0, entries_read as usize) }; + + let mut accounts = Vec::with_capacity(users.len()); + for user in users { + // SAFETY: `user.usri0_name` is a valid string. + let name = unsafe { user.usri0_name.display() }.to_string(); + let mut sid = [0u8; SECURITY_MAX_SID_SIZE as usize]; + let mut sid_size = sid.len() as u32; + let mut domain_name = [0u16; 256]; + let mut domain_size = domain_name.len() as u32; + let domain_name = PWSTR(domain_name.as_mut_ptr()); + let mut sid_type = SID_NAME_USE(0); + let sid = PSID(sid.as_mut_ptr().cast()); + + // SAFETY: `user.usri0_name` is a valid string. + // `sid` and `domain_name` buffers are correctly sized and initialized. + // `sid_size` and `domain_size` are set to their respective buffer lengths. + // All pointers are valid for writes. + unsafe { LookupAccountNameW( None, user.usri0_name, Some(sid), &mut sid_size, - Some(PWSTR(domain_name.as_mut_ptr())), + Some(domain_name), &mut domain_size, &mut sid_type, - )?; + ) + }?; - let mut sid_str: PWSTR = PWSTR::null(); - ConvertSidToStringSidW(sid, &mut sid_str)?; + let mut sid_str: PWSTR = PWSTR::null(); + // SAFETY: `sid` is a valid buffer previously populated by `LookupAccountNameW`. + unsafe { ConvertSidToStringSidW(sid, &mut sid_str) }?; + // SAFETY: `sid_str` is a valid string. + let s = unsafe { sid_str.to_string() }?; + let sid = Sid::from_str(&s)?; + accounts.push(Account { name, sid }) + } - // convert to string - let s = sid_str.to_string()?; - accounts.push(Account { - name: username.to_string(), - sid: Sid::from_str(&s)?, - }) - } + // SAFETY: `buf` was allocated by `NetUserEnum` and must be freed. + unsafe { NetApiBufferFree(Some(buf as *mut _)); - Ok(accounts) } + Ok(accounts) } /// A diff of changes between two lists of accounts. From c7126f0cfa901a7a717b6bfbe203b5a79b2d994d Mon Sep 17 00:00:00 2001 From: Allan Zhang <6740989+allan2@users.noreply.github.com> Date: Tue, 13 May 2025 13:43:07 -0400 Subject: [PATCH 4/7] Use `u32::try_from` --- crates/devolutions-pedm/src/account.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/crates/devolutions-pedm/src/account.rs b/crates/devolutions-pedm/src/account.rs index c23befbb7..ba00769ca 100644 --- a/crates/devolutions-pedm/src/account.rs +++ b/crates/devolutions-pedm/src/account.rs @@ -1,6 +1,6 @@ use core::{error, fmt}; use std::collections::{HashMap, HashSet}; -use std::num::ParseIntError; +use std::num::{ParseIntError, TryFromIntError}; use std::str::FromStr; use std::string::FromUtf16Error; @@ -209,7 +209,6 @@ impl PartialEq for AccountWithId { /// /// `LookupAccountNameW` must be called to enable `ConvertSidToStringSidW` to work. #[cfg(target_os = "windows")] -#[allow(clippy::cast_possible_truncation)] pub(crate) unsafe fn list_accounts() -> Result, ListAccountsError> { use windows::core::PWSTR; use windows::Win32::NetworkManagement::NetManagement::{ @@ -255,9 +254,9 @@ pub(crate) unsafe fn list_accounts() -> Result, ListAccountsError> // SAFETY: `user.usri0_name` is a valid string. let name = unsafe { user.usri0_name.display() }.to_string(); let mut sid = [0u8; SECURITY_MAX_SID_SIZE as usize]; - let mut sid_size = sid.len() as u32; + let mut sid_size = u32::try_from(sid.len())?; let mut domain_name = [0u16; 256]; - let mut domain_size = domain_name.len() as u32; + let mut domain_size = u32::try_from(domain_name.len())?; let domain_name = PWSTR(domain_name.as_mut_ptr()); let mut sid_type = SID_NAME_USE(0); let sid = PSID(sid.as_mut_ptr().cast()); @@ -405,6 +404,7 @@ pub(crate) fn diff_accounts(old: &[AccountWithId], new: &[Account]) -> AccountsD pub enum ListAccountsError { FromUtf16(FromUtf16Error), ParseSid(ParseSidError), + TryFromInt(TryFromIntError), #[cfg(target_os = "windows")] Windows(windows_result::Error), /// Contains `nStatus`. @@ -416,6 +416,7 @@ impl error::Error for ListAccountsError { match self { Self::FromUtf16(e) => Some(e), Self::ParseSid(e) => Some(e), + Self::TryFromInt(e) => Some(e), #[cfg(target_os = "windows")] Self::Windows(e) => Some(e), Self::NetUserEnumFail(_) => None, @@ -428,6 +429,7 @@ impl fmt::Display for ListAccountsError { match self { Self::FromUtf16(e) => e.fmt(f), Self::ParseSid(e) => e.fmt(f), + Self::TryFromInt(e) => e.fmt(f), #[cfg(target_os = "windows")] Self::Windows(e) => e.fmt(f), Self::NetUserEnumFail(n) => { @@ -447,6 +449,11 @@ impl From for ListAccountsError { Self::ParseSid(e) } } +impl From for ListAccountsError { + fn from(e: TryFromIntError) -> Self { + Self::TryFromInt(e) + } +} #[cfg(target_os = "windows")] impl From for ListAccountsError { fn from(e: windows_result::Error) -> Self { From 71f20c4e678b49674164a6f72ec8e55b97473910 Mon Sep 17 00:00:00 2001 From: Allan Zhang <6740989+allan2@users.noreply.github.com> Date: Tue, 13 May 2025 13:44:23 -0400 Subject: [PATCH 5/7] Unmark `list_accounts` as unsafe --- crates/devolutions-pedm/src/account.rs | 2 +- crates/devolutions-pedm/src/api/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/devolutions-pedm/src/account.rs b/crates/devolutions-pedm/src/account.rs index ba00769ca..88522ee7f 100644 --- a/crates/devolutions-pedm/src/account.rs +++ b/crates/devolutions-pedm/src/account.rs @@ -209,7 +209,7 @@ impl PartialEq for AccountWithId { /// /// `LookupAccountNameW` must be called to enable `ConvertSidToStringSidW` to work. #[cfg(target_os = "windows")] -pub(crate) unsafe fn list_accounts() -> Result, ListAccountsError> { +pub(crate) fn list_accounts() -> Result, ListAccountsError> { use windows::core::PWSTR; use windows::Win32::NetworkManagement::NetManagement::{ NERR_Success, NetApiBufferFree, NetUserEnum, FILTER_NORMAL_ACCOUNT, MAX_PREFERRED_LENGTH, USER_INFO_0, diff --git a/crates/devolutions-pedm/src/api/mod.rs b/crates/devolutions-pedm/src/api/mod.rs index c438c820b..a7a24a166 100644 --- a/crates/devolutions-pedm/src/api/mod.rs +++ b/crates/devolutions-pedm/src/api/mod.rs @@ -183,7 +183,7 @@ pub async fn serve(config: Config, shutdown_signal: ShutdownSignal) -> Result<() // Update the list of accounts in the database. // SAFETY: uses `NetUserEnum` and `LookupAccountNameW` from `windows` - let accounts = unsafe { list_accounts()? }; + let accounts = list_accounts()?; let db_accounts = db.get_accounts().await?; info!("Accounts retrieved successfully"); From d58bc9071da60073874c927940bbb4de1a9030b2 Mon Sep 17 00:00:00 2001 From: Allan Zhang <6740989+allan2@users.noreply.github.com> Date: Tue, 13 May 2025 14:05:51 -0400 Subject: [PATCH 6/7] Rebase master --- crates/devolutions-pedm/src/api/mod.rs | 3 +-- crates/devolutions-pedm/src/db/mod.rs | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/devolutions-pedm/src/api/mod.rs b/crates/devolutions-pedm/src/api/mod.rs index a7a24a166..17a7a7809 100644 --- a/crates/devolutions-pedm/src/api/mod.rs +++ b/crates/devolutions-pedm/src/api/mod.rs @@ -182,9 +182,8 @@ pub async fn serve(config: Config, shutdown_signal: ShutdownSignal) -> Result<() // Update the list of accounts in the database. - // SAFETY: uses `NetUserEnum` and `LookupAccountNameW` from `windows` + // Get the system's accounts and update if needed. let accounts = list_accounts()?; - let db_accounts = db.get_accounts().await?; info!("Accounts retrieved successfully"); let diff = diff_accounts(&db_accounts, &accounts); diff --git a/crates/devolutions-pedm/src/db/mod.rs b/crates/devolutions-pedm/src/db/mod.rs index bb70d60d3..ee6a09858 100644 --- a/crates/devolutions-pedm/src/db/mod.rs +++ b/crates/devolutions-pedm/src/db/mod.rs @@ -283,7 +283,7 @@ pub(crate) struct DbAsyncBridgeTask { } impl DbAsyncBridgeTask { - pub fn new(db: Db) -> (DbHandle, Self) { + pub(crate) fn new(db: Db) -> (DbHandle, Self) { let (tx, rx) = tokio::sync::mpsc::channel(8); (DbHandle { tx }, Self { db, rx }) } From 409200f30e4ab9c834ac8712f60ee86a536cd49d Mon Sep 17 00:00:00 2001 From: Allan Zhang <6740989+allan2@users.noreply.github.com> Date: Tue, 20 May 2025 13:09:04 -0400 Subject: [PATCH 7/7] Remove mistakenly added build.rs --- crates/devolutions-pedm/build.rs | 19 ------------------- 1 file changed, 19 deletions(-) delete mode 100644 crates/devolutions-pedm/build.rs diff --git a/crates/devolutions-pedm/build.rs b/crates/devolutions-pedm/build.rs deleted file mode 100644 index 32abd9420..000000000 --- a/crates/devolutions-pedm/build.rs +++ /dev/null @@ -1,19 +0,0 @@ -use std::path::Path; -use std::{env, fs}; - -fn main() -> Result<(), Box> { - let manifest_dir = env::var("CARGO_MANIFEST_DIR")?; - // This expects the PEDM crate to be in repo_root/crates/devolutions-pedm. - let repo_root = Path::new(&manifest_dir) - .parent() - .expect("crates dir missing") - .parent() - .expect("repo root missing"); - - let version = fs::read_to_string(repo_root.join("VERSION")) - .expect("failed to read VERSION file") - .trim_end() - .to_string(); - println!("cargo:rustc-env=VERSION={}", version); - Ok(()) -}