feat(picky-krb): add IAKerb proxy message encoding/decoding - #531
Benoît Cortier (CBenoit) merged 3 commits into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Decoding does not enforce outer framing boundaries or handle valid unknown header extensions.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (3)
What changed in this PR
Adds IAKERB proxy message support for downstream Kerberos proxy integrations.
Changes:
- Adds IAKERB headers, constants, error codes, and mechanism OID.
- Implements proxy message encoding/decoding.
- Adds DER round-trip tests.
| File | Description |
|---|---|
picky-krb/src/messages.rs |
Defines the IAKERB header and tests. |
picky-krb/src/gss_api.rs |
Implements proxy token encoding and decoding. |
picky-krb/src/constants.rs |
Adds IAKERB token and error constants. |
picky-asn1-x509/src/oids.rs |
Registers the IAKERB mechanism OID. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| let Asn1RawDer(raw) = Asn1RawDer::deserialize(deserializer)?; | ||
| let IAKerbHeaderHelper { | ||
| target_realm, | ||
| cookie, | ||
| flags, | ||
| } = picky_asn1_der::from_bytes(&raw) | ||
| .map_err(|err| D::Error::custom(format!("Cannot deserialize IAKerbHeader: {err:?}")))?; |
There was a problem hiding this comment.
Parse the sequence by tag instead, propagating errors for known [2]/[3] fields and skipping only genuinely unknown extension tags.
This requires manual deserialization implementation. I don't think that's a good idea.
It would be better to fix the Optional type so that it returns an error if a value is present but malformed. But this should be done in a separate PR, since it touches many structures.
There was a problem hiding this comment.
Indeed. I think that the picky-asn1-der abstractions are not very good, and I’m considering a migration to the der crate instead of using serde for what it’s not good at. For now, I’ll merge as is, but I would gladly accept separate PRs to improve this. Thanks!
Benoît Cortier (CBenoit)
left a comment
Benoît Cortier (CBenoit)
left a comment
There was a problem hiding this comment.
Thank you for contributing this. LGTM!
|
Looks like merge is blocked on the code formatting, can you just run |
6f98044
into
Devolutions:master
This PR implements encoding and decoding of the
IAKERB_PROXYmessage (IAKrbProxyMessage) according to the IAKERB specification. The implementation is also covered with unit-tests.What is IAKERB
IAKERB extends Kerberos to support scenarios where the client cannot directly access the KDC. Instead, KDC messages are encapsulated in GSS-API tokens and exchanged through an IAKERB proxy. The server forwards these messages to the LocalKDC, allowing the client to obtain the required Kerberos tickets without direct network access to the KDC.
Microsoft recently introduced IAKERB support in Windows Insider builds as part of its effort to reduce NTLM dependency: https://techcommunity.microsoft.com/blog/windows-itpro-blog/reducing-ntlm-dependency-iakerb-and-localkdc-in-windows-insider-preview/4524615.
Related PRs
KdcResolutionenum instead of KDC url IronRDP#1987