Skip to content

build(connector)!: update sspi to 0.23 and picky to rc.26 - #2080

Open
Suresh Kumar Ramu (sureshkumarramu) wants to merge 2 commits into
Devolutions:masterfrom
sureshkumarramu:build/sspi-0.23-stable-dalek
Open

Suresh Kumar Ramu (sureshkumarramu) wants to merge 2 commits into
Devolutions:masterfrom
sureshkumarramu:build/sspi-0.23-stable-dalek

Conversation

@sureshkumarramu

Copy link
Copy Markdown

sspi 0.21 and picky 7.0.0-rc.25 pin pre-release ed25519-dalek / curve25519-dalek. Cargo treats a pre-release and the stable release of the same major as one slot, so a dependency graph that already contains the stable crates cannot resolve ironrdp-connector at all. sspi 0.23 pins no dalek crate and picky rc.26 uses the stable ones.

TsRequest::buffer_len is fallible in sspi 0.23; the connector and the acceptor map its error like the encode step beside it. sspi is re-exported by the connector, so this is a breaking dependency change for consumers of its sspi types, and the manifest line now carries the public marker.

picky-krb is held at 0.12.4 in the lockfile because sspi 0.23.0 does not build against 0.12.5, published after it.

BREAKING CHANGE: ironrdp_connector::sspi is now sspi 0.23.

Issue: #1363
Co-authored-by: Claude Fable 5.1 noreply@anthropic.com

@github-actions github-actions Bot added automation-failed Exact-head automated classification or review failed or was unavailable risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny scope/core Touches the core architectural tier scope/ffi Affects native or .NET bindings size/M Size: up to 449 counted lines and 10 files; exceeds S in either measure labels Oct 6, 2026
@sureshkumarramu

Copy link
Copy Markdown
Author

