feat(kdc): add initial KDC implementation #541

TheBestTvarynka · 2025-11-12T16:27:34Z

Hi,

I added a simple KDC implementation.

This KDC implementation will be used by the Devolutions-Gateway to do the Kerberos credentials-injection.

First, I planned to place it in the Devolutions-Gateway's repo, but then I noticed that it requires ASN1-related picky-* deps and I didn't want to add them to Devolution-Gateway.
I decided to placed it in the crates/kdc. This way, we will keep all Kerberos-related implementations in one place, with the identical dependency versions.

I added many comments and references to the RFC, so it should not be difficult to review this PR.

CBenoit · 2025-11-13T07:59:35Z

crates/kdc/Cargo.toml

+picky-asn1 = { workspace = true, features = ["time_conversion"] }
+picky-asn1-der.workspace = true
+picky-asn1-x509.workspace = true


I’m really sorry to say that now, but we’ll stop using the picky crates soon in the future in favor of using the der crate from RustCrypto. Still, I think it’s fine to merge this code now that it’s here, but we should stop writing any new code using this infrastructure from now on. If it’s not too much work for you, you may consider changing that before we merge.

Should we create a task on our side for migrating the sspi-rs to der crate from RustCrypto?

If it’s not too much work for you, you may consider changing that before we merge.

To remove picky-asn1-* usage in the kdc implementation, we need to migrate the picky-krb crate to der-based encoding/decoding. It will significantly delay the credentials-injection feature.

I would like to address it in a separate PR. We need to refactor the picky-krb first.

I see! In that case there was no choice for this crate. Please do not refactor immediately though, we need to establish priority first. Shipping the credentials injection feature is more important. My only request for now is to avoid using picky-asn1-* for new code unless absolutely required. I’ll send a follow up on Slack in the next days.

CBenoit · 2025-11-13T08:01:12Z

crates/kdc/Cargo.toml

+picky-asn1 = { workspace = true, features = ["time_conversion"] }
+picky-asn1-der.workspace = true
+picky-asn1-x509.workspace = true
+picky-krb.workspace = true


For picky-krb it’s fine to keep it, but ultimately we’ll move it into sspi-rs, and migrate away from the picky infrastructure for serialization/deserialization.

CBenoit · 2025-11-13T20:30:55Z

crates/kdc/src/lib.rs

+        let from = OffsetDateTime::try_from(from.0.clone())
+            .map_err(|err| KdcError::NeverValid(format!("KdcReq::from time is not valid: {err}")))?;
+        // RFC (https://www.rfc-editor.org/rfc/rfc4120#section-3.1.3):
+        // > If the requested starttime is absent, indicates a time in the past,


Suggested change

// > If the requested starttime is absent, indicates a time in the past,

// > If the requested start time is absent, indicates a time in the past,

I copied this sentence directly from the RFC. Should I change it, though?

I think it will end up annoying if we use typos-cli at some point in the future 🤔

Copilot

Pull Request Overview

This PR adds a new Key Distribution Center (KDC) implementation to support Kerberos credentials injection in Devolutions-Gateway. The implementation follows RFC 4120 and includes both Authentication Service (AS) and Ticket-Granting Service (TGS) exchanges.

Implements a complete KDC mimicking functionality for Kerberos authentication
Supports both AS-REQ/AS-REP and TGS-REQ/TGS-REP message exchanges
Includes comprehensive error handling with proper Kerberos error code mapping
Uses secure AES256-CTS-HMAC-SHA1-96 encryption by default for ticket encryption

Reviewed Changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
Cargo.toml	Adds the new `kdc` crate to the workspace members
Cargo.lock	Updates lockfile with new dependencies (argon2, blake2, password-hash) for the KDC crate
crates/kdc/Cargo.toml	Defines the KDC crate with necessary dependencies for Kerberos operations
crates/kdc/src/config.rs	Defines configuration structures for the KDC server including user credentials and realm settings
crates/kdc/src/error.rs	Implements KDC error types and conversion to Kerberos error messages
crates/kdc/src/lib.rs	Main entry point handling KDC proxy messages and routing to AS/TGS handlers
crates/kdc/src/ticket.rs	Implements ticket generation and encryption according to RFC 4120
crates/kdc/src/as_exchange.rs	Handles Authentication Service (AS-REQ/AS-REP) exchange for initial ticket granting
crates/kdc/src/tgs_exchange.rs	Handles Ticket-Granting Service (TGS-REQ/TGS-REP) exchange for service tickets

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-13T20:32:47Z

