From 9eb60d5f88932410f71f80556c425a859459ca58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20=C5=9Al=C4=99zak?= Date: Wed, 16 Sep 2026 13:15:58 +0200 Subject: [PATCH 1/4] allow fido2 in register process --- build.rs | 8 ++++++++ proto | 2 +- src/handlers/register_mfa.rs | 36 ++++++++++++++++++++++++++++-------- 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/build.rs b/build.rs index b6c53e5..3ea10db 100644 --- a/build.rs +++ b/build.rs @@ -32,6 +32,14 @@ fn main() -> Result<(), Box> { "ClientMfaStartRequest.selected_methods", "#[serde(default)]", ) + // These are only ever sent when setting up a FIDO2 factor. Clients configuring a + // code factor leave them out of the request body entirely, so they cannot be + // required here just because protobuf lists them. + .field_attribute("CodeMfaSetupFinishRequest.name", "#[serde(default)]") + .field_attribute( + "CodeMfaSetupFinishRequest.fido2_attestation", + "#[serde(default)]", + ) // Protobuf enum values carry the enum name prefix to avoid package-scope // collisions, so the generated Rust variants all share a prefix that clippy // flags. Suppress it on the generated type. diff --git a/proto b/proto index cd1afb7..0a76a28 160000 --- a/proto +++ b/proto @@ -1 +1 @@ -Subproject commit cd1afb727774b51118c470fafbb5efd808e4d405 +Subproject commit 0a76a28870abbb3431c76824d0be7c4da380ed22 diff --git a/src/handlers/register_mfa.rs b/src/handlers/register_mfa.rs index d241e39..35c92f7 100644 --- a/src/handlers/register_mfa.rs +++ b/src/handlers/register_mfa.rs @@ -18,7 +18,7 @@ pub(crate) fn router() -> Router { .route("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/code/finish", post(register_code_mfa_finish)) } -/// Forwards a code MFA setup start to Core. +/// Forwards an MFA factor setup start to Core. /// /// `req.token` is either an enrollment token or an authorized MFA config session token. pub(super) async fn code_mfa_setup_start( @@ -26,8 +26,8 @@ pub(super) async fn code_mfa_setup_start( device_info: DeviceInfo, req: CodeMfaSetupStartRequest, ) -> Result, ApiError> { - debug!("Code MFA setup started"); - reject_non_code_method(req.method)?; + debug!("MFA factor setup started"); + reject_unsupported_method(req.method)?; let rx = state .grpc_server @@ -39,13 +39,13 @@ pub(super) async fn code_mfa_setup_start( } } -/// Forwards a code MFA setup finish to Core. See [`code_mfa_setup_start`] for the token. +/// Forwards an MFA factor setup finish to Core. See [`code_mfa_setup_start`] for the token. pub(super) async fn code_mfa_setup_finish( state: &AppState, device_info: DeviceInfo, req: CodeMfaSetupFinishRequest, ) -> Result, ApiError> { - reject_non_code_method(req.method)?; + reject_unsupported_method(req.method)?; let rx = state .grpc_server @@ -57,9 +57,13 @@ pub(super) async fn code_mfa_setup_finish( } } -/// Code MFA setup only knows how to deliver a code by email or TOTP. -fn reject_non_code_method(method: i32) -> Result<(), ApiError> { - if method == MfaMethod::Email as i32 || method == MfaMethod::Totp as i32 { +/// The factors Core's MFA setup can enable: a code by email or TOTP, or a FIDO2 +/// security key, whose proof is an attestation rather than a code. +fn reject_unsupported_method(method: i32) -> Result<(), ApiError> { + if method == MfaMethod::Email as i32 + || method == MfaMethod::Totp as i32 + || method == MfaMethod::Fido2 as i32 + { Ok(()) } else { error!("Requested method not supported"); @@ -67,6 +71,17 @@ fn reject_non_code_method(method: i32) -> Result<(), ApiError> { } } +/// The enrollment routes below carry neither a key name nor an attestation, so they +/// can only ever set up a code factor; FIDO2 has to go through MFA configuration. +fn reject_non_code_method(method: MfaMethod) -> Result<(), ApiError> { + if matches!(method, MfaMethod::Email | MfaMethod::Totp) { + Ok(()) + } else { + error!("Requested method not supported during enrollment"); + Err(ApiError::BadRequest("Method not supported.".to_string())) + } +} + #[derive(Debug, Clone, Deserialize)] struct RegisterMfaCodeStartRequest { pub method: MfaMethod, @@ -80,6 +95,7 @@ async fn register_code_mfa_start( Json(req): Json, ) -> Result, ApiError> { let token = enrollment_token(&cookie_jar)?; + reject_non_code_method(req.method)?; code_mfa_setup_start( &state, device_info, @@ -105,6 +121,7 @@ async fn register_code_mfa_finish( Json(req): Json, ) -> Result, ApiError> { let token = enrollment_token(&cookie_jar)?; + reject_non_code_method(req.method)?; code_mfa_setup_finish( &state, device_info, @@ -112,6 +129,9 @@ async fn register_code_mfa_finish( token, code: req.code, method: req.method as i32, + // FIDO2 only, and rejected above. + name: None, + fido2_attestation: None, }, ) .await From 4356601b267344827901382c25a0d7a5ef87fb5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20=C5=9Al=C4=99zak?= Date: Fri, 18 Sep 2026 10:18:22 +0200 Subject: [PATCH 2/4] bump proto --- proto | 2 +- src/handlers/register_mfa.rs | 8 ++++---- src/tests/mfa_config.rs | 2 ++ 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/proto b/proto index 0a76a28..c7bda9d 160000 --- a/proto +++ b/proto @@ -1 +1 @@ -Subproject commit 0a76a28870abbb3431c76824d0be7c4da380ed22 +Subproject commit c7bda9d94d4f1906331e0d1d21af197a40ecc160 diff --git a/src/handlers/register_mfa.rs b/src/handlers/register_mfa.rs index 35c92f7..4f91a3b 100644 --- a/src/handlers/register_mfa.rs +++ b/src/handlers/register_mfa.rs @@ -60,10 +60,10 @@ pub(super) async fn code_mfa_setup_finish( /// The factors Core's MFA setup can enable: a code by email or TOTP, or a FIDO2 /// security key, whose proof is an attestation rather than a code. fn reject_unsupported_method(method: i32) -> Result<(), ApiError> { - if method == MfaMethod::Email as i32 - || method == MfaMethod::Totp as i32 - || method == MfaMethod::Fido2 as i32 - { + if matches!( + MfaMethod::try_from(method), + Ok(MfaMethod::Email | MfaMethod::Totp | MfaMethod::Fido2) + ) { Ok(()) } else { error!("Requested method not supported"); diff --git a/src/tests/mfa_config.rs b/src/tests/mfa_config.rs index d4f25a1..d5a7de3 100644 --- a/src/tests/mfa_config.rs +++ b/src/tests/mfa_config.rs @@ -45,6 +45,7 @@ fn fallback_then_totp( assert_eq!(req.code, "123456"); core_response::Payload::MfaConfigAuthorize(MfaConfigAuthorizeResponse { deadline_timestamp: 1_800_003_600, + recovery_codes: vec![], }) } core_request::Payload::CodeMfaSetupStart(req) => { @@ -52,6 +53,7 @@ fn fallback_then_totp( assert_eq!(req.method, TOTP); core_response::Payload::CodeMfaSetupStartResponse(CodeMfaSetupStartResponse { totp_secret: Some("JBSWY3DPEHPK3PXP".into()), + fido2_creation_challenge: None, }) } core_request::Payload::CodeMfaSetupFinish(req) => { From 4d0466799a53e7bbe8ec509a3287001944a1d4e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20=C5=9Al=C4=99zak?= Date: Fri, 18 Sep 2026 10:41:29 +0200 Subject: [PATCH 3/4] compact comments --- build.rs | 4 +--- src/handlers/register_mfa.rs | 6 ++---- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/build.rs b/build.rs index 3ea10db..ee65df4 100644 --- a/build.rs +++ b/build.rs @@ -32,9 +32,7 @@ fn main() -> Result<(), Box> { "ClientMfaStartRequest.selected_methods", "#[serde(default)]", ) - // These are only ever sent when setting up a FIDO2 factor. Clients configuring a - // code factor leave them out of the request body entirely, so they cannot be - // required here just because protobuf lists them. + // Sent only when setting up FIDO2, absent from code-factor request bodies. .field_attribute("CodeMfaSetupFinishRequest.name", "#[serde(default)]") .field_attribute( "CodeMfaSetupFinishRequest.fido2_attestation", diff --git a/src/handlers/register_mfa.rs b/src/handlers/register_mfa.rs index 4f91a3b..f30a48e 100644 --- a/src/handlers/register_mfa.rs +++ b/src/handlers/register_mfa.rs @@ -57,8 +57,6 @@ pub(super) async fn code_mfa_setup_finish( } } -/// The factors Core's MFA setup can enable: a code by email or TOTP, or a FIDO2 -/// security key, whose proof is an attestation rather than a code. fn reject_unsupported_method(method: i32) -> Result<(), ApiError> { if matches!( MfaMethod::try_from(method), @@ -71,8 +69,8 @@ fn reject_unsupported_method(method: i32) -> Result<(), ApiError> { } } -/// The enrollment routes below carry neither a key name nor an attestation, so they -/// can only ever set up a code factor; FIDO2 has to go through MFA configuration. +/// Enrollment routes carry no key name or attestation, so FIDO2 must go +/// through MFA configuration instead. fn reject_non_code_method(method: MfaMethod) -> Result<(), ApiError> { if matches!(method, MfaMethod::Email | MfaMethod::Totp) { Ok(()) From bdb4f39a7655280ac5ad8165661f1208a9cac4bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20=C5=9Al=C4=99zak?= Date: Fri, 18 Sep 2026 11:20:32 +0200 Subject: [PATCH 4/4] fix ci --- .github/workflows/release.yml | 9 ++++++++- .github/workflows/test.yml | 4 ++-- Dockerfile | 3 ++- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 096c8db..eaf56d6 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -123,6 +123,12 @@ jobs: - name: Run sccache-cache uses: mozilla-actions/sccache-action@1583d6b38d7be47f593cb472781bbb21cab4321e # v0.0.10 + - name: Install build dependencies + run: | + sudo dpkg --add-architecture arm64 + sudo apt-get update + sudo apt-get -y install pkg-config libudev-dev libudev-dev:arm64 + - name: Build Linux x86_64 binary run: | cargo build --locked --release --target x86_64-unknown-linux-gnu @@ -133,7 +139,8 @@ jobs: - name: Build Linux aarch64 binary env: CARGO_TARGET_AARCH64_UNKNOWN_LINUX_GNU_LINKER: aarch64-linux-gnu-gcc - PKG_CONFIG_SYSROOT_DIR: /usr/lib/aarch64-linux-gnu + PKG_CONFIG_ALLOW_CROSS: "1" + PKG_CONFIG_PATH: /usr/lib/aarch64-linux-gnu/pkgconfig run: | cargo build --locked --release --target aarch64-unknown-linux-gnu mv target/aarch64-unknown-linux-gnu/release/defguard-proxy defguard-proxy-${{ env.VERSION }}-aarch64-unknown-linux-gnu diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 85cbae9..15d3d20 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -52,8 +52,8 @@ jobs: - name: Run sccache-cache uses: mozilla-actions/sccache-action@1583d6b38d7be47f593cb472781bbb21cab4321e # v0.0.10 - - name: Install protoc - run: apt-get update && apt-get -y install protobuf-compiler + - name: Install protoc and build dependencies + run: apt-get update && apt-get -y install protobuf-compiler pkg-config libudev-dev - name: Check format run: | diff --git a/Dockerfile b/Dockerfile index 0dd91de..0d3aeac 100644 --- a/Dockerfile +++ b/Dockerfile @@ -12,6 +12,7 @@ FROM rust:1 AS chef WORKDIR /build # install & cache necessary components +RUN apt-get update && apt-get -y install pkg-config libudev-dev && rm -rf /var/lib/apt/lists/* RUN cargo install cargo-chef RUN rustup component add rustfmt @@ -42,7 +43,7 @@ FROM debian:13-slim AS runtime # Bust the cache for the layer below on every build so OS security updates are always applied. ARG CACHEBUST=0 RUN echo "cachebust=${CACHEBUST}" && apt-get update -y && apt-get upgrade -y && \ - apt-get install --no-install-recommends -y ca-certificates libssl-dev lsb-release && \ + apt-get install --no-install-recommends -y ca-certificates libssl-dev libudev1 lsb-release && \ rm -rf /var/lib/apt/lists/* WORKDIR /app COPY --from=builder /build/bin/defguard-proxy .