From 2fc327a8d50b0c9b96093643f10b215af384d17a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 6 May 2026 09:25:44 -0400 Subject: [PATCH] [F55 Path A] DCOM-managed INmxSvcCallback sink MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the hand-rolled CallbackExporter (TCP listener + custom OBJREF) with a real `windows-rs` `#[implement]` COM class for INmxSvcCallback, marshalled via CoMarshalInterface. NmxSvc validates the callback OBJREF by calling IObjectExporter::ResolveOxid against the local RPCSS at 127.0.0.1:135; hand-rolled OXIDs aren't registered there, which is why RegisterEngine2 returned RPC_S_SERVER_UNAVAILABLE (1722) on every live attempt. CoMarshalInterface registers the OXID with RPCSS automatically, so the SCM-side resolution succeeds. Mirrors MxNativeSession.CreateRegisteredService (cs:624), which is the .NET reference's working path: ComObjRefProvider.MarshalInterfaceObjRef(callback, INmxSvcCallback, DifferentMachine) Layout: - mxaccess-callback::dcom_sink — INmxSvcCallback + DcomCallbackSink + create_dcom_callback_sink_objref. Forwards inbound calls into the same CallbackEvent::CallbackInvoked { opnum, body } shape the legacy exporter produces, so callback_router stays path-agnostic. - Session::from_nmx_client — branched on `windows-com`. Real DCOM sink when on; legacy CallbackExporter when off (kept for unit tests that run against an in-process fake NMX peer). - SessionInner.dcom_sink_holder: Option — keeps the COM ref alive for the session's lifetime; shutdown_nmx drops it. - mxaccess-rpc + mxaccess-callback: windows-rs 0.59 → 0.62. The 0.59 #[implement] macro generates code that doesn't compile under edition 2024; 0.62 is fixed. Live result: cargo test -p mxaccess-compat --features live-windows-com --test lmx_write_complete_live -- --ignored --nocapture passes end-to-end. RegisterEngine2 OK, write round-trips, OnWriteComplete fires with the captured MxStatus shape. Unblocks F49 step 5; F55 marked Resolved in design/followups.md. Co-Authored-By: Claude Opus 4.7 (1M context) --- design/followups.md | 5 +- rust/Cargo.lock | 154 +++++------ rust/crates/mxaccess-callback/Cargo.toml | 31 +++ .../crates/mxaccess-callback/src/dcom_sink.rs | 248 ++++++++++++++++++ rust/crates/mxaccess-callback/src/lib.rs | 17 +- rust/crates/mxaccess-rpc/Cargo.toml | 2 +- .../mxaccess-rpc/src/com_objref_provider.rs | 14 +- rust/crates/mxaccess/Cargo.toml | 2 +- rust/crates/mxaccess/src/session.rs | 151 ++++++++--- 9 files changed, 493 insertions(+), 131 deletions(-) create mode 100644 rust/crates/mxaccess-callback/src/dcom_sink.rs diff --git a/design/followups.md b/design/followups.md index 589f4ee..06e02b0 100644 --- a/design/followups.md +++ b/design/followups.md @@ -37,7 +37,9 @@ Between each publish: wait for the crate to be indexed before the next one's `ca 4. **F40 metrics** — install a `metrics` exporter (`metrics-exporter-prometheus` is the lightest), run `connect-write-read` + `subscribe` examples with `--features metrics`, confirm at least one counter increment and one histogram observation per metric name in the registered set. 5. **F54 OnWriteComplete (LmxClient round-trip)** — scaffold lives at `crates/mxaccess-compat/tests/lmx_write_complete_live.rs`. Run `cargo test -p mxaccess-compat --features live-windows-com --test lmx_write_complete_live -- --ignored --nocapture` to drive `LmxClient::write` → drain `client.on_write_complete()` and assert the `WriteCompleteEvent { server_handle, item_handle, statuses, is_during_recovery }` shape matches `LMX_OnWriteComplete(int hLMXServerHandle, int phItemHandle, ref MXSTATUS_PROXY[] pVars)`. -**Live attempt 2026-05-06.** Steps 1-4 not run yet. Step 5 attempted; the test compiled and ran past Frida-style `--probe-resolve-oxid-managed-ntlm-integrity` resolution + `--probe-remqi-managed` IPID extraction, but `connect_nmx_auto` (preferred path) and `connect_nmx` (fallback with probe-resolved IPID) both fail with `Status { detail: 1722 }` (RPC_S_SERVER_UNAVAILABLE). The .NET `MxNativeClient.Probe --probe-session-write` runs the same scenario successfully end-to-end against the same AVEVA install, so the wire is functional and the failure is Rust-port specific. Documented as the F12 hardening followup; the F54 unit-level integration tests (`router_populates_operation_status_context_from_pending_ops_fifo` + `write_handle_correlates_with_router_emitted_status`) cover the F54 logic exhaustively at the layer boundaries. +**Live attempt 2026-05-06.** Steps 1-4 not run yet. Step 5 attempted; the test compiled and ran past Frida-style `--probe-resolve-oxid-managed-ntlm-integrity` resolution + `--probe-remqi-managed` IPID extraction, but `connect_nmx_auto` (preferred path) and `connect_nmx` (fallback with probe-resolved IPID) both fail with `Status { detail: 1722 }` (RPC_S_SERVER_UNAVAILABLE). The .NET `MxNativeClient.Probe --probe-session-write` runs the same scenario successfully end-to-end against the same AVEVA install, so the wire is functional and the failure is Rust-port specific. Root-caused as F55 (hand-rolled callback exporter rejected by NmxSvc's SCM-side OXID resolution); not a tokio-runtime COM-activation issue. + +**Step 5 unblocked 2026-05-06 by F55 / Path A.** `cargo test -p mxaccess-compat --features live-windows-com --test lmx_write_complete_live -- --ignored --nocapture` passes against the live AVEVA install: RegisterEngine2 OK, write round-trips, OnWriteComplete fires with the expected `WriteCompleteEvent { server_handle, item_handle, statuses, is_during_recovery }` shape. Steps 1-4 still pending. **Definition of done:** 1. Per-feature evidence summary in `docs/M6-live-verification.md` (one paragraph per feature with the wire-trace excerpt or metrics-exporter snapshot). @@ -99,6 +101,7 @@ Between each publish: wait for the crate to be indexed before the next one's `ca **Resolves when:** the lint is on and the workspace doc build is warning-clean with it. ### F55 — Hand-rolled callback exporter rejected by `RegisterEngine2` on this AVEVA install +**Status:** Resolved 2026-05-06 by Path A (DCOM-managed `INmxSvcCallback` sink in `mxaccess-callback::dcom_sink`, wired into `Session::from_nmx_client` behind the `windows-com` feature). Live test `cargo test -p mxaccess-compat --features live-windows-com --test lmx_write_complete_live -- --ignored --nocapture` passes end-to-end: RegisterEngine2 succeeds, write round-trips, OnWriteComplete fires with status from the wire. The hand-rolled `CallbackExporter` is retained for unit tests that exercise the exporter against an in-process fake NMX peer. **Severity:** P1 — blocks F49 live verification of every M6 feature that needs an `Engine` registered (i.e. all of them). **Source:** Live attempt 2026-05-06 against the local AVEVA install. Both the Rust port and the .NET reference's `--probe-register-managed-callback` (which uses the same hand-rolled-exporter approach as the Rust port) fail `RegisterEngine2` with HRESULT `0x800706BA` (`RPC_S_SERVER_UNAVAILABLE` wrapped as Win32 HRESULT). The .NET reference's `--probe-session-write` SUCCEEDS because it goes through `MxNativeSession.Open` → `CreateRegisteredService` (`MxNativeSession.cs:624`) which does **`ComObjRefProvider.MarshalInterfaceObjRef(callback, INmxSvcCallback, DifferentMachine)`** on a real C# COM object — letting Windows DCOM proxy/stub infrastructure handle the callback dispatch — instead of building a hand-rolled OBJREF + TCP listener. diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 9df44ed..b2043d0 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -561,6 +561,8 @@ dependencies = [ "rand 0.8.6", "tokio", "tracing", + "windows", + "windows-core", ] [[package]] @@ -1272,32 +1274,54 @@ dependencies = [ [[package]] name = "windows" -version = "0.59.0" +version = "0.62.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7f919aee0a93304be7f62e8e5027811bbba96bcb1de84d6618be56e43f8a32a1" +checksum = "527fadee13e0c05939a6a05d5bd6eec6cd2e3dbd648b9f8e447c6518133d8580" +dependencies = [ + "windows-collections", + "windows-core", + "windows-future", + "windows-numerics", +] + +[[package]] +name = "windows-collections" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23b2d95af1a8a14a3c7367e1ed4fc9c20e0a26e79551b1454d72583c97cc6610" dependencies = [ "windows-core", - "windows-targets 0.53.5", ] [[package]] name = "windows-core" -version = "0.59.0" +version = "0.62.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "810ce18ed2112484b0d4e15d022e5f598113e220c53e373fb31e67e21670c1ce" +checksum = "b8e83a14d34d0623b51dce9581199302a221863196a1dde71a7663a4c2be9deb" dependencies = [ "windows-implement", "windows-interface", + "windows-link", "windows-result", "windows-strings", - "windows-targets 0.53.5", +] + +[[package]] +name = "windows-future" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e1d6f90251fe18a279739e78025bd6ddc52a7e22f921070ccdc67dde84c605cb" +dependencies = [ + "windows-core", + "windows-link", + "windows-threading", ] [[package]] name = "windows-implement" -version = "0.59.0" +version = "0.60.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "83577b051e2f49a058c308f17f273b570a6a758386fc291b5f6a934dd84e48c1" +checksum = "053e2e040ab57b9dc951b72c264860db7eb3b0200ba345b4e4c3b14f67855ddf" dependencies = [ "proc-macro2", "quote", @@ -1315,12 +1339,6 @@ dependencies = [ "syn", ] -[[package]] -name = "windows-link" -version = "0.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5e6ad25900d524eaabdbbb96d20b4311e1e7ae1699af4fb28c17ae66c80d798a" - [[package]] name = "windows-link" version = "0.2.1" @@ -1328,21 +1346,31 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" [[package]] -name = "windows-result" -version = "0.3.4" +name = "windows-numerics" +version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "56f42bd332cc6c8eac5af113fc0c1fd6a8fd2aa08a0119358686e5160d0586c6" +checksum = "6e2e40844ac143cdb44aead537bbf727de9b044e107a0f1220392177d15b0f26" dependencies = [ - "windows-link 0.1.3", + "windows-core", + "windows-link", +] + +[[package]] +name = "windows-result" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7781fa89eaf60850ac3d2da7af8e5242a5ea78d1a11c49bf2910bb5a73853eb5" +dependencies = [ + "windows-link", ] [[package]] name = "windows-strings" -version = "0.3.1" +version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "87fa48cc5d406560701792be122a10132491cff9d0aeb23583cc2dcafc847319" +checksum = "7837d08f69c77cf6b07689544538e017c1bfcf57e34b4c0ff58e6c2cd3b37091" dependencies = [ - "windows-link 0.1.3", + "windows-link", ] [[package]] @@ -1351,7 +1379,7 @@ version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "282be5f36a8ce781fad8c8ae18fa3f9beff57ec1b52cb3de0789201425d9a33d" dependencies = [ - "windows-targets 0.52.6", + "windows-targets", ] [[package]] @@ -1360,7 +1388,7 @@ version = "0.61.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ae137229bcbd6cdf0f7b80a31df61766145077ddf49416a728b02cb3921ff3fc" dependencies = [ - "windows-link 0.2.1", + "windows-link", ] [[package]] @@ -1369,31 +1397,23 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9b724f72796e036ab90c1021d4780d4d3d648aca59e491e6b98e725b84e99973" dependencies = [ - "windows_aarch64_gnullvm 0.52.6", - "windows_aarch64_msvc 0.52.6", - "windows_i686_gnu 0.52.6", - "windows_i686_gnullvm 0.52.6", - "windows_i686_msvc 0.52.6", - "windows_x86_64_gnu 0.52.6", - "windows_x86_64_gnullvm 0.52.6", - "windows_x86_64_msvc 0.52.6", + "windows_aarch64_gnullvm", + "windows_aarch64_msvc", + "windows_i686_gnu", + "windows_i686_gnullvm", + "windows_i686_msvc", + "windows_x86_64_gnu", + "windows_x86_64_gnullvm", + "windows_x86_64_msvc", ] [[package]] -name = "windows-targets" -version = "0.53.5" +name = "windows-threading" +version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4945f9f551b88e0d65f3db0bc25c33b8acea4d9e41163edf90dcd0b19f9069f3" +checksum = "3949bd5b99cafdf1c7ca86b43ca564028dfe27d66958f2470940f73d86d75b37" dependencies = [ - "windows-link 0.2.1", - "windows_aarch64_gnullvm 0.53.1", - "windows_aarch64_msvc 0.53.1", - "windows_i686_gnu 0.53.1", - "windows_i686_gnullvm 0.53.1", - "windows_i686_msvc 0.53.1", - "windows_x86_64_gnu 0.53.1", - "windows_x86_64_gnullvm 0.53.1", - "windows_x86_64_msvc 0.53.1", + "windows-link", ] [[package]] @@ -1402,96 +1422,48 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3" -[[package]] -name = "windows_aarch64_gnullvm" -version = "0.53.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a9d8416fa8b42f5c947f8482c43e7d89e73a173cead56d044f6a56104a6d1b53" - [[package]] name = "windows_aarch64_msvc" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" -[[package]] -name = "windows_aarch64_msvc" -version = "0.53.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b9d782e804c2f632e395708e99a94275910eb9100b2114651e04744e9b125006" - [[package]] name = "windows_i686_gnu" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8e9b5ad5ab802e97eb8e295ac6720e509ee4c243f69d781394014ebfe8bbfa0b" -[[package]] -name = "windows_i686_gnu" -version = "0.53.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "960e6da069d81e09becb0ca57a65220ddff016ff2d6af6a223cf372a506593a3" - [[package]] name = "windows_i686_gnullvm" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66" -[[package]] -name = "windows_i686_gnullvm" -version = "0.53.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fa7359d10048f68ab8b09fa71c3daccfb0e9b559aed648a8f95469c27057180c" - [[package]] name = "windows_i686_msvc" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66" -[[package]] -name = "windows_i686_msvc" -version = "0.53.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e7ac75179f18232fe9c285163565a57ef8d3c89254a30685b57d83a38d326c2" - [[package]] name = "windows_x86_64_gnu" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78" -[[package]] -name = "windows_x86_64_gnu" -version = "0.53.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c3842cdd74a865a8066ab39c8a7a473c0778a3f29370b5fd6b4b9aa7df4a499" - [[package]] name = "windows_x86_64_gnullvm" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d" -[[package]] -name = "windows_x86_64_gnullvm" -version = "0.53.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ffa179e2d07eee8ad8f57493436566c7cc30ac536a3379fdf008f47f6bb7ae1" - [[package]] name = "windows_x86_64_msvc" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" -[[package]] -name = "windows_x86_64_msvc" -version = "0.53.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d6bbff5f0aada427a1e5a6da5f1f98158182f26556f345ac9e04d36d0ebed650" - [[package]] name = "zerocopy" version = "0.8.48" diff --git a/rust/crates/mxaccess-callback/Cargo.toml b/rust/crates/mxaccess-callback/Cargo.toml index 73bb44f..9290ba5 100644 --- a/rust/crates/mxaccess-callback/Cargo.toml +++ b/rust/crates/mxaccess-callback/Cargo.toml @@ -15,5 +15,36 @@ tokio = { workspace = true } tracing = { workspace = true } rand = "0.8" +# F55 / Path A — DCOM-managed callback sink. +# `windows-com` enables `dcom_sink.rs` which implements +# `INmxSvcCallback` as a real COM class via `windows-rs` `#[implement]`. +# The marshalled OBJREF passes NmxSvc's SCM-side OXID resolution +# where the hand-rolled `exporter.rs` approach fails. Default build +# stays slim — the windows crate is only pulled in when the consumer +# enables `windows-com`. Propagates through to +# `mxaccess-rpc/windows-com` so the OBJREF marshaller is available. +windows = { version = "0.62", features = [ + "Win32_Foundation", + "Win32_System_Com", + "Win32_System_Com_Marshal", + "Win32_System_Com_StructuredStorage", + "Win32_System_Memory", +], optional = true } +# windows-rs's `#[interface]` and `#[implement]` macros expand to +# absolute `::windows_core::*` paths, so the consumer must depend on +# `windows-core` directly (the `windows` crate's re-export at +# `windows::core` doesn't satisfy the macro's path resolution). +# Pin to the same 0.62 line as the `windows` dep above so the +# `IUnknown` / `IUnknown_Vtbl` types resolve to the same crate +# version that `mxaccess-rpc::com_objref_provider::IUnknownHolder` +# wraps — version skew between the two would surface as "expected +# IUnknown, found IUnknown" type errors at the +# `IUnknownHolder::from_iunknown` boundary. +windows-core = { version = "0.62", optional = true } + +[features] +default = [] +windows-com = ["dep:windows", "dep:windows-core", "mxaccess-rpc/windows-com"] + [lints] workspace = true diff --git a/rust/crates/mxaccess-callback/src/dcom_sink.rs b/rust/crates/mxaccess-callback/src/dcom_sink.rs new file mode 100644 index 0000000..50a6116 --- /dev/null +++ b/rust/crates/mxaccess-callback/src/dcom_sink.rs @@ -0,0 +1,248 @@ +// `windows_core::interface` doesn't tolerate sibling attributes on the +// trait, and the COM method names must mirror the .NET reference's +// PascalCase to keep the IDL/MIDL trail readable. Allow at module +// scope so the generated `_Impl` trait + vtable struct don't trip +// `non_snake_case`. +#![allow(non_snake_case)] + +//! DCOM-managed `INmxSvcCallback` sink — Path A of F55. +//! +//! The hand-rolled `CallbackExporter` (this crate's [`crate::exporter`] +//! module) advertises a TCP listener via a custom OBJREF that NmxSvc +//! refuses with `RPC_S_SERVER_UNAVAILABLE` (1722) on RegisterEngine2. +//! Live diff against the working .NET `MxNativeSession.Open` path +//! (which uses `ComObjRefProvider.MarshalInterfaceObjRef(callback, +//! INmxSvcCallback, DifferentMachine)` per `MxNativeSession.cs:624`) +//! showed the failure isn't an OBJREF byte-format issue — it's that +//! NmxSvc does its own SCM-side `IObjectExporter::ResolveOxid` against +//! the local RPCSS at `127.0.0.1:135` to validate the callback OXID, +//! and a hand-rolled OXID isn't registered with RPCSS. +//! +//! This module sidesteps that by implementing `INmxSvcCallback` as a +//! real `windows-rs` `#[implement]` COM class. `CoMarshalInterface` +//! then registers the callback's OXID with RPCSS automatically, so +//! NmxSvc's SCM-side resolution succeeds. Inbound `DataReceivedRaw` / +//! `StatusReceivedRaw` calls arrive on the DCOM stub thread and are +//! forwarded into the same `CallbackEvent` mpsc the hand-rolled +//! exporter feeds, so the upstream `callback_router` in `mxaccess` +//! doesn't need to know which path produced the event. +//! +//! Mirrors `src/MxNativeClient/NmxCallbackSink.cs` (the .NET reference's +//! DCOM-managed callback used by the `MxNativeSession.Open` path). + +use std::ptr; + +use tokio::sync::mpsc; +use tracing::{debug, warn}; +use windows::Win32::System::Com::Marshal::CoMarshalInterface; +use windows::Win32::System::Com::StructuredStorage::{ + CreateStreamOnHGlobal, GetHGlobalFromStream, +}; +use windows::Win32::System::Com::{IStream, MSHCTX_DIFFERENTMACHINE, MSHLFLAGS_NORMAL}; +use windows::Win32::System::Memory::{GlobalLock, GlobalSize, GlobalUnlock}; +// `#[interface]` / `#[implement]` macros expand to `::windows_core::*` +// paths, so we import via windows_core (which the windows crate +// re-exports). `IUnknown_Vtbl` etc. need to be in scope at the crate +// root. +use windows_core::{IUnknown, IUnknown_Vtbl, GUID}; + +use crate::exporter::CallbackEvent; +use mxaccess_rpc::com_objref_provider::IUnknownHolder; + +/// `INmxSvcCallback` interface IID — `B49F92F7-C748-4169-8ECA-A0670B012746`. +/// Mirrors the .NET reference's `INmxSvcCallback` declaration at +/// `src/MxNativeClient/NmxComContracts.cs:84`. +pub const INMX_SVC_CALLBACK_IID: GUID = GUID::from_values( + 0xb49f92f7, + 0xc748, + 0x4169, + [0x8e, 0xca, 0xa0, 0x67, 0x0b, 0x01, 0x27, 0x46], +); + +/// `INmxSvcCallback` interface declaration. +/// +/// Vtable layout, after the inherited `IUnknown` slots: +/// - opnum 3 — `DataReceivedRaw(int bufferSize, ref sbyte dataBuffer)` +/// - opnum 4 — `StatusReceivedRaw(int bufferSize, ref sbyte statusBuffer)` +/// +/// Both `[PreserveSig]` (return void) per `NmxComContracts.cs:87-91`. +/// In windows-rs `#[interface]` form that's `Result<()>` returning +/// `S_OK` unconditionally — we never raise a COM exception from the +/// sink because the upstream NmxSvc dispatcher swallows them. +#[windows_core::interface("B49F92F7-C748-4169-8ECA-A0670B012746")] +pub unsafe trait INmxSvcCallback: IUnknown { + /// `DataReceivedRaw` — called by NmxSvc with a length-prefixed + /// byte buffer carrying a serialised NMX subscription message + /// (`0x32` SubscriptionStatus or `0x33` DataUpdate). + /// + /// # Safety + /// `data_buffer` is a stub-side pointer to `buffer_size` bytes + /// owned by the COM proxy/stub layer; valid for the duration of + /// the call. Implementations MUST copy the buffer before returning. + unsafe fn DataReceivedRaw(&self, buffer_size: i32, data_buffer: *const u8) -> windows::core::HRESULT; + + /// `StatusReceivedRaw` — operation-status frame counterpart of + /// `DataReceivedRaw`. Same buffer-ownership contract. + /// + /// # Safety + /// As above. + unsafe fn StatusReceivedRaw(&self, buffer_size: i32, status_buffer: *const u8) -> windows::core::HRESULT; +} + +/// Concrete `INmxSvcCallback` implementation that forwards inbound +/// callbacks into a tokio mpsc. The implementing struct holds an +/// [`mpsc::UnboundedSender`]; each inbound call copies +/// the buffer and pushes a [`CallbackEvent::CallbackInvoked`] event +/// (matching the shape the hand-rolled `CallbackExporter` produces). +#[windows_core::implement(INmxSvcCallback)] +pub struct DcomCallbackSink { + event_tx: mpsc::UnboundedSender, +} + +impl DcomCallbackSink { + /// Construct a new sink. The returned `Self` is a Rust value; + /// convert to an `IUnknown` for marshalling via + /// `IUnknown::from(sink)` (the conversion impl is generated by + /// the `#[implement]` macro). + #[must_use] + pub fn new(event_tx: mpsc::UnboundedSender) -> Self { + Self { event_tx } + } + + fn forward(&self, opnum: u16, buffer_size: i32, buffer: *const u8) { + let body: Vec = if buffer_size <= 0 || buffer.is_null() { + Vec::new() + } else { + // SAFETY: the COM stub guarantees `buffer` is valid for + // `buffer_size` bytes for the duration of the call, and + // the slice is read-only. We copy out before returning. + unsafe { std::slice::from_raw_parts(buffer, buffer_size as usize) }.to_vec() + }; + if let Err(e) = self.event_tx.send(CallbackEvent::CallbackInvoked { opnum, body }) { + // The receiver was dropped (the upstream router + // probably exited). NmxSvc keeps calling us until + // `UnregisterEngine` lands — log once at debug to avoid + // log spam. + debug!("DcomCallbackSink: dropped event for opnum {opnum} (rx closed): {e}"); + } + } +} + +impl INmxSvcCallback_Impl for DcomCallbackSink_Impl { + unsafe fn DataReceivedRaw( + &self, + buffer_size: i32, + data_buffer: *const u8, + ) -> windows::core::HRESULT { + // Opnum 3 per `NmxProcedureMetadata.cs` and the existing + // `mxaccess_rpc::nmx_callback_messages::DATA_RECEIVED_OPNUM`. + self.forward(3, buffer_size, data_buffer); + windows::Win32::Foundation::S_OK + } + + unsafe fn StatusReceivedRaw( + &self, + buffer_size: i32, + status_buffer: *const u8, + ) -> windows::core::HRESULT { + // Opnum 4. + self.forward(4, buffer_size, status_buffer); + windows::Win32::Foundation::S_OK + } +} + +/// Build a DCOM-managed callback sink, marshal it for cross-machine +/// dispatch, and return the bundle of: +/// 1. an [`IUnknownHolder`] — keeps the COM ref alive for the +/// consumer's lifetime (see `IUnknownHolder` doc on why this +/// matters), +/// 2. an `mpsc::UnboundedReceiver` — drained by the +/// upstream `callback_router` (the same shape the hand-rolled +/// `CallbackExporter::bind` returns), +/// 3. the OBJREF byte blob — passed to `RegisterEngine2` as the +/// callback parameter. +/// +/// Mirrors `MxNativeSession.CreateRegisteredService` (`cs:624`): +/// ```csharp +/// byte[] callbackObjRef = ComObjRefProvider.MarshalInterfaceObjRef( +/// callback, +/// NmxProcedureMetadata.INmxSvcCallback, +/// ComObjRefProvider.MarshalContextDifferentMachine); +/// ``` +/// +/// # Errors +/// +/// Surfaces `windows::core::Error` for any failure in the `IStream` +/// allocation, `CoMarshalInterface`, `GetHGlobalFromStream`, or +/// `GlobalLock` chain. +pub fn create_dcom_callback_sink_objref() -> Result< + ( + IUnknownHolder, + mpsc::UnboundedReceiver, + Vec, + ), + windows::core::Error, +> { + mxaccess_rpc::com_objref_provider::ensure_apartment().map_err(|e| { + warn!("ensure_apartment failed: {e:?}"); + windows::core::Error::from_hresult(windows::Win32::Foundation::E_FAIL) + })?; + + let (event_tx, event_rx) = mpsc::unbounded_channel(); + let sink = DcomCallbackSink::new(event_tx); + let unknown: IUnknown = sink.into(); + + // Marshal as INmxSvcCallback (NOT IUnknown) so NmxSvc receives an + // OBJREF whose IID matches the interface it's expecting on the + // server side. The .NET reference does the same at + // `MxNativeSession.cs:624` — pass `NmxProcedureMetadata.INmxSvcCallback`. + let blob = marshal_for_dcom(&unknown, INMX_SVC_CALLBACK_IID)?; + + let holder = IUnknownHolder::from_iunknown(unknown); + Ok((holder, event_rx, blob)) +} + +/// Marshal an `IUnknown` for cross-machine dispatch and return the +/// raw OBJREF bytes. Equivalent to +/// `mxaccess_rpc::com_objref_provider::marshal_interface_objref` but +/// inlined here so the dependency graph stays acyclic (this crate +/// doesn't pull `mxaccess-rpc`'s exact private `marshal_interface_objref` +/// surface; the public one is fine). +fn marshal_for_dcom(unknown: &IUnknown, iid: GUID) -> Result, windows::core::Error> { + // SAFETY: The Win32 COM call sequence below is a textbook OBJREF + // production: + // 1. CreateStreamOnHGlobal allocates an HGlobal-backed IStream. + // 2. CoMarshalInterface writes the OBJREF into the stream. + // 3. GetHGlobalFromStream extracts the underlying handle. + // 4. GlobalLock / GlobalSize / GlobalUnlock copy out the bytes. + // Each call's HRESULT is checked. + unsafe { + let stream: IStream = CreateStreamOnHGlobal( + windows::Win32::Foundation::HGLOBAL(ptr::null_mut()), + true, + )?; + CoMarshalInterface( + &stream, + &iid, + unknown, + MSHCTX_DIFFERENTMACHINE.0 as u32, + None, + MSHLFLAGS_NORMAL.0 as u32, + )?; + let hglobal = GetHGlobalFromStream(&stream)?; + let size = GlobalSize(hglobal); + if size == 0 { + return Ok(Vec::new()); + } + let ptr = GlobalLock(hglobal); + if ptr.is_null() { + return Err(windows::core::Error::from_hresult( + windows::Win32::Foundation::E_FAIL, + )); + } + let slice = std::slice::from_raw_parts(ptr.cast::(), size); + let blob = slice.to_vec(); + let _ = GlobalUnlock(hglobal); // best-effort; lock count drops to 0 + Ok(blob) + } +} diff --git a/rust/crates/mxaccess-callback/src/lib.rs b/rust/crates/mxaccess-callback/src/lib.rs index cc11588..54ad994 100644 --- a/rust/crates/mxaccess-callback/src/lib.rs +++ b/rust/crates/mxaccess-callback/src/lib.rs @@ -12,8 +12,23 @@ //! Plus the `IRemUnknown::RemQueryInterface` handler that completes the //! server-side handshake against our exported OBJREF (DoD condition for M2). -#![forbid(unsafe_code)] +// `forbid(unsafe_code)` lifted: the F55 / Path A `dcom_sink` module +// (gated behind `windows-com`) implements an `INmxSvcCallback` COM +// class that must dereference stub-side buffer pointers in +// `DataReceivedRaw` / `StatusReceivedRaw`. Each unsafe block carries +// a SAFETY comment documenting the COM stub's buffer-validity +// contract. +#![deny(unsafe_op_in_unsafe_fn)] pub mod exporter; pub use exporter::{CallbackEvent, CallbackExporter, ExporterIdentities, IUNKNOWN_IID}; + +/// Path A — DCOM-managed `INmxSvcCallback` sink. Required because +/// NmxSvc rejects hand-rolled OBJREFs from [`exporter::CallbackExporter`] +/// with `RPC_S_SERVER_UNAVAILABLE` (1722) on RegisterEngine2 — see F55. +#[cfg(all(windows, feature = "windows-com"))] +pub mod dcom_sink; + +#[cfg(all(windows, feature = "windows-com"))] +pub use dcom_sink::{create_dcom_callback_sink_objref, INMX_SVC_CALLBACK_IID}; diff --git a/rust/crates/mxaccess-rpc/Cargo.toml b/rust/crates/mxaccess-rpc/Cargo.toml index 95ad2de..fc465d9 100644 --- a/rust/crates/mxaccess-rpc/Cargo.toml +++ b/rust/crates/mxaccess-rpc/Cargo.toml @@ -27,7 +27,7 @@ subtle = "2" # / CoCreateInstance / CoMarshalInterface, Win32_System_Memory for # GlobalLock / GlobalSize, Win32_System_Ole for the historical # CreateStreamOnHGlobal / GetHGlobalFromStream re-exports. -windows = { version = "0.59", features = [ +windows = { version = "0.62", features = [ "Win32_Foundation", "Win32_System_Com", "Win32_System_Com_Marshal", diff --git a/rust/crates/mxaccess-rpc/src/com_objref_provider.rs b/rust/crates/mxaccess-rpc/src/com_objref_provider.rs index 3335297..ad95baa 100644 --- a/rust/crates/mxaccess-rpc/src/com_objref_provider.rs +++ b/rust/crates/mxaccess-rpc/src/com_objref_provider.rs @@ -129,7 +129,7 @@ pub enum ProviderError { /// which we accept. If a thread is already initialised to STA we receive /// `RPC_E_CHANGED_MODE` — also treated as success (the existing apartment /// is fine for `CoMarshalInterface`). -fn ensure_apartment() -> Result<(), ProviderError> { +pub fn ensure_apartment() -> Result<(), ProviderError> { thread_local! { // `OnceLock` per thread guarantees we only attempt CoInitializeEx // once per worker; subsequent calls are a no-op. @@ -270,6 +270,18 @@ pub struct IUnknownHolder { inner: IUnknown, } +impl IUnknownHolder { + /// Wrap an existing `IUnknown` into a holder. Used by callers + /// (e.g. `mxaccess-callback::dcom_sink`) that have an `IUnknown` + /// from a `windows-rs` `#[implement]` cast and need to keep the + /// COM ref alive for the same Path-A reasons documented at the + /// type level. + #[must_use] + pub fn from_iunknown(inner: IUnknown) -> Self { + Self { inner } + } +} + impl std::fmt::Debug for IUnknownHolder { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("IUnknownHolder").finish_non_exhaustive() diff --git a/rust/crates/mxaccess/Cargo.toml b/rust/crates/mxaccess/Cargo.toml index 28edf2b..1611d30 100644 --- a/rust/crates/mxaccess/Cargo.toml +++ b/rust/crates/mxaccess/Cargo.toml @@ -45,7 +45,7 @@ serde = ["mxaccess-codec/serde"] live = [] # Pulls F12's `Session::connect_nmx_auto` constructor — the auto-resolving # COM-activation path. Propagates to `mxaccess-nmx/windows-com`. -windows-com = ["mxaccess-nmx/windows-com"] +windows-com = ["mxaccess-nmx/windows-com", "mxaccess-callback/windows-com"] [lints] workspace = true diff --git a/rust/crates/mxaccess/src/session.rs b/rust/crates/mxaccess/src/session.rs index 7fb65ee..39e96d8 100644 --- a/rust/crates/mxaccess/src/session.rs +++ b/rust/crates/mxaccess/src/session.rs @@ -32,7 +32,19 @@ use std::sync::Arc; use std::time::SystemTime; -use mxaccess_callback::{CallbackEvent, CallbackExporter, ExporterIdentities}; +use mxaccess_callback::{CallbackEvent, CallbackExporter}; +// `ExporterIdentities` is only used by the legacy hand-rolled +// CallbackExporter path. Path A (windows-com) skips it entirely. +#[cfg(not(all(windows, feature = "windows-com")))] +use mxaccess_callback::ExporterIdentities; +// F55 / Path A — DCOM-managed `INmxSvcCallback` sink. Only used when +// `windows-com` is on; without it the hand-rolled `CallbackExporter` +// path stays in place (it's still useful for unit tests that exercise +// the exporter against a fake NMX peer in-process). The DCOM sink is +// the path that survives NmxSvc's SCM-side OXID validation against the +// live AVEVA install — see `mxaccess_callback::dcom_sink` for context. +#[cfg(all(windows, feature = "windows-com"))] +use mxaccess_rpc::com_objref_provider::IUnknownHolder; use mxaccess_codec::{ MxStatus, NmxOperationStatusMessage, NmxReferenceRegistrationMessage, NmxSubscriptionMessage, NmxSubscriptionRecord, @@ -40,7 +52,11 @@ use mxaccess_codec::{ use mxaccess_galaxy::{GalaxyTagMetadata, Resolver, ResolverError}; use mxaccess_nmx::{NmxClient, NmxClientError, WriteValue}; use mxaccess_rpc::guid::Guid; -use mxaccess_rpc::ntlm::{NtlmClientContext, local_hostname}; +use mxaccess_rpc::ntlm::NtlmClientContext; +// Same as `ExporterIdentities` above — only the legacy exporter path +// derives the OBJREF host from `local_hostname()`. +#[cfg(not(all(windows, feature = "windows-com")))] +use mxaccess_rpc::ntlm::local_hostname; use mxaccess_rpc::transport::TransportError; use std::collections::{HashMap, VecDeque}; use std::net::SocketAddr; @@ -598,6 +614,20 @@ pub struct SessionInner { /// dictionaries (`MxNativeSession.cs` field-level comments) plus /// the ordered list those dictionaries are consulted against. pub(crate) pending_ops: Arc>, + /// F55 / Path A — keeps the DCOM-managed `INmxSvcCallback`'s + /// `IUnknown` ref alive for the session's lifetime. The marshalled + /// OBJREF passed to `RegisterEngine2` references this object's + /// OXID/IPID via the SCM's stub manager; once the last `IUnknown` + /// ref drops, the stub is torn down and inbound NmxSvc callbacks + /// fail to dispatch. `None` when the legacy `CallbackExporter` + /// path is in use (no `windows-com` feature) or after `shutdown_nmx` + /// drops the ref. + /// + /// Mirrors the .NET reference's `MxNativeSession._callbackSink` + /// field (`MxNativeSession.cs`), which holds the `NmxCallbackSink` + /// instance for the same reason. + #[cfg(all(windows, feature = "windows-com"))] + pub(crate) dcom_sink_holder: Mutex>, } /// FIFO-ordered registry of outstanding NMX operations waiting for an @@ -908,35 +938,67 @@ impl Session { options: SessionOptions, resolver: Arc, ) -> Result { - // 1. Bind a local CallbackExporter on an OS-assigned ephemeral - // port, then build the OBJREF advertising it. Hostname comes - // from `local_hostname()` (env-var lookup); falls back to - // `127.0.0.1` when neither `COMPUTERNAME` nor `HOSTNAME` is - // set so the OBJREF binding is always parseable as - // "[]". - let identities = ExporterIdentities::random(); - // Bind on UNSPECIFIED (`0.0.0.0`) so the listener accepts - // dial-backs on every interface NmxSvc could resolve the - // hostname to. The OBJREF's host string is the machine's - // `COMPUTERNAME` (or `127.0.0.1` fallback), and NmxSvc - // resolves that via DNS — which on a typical AVEVA install - // returns the machine's primary NIC IP, not loopback. If the - // exporter binds only on `127.0.0.1`, the dial-back lands on - // a different interface and the TCP SYN is dropped, surfacing - // as `RegisterEngine2 → Fault(0x800706BA RPC_S_SERVER_UNAVAILABLE)` - // because NmxSvc can't reach our exporter to negotiate the - // callback bind. Binding on UNSPECIFIED (= bind to all v4 - // interfaces, including loopback + primary NIC) avoids this. - let exporter_addr = - SocketAddr::new(std::net::IpAddr::V4(std::net::Ipv4Addr::UNSPECIFIED), 0); - let (exporter, callback_events) = CallbackExporter::bind(exporter_addr, identities) - .await - .map_err(Error::Io)?; - let hostname = match local_hostname() { - s if s.is_empty() => "127.0.0.1".to_string(), - s => s, + // 1. Build the callback sink + OBJREF. + // + // Two paths exist: + // + // - F55 / Path A (`windows-com` feature): Build a DCOM-managed + // `INmxSvcCallback` instance via `windows-rs` `#[implement]`, + // marshal it through `CoMarshalInterface`. This registers the + // sink's OXID with the local RPCSS so NmxSvc's SCM-side + // `IObjectExporter::ResolveOxid` validation passes — the + // hand-rolled OBJREF below fails this check with + // `RPC_S_SERVER_UNAVAILABLE` (1722) on `RegisterEngine2`. + // Mirrors `MxNativeSession.CreateRegisteredService` which + // calls `ComObjRefProvider.MarshalInterfaceObjRef(callback, + // INmxSvcCallback, DifferentMachine)` + // (`src/MxNativeClient/MxNativeSession.cs:624`). + // + // - Legacy hand-rolled exporter (no `windows-com`): Bind a + // local TCP listener that serves `IRemUnknown` + + // `INmxSvcCallback` directly, then advertise it via a + // custom OBJREF. Useful for unit tests that exercise the + // exporter against a fake NMX peer in-process. NOT used + // against a real NmxSvc — see F55 in `design/followups.md`. + #[cfg(all(windows, feature = "windows-com"))] + let (exporter, dcom_sink_holder, callback_events, callback_obj_ref) = { + let (holder, events, blob) = mxaccess_callback::create_dcom_callback_sink_objref() + .map_err(|e| { + Error::Connection(ConnectionError::TransportFailure { + detail: format!("DCOM callback sink marshal failed: {e:?}"), + }) + })?; + (None::, Some(holder), events, blob) + }; + + #[cfg(not(all(windows, feature = "windows-com")))] + let (exporter, callback_events, callback_obj_ref) = { + let identities = ExporterIdentities::random(); + // Bind on UNSPECIFIED (`0.0.0.0`) so the listener accepts + // dial-backs on every interface NmxSvc could resolve the + // hostname to. The OBJREF's host string is the machine's + // `COMPUTERNAME` (or `127.0.0.1` fallback), and NmxSvc + // resolves that via DNS — which on a typical AVEVA install + // returns the machine's primary NIC IP, not loopback. If + // the exporter binds only on `127.0.0.1`, the dial-back + // lands on a different interface and the TCP SYN is + // dropped, surfacing as `RegisterEngine2 → Fault(0x800706BA + // RPC_S_SERVER_UNAVAILABLE)` because NmxSvc can't reach + // our exporter to negotiate the callback bind. Binding on + // UNSPECIFIED (= bind to all v4 interfaces, including + // loopback + primary NIC) avoids this. + let exporter_addr = + SocketAddr::new(std::net::IpAddr::V4(std::net::Ipv4Addr::UNSPECIFIED), 0); + let (exporter, events) = CallbackExporter::bind(exporter_addr, identities) + .await + .map_err(Error::Io)?; + let hostname = match local_hostname() { + s if s.is_empty() => "127.0.0.1".to_string(), + s => s, + }; + let blob = exporter.create_callback_objref(&hostname); + (Some(exporter), events, blob) }; - let callback_obj_ref = exporter.create_callback_objref(&hostname); // 2. Spawn the router task that broadcasts parsed callback // messages. @@ -996,7 +1058,7 @@ impl Session { options, resolver, nmx: Mutex::new(nmx), - callback_exporter: Mutex::new(Some(exporter)), + callback_exporter: Mutex::new(exporter), callback_tx, operation_status_tx, recovery_active, @@ -1007,6 +1069,8 @@ impl Session { callback_obj_ref, rebuild_factory: Mutex::new(None), pending_ops, + #[cfg(all(windows, feature = "windows-com"))] + dcom_sink_holder: Mutex::new(dcom_sink_holder), }), }) } @@ -2102,6 +2166,15 @@ impl Session { if let Some(exp) = self.inner.callback_exporter.lock().await.take() { exp.shutdown().await; } + // F55 / Path A — drop the DCOM-managed sink's IUnknown ref so + // CoMarshalInterface's stub manager unregisters our OXID from + // RPCSS. Mirrors the .NET reference's `MxNativeSession.Dispose` + // path, which lets the `NmxCallbackSink` go out of scope after + // unregister. + #[cfg(all(windows, feature = "windows-com"))] + { + self.inner.dcom_sink_holder.lock().await.take(); + } // 3. Wait for the router task. Once the exporter is dropped its // upstream mpsc::Sender closes, the router's recv() returns @@ -2403,10 +2476,16 @@ mod tests { // matches production. Tests don't drive real callbacks through // this path, but keeping the shape symmetric means // shutdown_nmx exercises the full cleanup chain. - let (exporter, callback_events) = - CallbackExporter::bind("127.0.0.1:0".parse().unwrap(), ExporterIdentities::random()) - .await - .unwrap(); + // Test-only helper exercises the legacy hand-rolled CallbackExporter + // path even when the crate is built with `windows-com`. The DCOM + // path needs a real NmxSvc on the wire; this shim talks to a + // `loopback_listener::expect_*` peer. + let (exporter, callback_events) = CallbackExporter::bind( + "127.0.0.1:0".parse().unwrap(), + mxaccess_callback::ExporterIdentities::random(), + ) + .await + .unwrap(); let (callback_tx, _) = broadcast::channel(CALLBACK_BROADCAST_CAPACITY); let (operation_status_tx, _) = broadcast::channel::>(OPERATION_STATUS_BROADCAST_CAPACITY); @@ -2437,6 +2516,8 @@ mod tests { callback_obj_ref: Vec::new(), rebuild_factory: Mutex::new(None), pending_ops, + #[cfg(all(windows, feature = "windows-com"))] + dcom_sink_holder: Mutex::new(None), }), }) }