crates/kdc/src/ticket.rs

+        sname: ExplicitContextTag10::from(sname),
+        // RFC (https://datatracker.ietf.org/doc/html/rfc4120#section-3.1.3):
+        // > ...It then formats a KRB_AS_REP message, copying the addresses in the request into the caddr of the response...
+        caadr: Optional::from(addresses.map(|addresses| ExplicitContextTag11::from(Asn1SequenceOf::from(addresses.0)))),


Spelling error: field name caadr should be caddr (client addresses). This appears to be a typo in the field name.

Suggested change

caadr: Optional::from(addresses.map(|addresses| ExplicitContextTag11::from(Asn1SequenceOf::from(addresses.0)))),

caddr: Optional::from(addresses.map(|addresses| ExplicitContextTag11::from(Asn1SequenceOf::from(addresses.0)))),

It needs to be fixed on the picky-krb side first. I will submit a PR soon

Copilot · 2025-11-13T20:32:47Z

crates/kdc/src/error.rs

+                name_string: ExplicitContextTag1::from(Asn1SequenceOf::from(vec![
+                    KerberosStringAsn1::from(
+                        IA5String::from_string(TGT_SERVICE_NAME.to_owned())
+                            .expect("TBT_SERVICE_NAME is valid KerberosString"),


Spelling error in comment: "TBT_SERVICE_NAME" should be "TGT_SERVICE_NAME" (TGT, not TBT).

Suggested change

.expect("TBT_SERVICE_NAME is valid KerberosString"),

.expect("TGT_SERVICE_NAME is valid KerberosString"),

Copilot · 2025-11-13T20:32:48Z

crates/kdc/src/tgs_exchange.rs

+    if msg_type.0.0 != [TGS_REQ_MSG_TYPE] {
+        return Err(KdcError::BadMsgType {
+            msg_type: msg_type.0.0.clone(),
+            expected: AS_REQ_MSG_TYPE,


Bug: The error message expects AS_REQ_MSG_TYPE but this is a TGS request handler, so it should expect TGS_REQ_MSG_TYPE. The comparison on line 162 correctly checks for TGS_REQ_MSG_TYPE, but the error on line 165 incorrectly references AS_REQ_MSG_TYPE. Change expected: AS_REQ_MSG_TYPE to expected: TGS_REQ_MSG_TYPE.

Suggested change

expected: AS_REQ_MSG_TYPE,

expected: TGS_REQ_MSG_TYPE,

Copilot · 2025-11-13T20:32:48Z

crates/kdc/src/lib.rs

+    // > If the requested expiration time minus the starttime (as determined above) is less than a site-determined minimum lifetime,
+    // > an error message with code KDC_ERR_NEVER_VALID is returned.
+    //
+    // We do not have a ticket minimum lifetime value configured, so we only check that the `end_time`` is after the `auth_time`.


Spelling error: Double backtick in `end_time should be a single backtick on each side: end_time ``.

Suggested change

// We do not have a ticket minimum lifetime value configured, so we only check that the `end_time`` is after the `auth_time`.

// We do not have a ticket minimum lifetime value configured, so we only check that the `end_time` is after the `auth_time`.

Copilot · 2025-11-13T20:32:48Z

crates/kdc/src/tgs_exchange.rs

+    tgs_rep_key_usage: i32,
+}
+
+/// Performs TGS pre-authentication: validated incoming PA-DATAs and extract needed parameters.


Documentation typo: "validated" should be "validates" to match standard function documentation style (third-person present tense).

Suggested change

/// Performs TGS pre-authentication: validated incoming PA-DATAs and extract needed parameters.

/// Performs TGS pre-authentication: validates incoming PA-DATAs and extracts needed parameters.

CBenoit · 2025-11-13T20:38:59Z

Excellent job, let’s just fix the minor typos etc before merging 👍

CBenoit · 2025-11-18T19:41:28Z

LGTM! I’ll merge after the fix from Allan for smartcards + WASM 👍

CBenoit

LGTM! Just need one last rebase 😉
I enabled auto-merge.

CBenoit · 2025-11-18T20:34:41Z

crates/kdc/Cargo.toml

+name = "kdc"
+version = "0.1.0"
+edition = "2024"
+


note: If you don’t want to publish the initial version right now, you should add publish = false here (and remove that later when publishing is intended)

TheBestTvarynka self-assigned this Nov 12, 2025

TheBestTvarynka requested a review from CBenoit November 12, 2025 16:34

TheBestTvarynka marked this pull request as ready for review November 12, 2025 16:34

CBenoit reviewed Nov 13, 2025

View reviewed changes

CBenoit requested a review from Copilot November 13, 2025 20:29

Copilot started reviewing on behalf of CBenoit November 13, 2025 20:30 View session

CBenoit reviewed Nov 13, 2025

View reviewed changes

Copilot finished reviewing on behalf of CBenoit November 13, 2025 20:31

Copilot AI reviewed Nov 13, 2025

View reviewed changes

TheBestTvarynka mentioned this pull request Nov 14, 2025

refactor(picky-krb): fix typo in EncKdcRepPart Devolutions/picky-rs#434

Merged

CBenoit approved these changes Nov 18, 2025

View reviewed changes

CBenoit enabled auto-merge (squash) November 18, 2025 20:32

CBenoit reviewed Nov 18, 2025

View reviewed changes

TheBestTvarynka added 3 commits November 20, 2025 22:58

feat(kdc): add initial KDC implementation;

08d5e75

feat(kdc): export config module;

3c165d2

refactor(kdc): fix typos;

f8d0755

TheBestTvarynka force-pushed the feat/kdc-impl branch from d9178fc to f8d0755 Compare November 20, 2025 20:58

CBenoit merged commit 10ed474 into master Nov 20, 2025
63 checks passed

CBenoit deleted the feat/kdc-impl branch November 20, 2025 21:06

devolutionsbot mentioned this pull request Nov 20, 2025

chore(release): prepare for publishing #549

Open

	// > If the requested starttime is absent, indicates a time in the past,
	// > If the requested start time is absent, indicates a time in the past,

	caadr: Optional::from(addresses.map(\|addresses\| ExplicitContextTag11::from(Asn1SequenceOf::from(addresses.0)))),
	caddr: Optional::from(addresses.map(\|addresses\| ExplicitContextTag11::from(Asn1SequenceOf::from(addresses.0)))),

	.expect("TBT_SERVICE_NAME is valid KerberosString"),
	.expect("TGT_SERVICE_NAME is valid KerberosString"),

	// We do not have a ticket minimum lifetime value configured, so we only check that the `end_time`` is after the `auth_time`.
	// We do not have a ticket minimum lifetime value configured, so we only check that the `end_time` is after the `auth_time`.

	/// Performs TGS pre-authentication: validated incoming PA-DATAs and extract needed parameters.
	/// Performs TGS pre-authentication: validates incoming PA-DATAs and extracts needed parameters.

feat(kdc): add initial KDC implementation #541

feat(kdc): add initial KDC implementation #541

Uh oh!

Conversation

TheBestTvarynka commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CBenoit Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CBenoit Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CBenoit Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CBenoit commented Nov 13, 2025

Uh oh!

CBenoit commented Nov 18, 2025

Uh oh!

CBenoit left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

TheBestTvarynka commented Nov 12, 2025 •

edited

Loading

CBenoit Nov 13, 2025 •

edited

Loading

CBenoit Nov 13, 2025 •

edited

Loading

CBenoit Nov 13, 2025 •

edited

Loading