The public API compatibility check fails inside sspi 0.23.0 itself, not in this change: the check resolves in a fresh workspace and takes picky-krb 0.12.5, which added GssApiMessageError::InvalidMechanismOid after sspi 0.23.0 was published. sspi-rs master handles the variant since 878be568 (#764), so the check should pass once the next sspi release is on crates.io. The workspace Cargo.lock here holds picky-krb at 0.12.4 for the same reason.

Verified locally on this branch: cargo fmt --check, cargo clippy --locked -D warnings on ironrdp-connector, ironrdp-acceptor and ffi, cargo check --locked -p ironrdp-mstsgu --features rustls,smartcard, and cargo test --locked -p ironrdp-testsuite-core (1727 passed).

Note

Human-tuned, LLM-assisted content.

@CBenoit

Copy link
Copy Markdown
Member

PR automation is failing because of a picky-krb 0.12.5 incompatibility, fixed on master by #2074. Please rebase on master to fix it.

@sureshkumarramu

Copy link
Copy Markdown
Author

Rebased on master (838cc42). The public-API check now fails before comparing anything: error: unsupported rustdoc format v61 from cargo-semver-checks 0.49.0, the same on every open PR today, so it looks like the pinned tool is behind the runner's toolchain rather than anything in this change.

@CBenoit

Copy link
Copy Markdown
Member

I think this should fix that one: #2088

@github-actions github-actions Bot added kind/protocol Affects RDP or related protocol behavior risk/medium Behavioral change that does not substantially alter a core public API and removed risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny automation-failed Exact-head automated classification or review failed or was unavailable labels Oct 7, 2026
`sspi 0.21` and `picky 7.0.0-rc.25` pin pre-release `ed25519-dalek` /
`curve25519-dalek`. Cargo treats a pre-release and the stable release of
the same major as one slot, so a dependency graph that already contains
the stable crates cannot resolve `ironrdp-connector` at all. `sspi 0.23`
pins no dalek crate and `picky rc.26` uses the stable ones.

`TsRequest::buffer_len` is fallible in `sspi 0.23`; the connector and
the acceptor map its error like the encode step beside it. `sspi` is
re-exported by the connector, so this is a breaking dependency change
for consumers of its `sspi` types, and the manifest line now carries the
public marker.

BREAKING CHANGE: `ironrdp_connector::sspi` is now `sspi 0.23`.

Issue: Devolutions#1363
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions github-actions Bot added breaking-change Includes a breaking change, and requires special scrutiny at the boundaries risk/high Substantial core public API impact, or fail-closed triage; needs maintainer-level scrutiny and removed risk/medium Behavioral change that does not substantially alter a core public API labels Oct 7, 2026
@sureshkumarramu

Copy link
Copy Markdown
Author

Rebased on master again (40902b4) after #2087 and #2088 landed; all checks pass now. Since picky-krb 0.12.5 is yanked and a fresh resolve takes 0.12.4, the lockfile no longer needs the pin and the commit message dropped that paragraph. Nothing else changed; ready for review.

@CBenoit Benoît Cortier (CBenoit) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@CBenoit
Benoît Cortier (CBenoit) enabled auto-merge (squash) October 8, 2026 11:46
@github-actions github-actions Bot added the needs-author-action The pull request author is the current next actor label Oct 8, 2026
`TsRequest::buffer_len` returns a `Result` in `sspi 0.23`. The native
CredSSP path in `ironrdp-vmconnect` builds only on Windows, so the
previous commit's Linux checks never compiled it; it now maps the error
like the encode step beside it, as the connector and the acceptor do.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
auto-merge was automatically disabled October 8, 2026 15:47

Head branch was pushed to by a user without write access

@sureshkumarramu

Copy link
Copy Markdown
Author

Windows CI failure: cause and fix

What failed: Checks [windows] stopped compiling ironrdp-vmconnect:

error[E0277]: the trait bound `usize: From<Result<u16, ironrdp_connector::sspi::Error>>` is not satisfied
  --> crates\ironrdp-vmconnect\src\native_credssp.rs:55:46

Cause: in sspi 0.23, TsRequest::buffer_len() returns Result<u16> instead of u16. This PR updated the two callers in ironrdp-connector and ironrdp-acceptor. It missed the third, in ironrdp-vmconnect's native CredSSP path, which only builds on Windows. I verified on Linux only, so that code was never compiled.

Fix (d90284f): one call site, handled the same way as the other two:

let mut encoded = Vec::with_capacity(usize::from(
    outgoing
        .buffer_len()
        .map_err(|error| custom_err!("native CredSSP request length", error))?,
));

A search for buffer_len() across crates/ and ffi/ finds no other caller left on the old signature.

Verified, cross-compiling for Windows (x86_64-pc-windows-gnu, toolchain 1.94.1 from rust-toolchain.toml):

  • cargo check --locked -p ironrdp-vmconnect: fails with the error above before the fix, and passes after it.
  • cargo clippy --locked -p ironrdp-vmconnect -- -D warnings: passes.
  • cargo fmt -p ironrdp-vmconnect -- --check: clean.

A correction: my earlier "all checks pass" was premature. The main CI run was still waiting for workflow approval, and only the automation checks had run. Sorry for the extra round.

The fix is pushed as a separate commit so the change is easy to see. I'm happy to squash it into the first commit if you prefer one.

@github-actions github-actions Bot removed the needs-author-action The pull request author is the current next actor label Oct 8, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Dependency upgrade (sspi 0.21->0.23, picky 7.0.0-rc.25->rc.26) with minimal code adaptation: three TsRequest::buffer_len() call sites (ironrdp-connector, ironrdp-acceptor, ironrdp-vmconnect) updated to handle the now-fallible sspi 0.23 signature by propagating an error via custom_err!, matching the adjacent encode_ts_request handling. Independently verified: all three call sites are the complete set of TsRequest::buffer_len() callers (the Windows-only vmconnect site fixed in d90284f included); custom_err! is in scope in all three files; no other sspi callers remain (acceptor lib.rs:200 uses the unrelated infallible EarlyUserAuthResult::buffer_len); the '# public' marker is correctly placed only on the connector, which re-exports sspi, while mstsgu, ironrdp, and ffi keep sspi private; lockfile transitions (pre-release dalek/ec crates to stable) are consistent with the upgrade and picky-krb is held at 0.12.4 consistent with the yanked 0.12.5. No PDU fields, constants, sequencing, or sec…

@github-actions github-actions Bot added ai-reviewed/1 One automated review completed needs-review A human reviewer is the current next actor labels Oct 9, 2026

This branch was successfully deployed

1 active deployment
llm-providers — d90284f5 Deployed Oct 8, 2026 by sureshkumarramu via Classify pull request #2135
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-reviewed/1 One automated review completed breaking-change Includes a breaking change, and requires special scrutiny at the boundaries kind/protocol Affects RDP or related protocol behavior needs-review A human reviewer is the current next actor risk/high Substantial core public API impact, or fail-closed triage; needs maintainer-level scrutiny scope/core Touches the core architectural tier scope/ffi Affects native or .NET bindings size/M Size: up to 449 counted lines and 10 files; exceeds S in either measure

Development

Successfully merging this pull request may close these issues.

2 participants