fix(MFA): prevent premature MFA client disconnect (#549)

* add mfa authorization timestamp * fix DB queries * update inactive device detection * update query data * add missing newlines --------- Co-authored-by: Maciej Wójcik <[email protected]>
DefGuard · Feb 19, 2024 · 4f745e7 · 4f745e7
1 parent 94a73dd
commit 4f745e7
Show file tree

Hide file tree

Showing 32 changed files with 88 additions and 60 deletions.
diff --git a/...582a6face350a9c4c11db8b32fc251605cd6.json → ...f8717344fcb0c8ead0e98981439887e8e366.json b/...582a6face350a9c4c11db8b32fc251605cd6.json → ...f8717344fcb0c8ead0e98981439887e8e366.json
diff --git a/.sqlx/query-12d13e1b78576a8751ae2c219f865ee71d125c20933c6659b747cf5ccdc1d9a1.json b/.sqlx/query-12d13e1b78576a8751ae2c219f865ee71d125c20933c6659b747cf5ccdc1d9a1.json
diff --git a/...073400544f1ce454d059ac13b24b32cbdbf4.json → ...cf0c9b09ea742ea07a8de4ef8e6b199f2269.json b/...073400544f1ce454d059ac13b24b32cbdbf4.json → ...cf0c9b09ea742ea07a8de4ef8e6b199f2269.json
diff --git a/...edea0baba7447d4557f681f26c666a2c47bc.json → ...7d23b9102682c25541a0cb51080689599405.json b/...edea0baba7447d4557f681f26c666a2c47bc.json → ...7d23b9102682c25541a0cb51080689599405.json
diff --git a/...ecafa1166f16151c69b465cd49f334de7190.json → ...b0baed9bb96f3d19b38d9573b8a4422155d9.json b/...ecafa1166f16151c69b465cd49f334de7190.json → ...b0baed9bb96f3d19b38d9573b8a4422155d9.json
diff --git a/.sqlx/query-9132a40b7729383ce9c108baa1b412e872ae9c96792ae9722f66ccfb24f0a144.json b/.sqlx/query-9132a40b7729383ce9c108baa1b412e872ae9c96792ae9722f66ccfb24f0a144.json
diff --git a/...deaf8eb6824c4246d4135274e760b6af73e0.json → ...09f39004776d14c0b37fc9d7ac636b2b58ec.json b/...deaf8eb6824c4246d4135274e760b6af73e0.json → ...09f39004776d14c0b37fc9d7ac636b2b58ec.json
diff --git a/Dockerfile.device b/Dockerfile.device
@@ -9,4 +9,3 @@ PublicKey = zGMeVGm9HV9I4wSKF9AXmYnnAIhDySyqLMuKpcfIaQo=\n\
 AllowedIPs = 10.1.1.0/24\n\
 Endpoint = gateway:50051" > /etc/wireguard/defguard.conf
 CMD wg-quick up defguard && ping -s 2000 10.1.1.1
-
diff --git a/Openid.md b/Openid.md
@@ -81,4 +81,3 @@ grant_type=authorization_code
      XUVrWOLrLl0nx7RkKU8NXNHq-rvKMzqg"
   }
 ```
-
diff --git a/README.md b/README.md
@@ -57,7 +57,7 @@ Better quality video can [be found here to download](https://github.com/DefGuard
 - Onboarding - displaying custom onboarding messages, with templates, links ...
 - Ability to route predefined VPN traffic or all traffic (server needs to have NAT configured - in gateway example)
 - Live & real-time network charts
-- In development: **Multi-Factor Authentication** for VPN, live logs, dark theme, settings, and more! 
+- In development: **Multi-Factor Authentication** for VPN, live logs, dark theme, settings, and more!
 
 ## Quick start
 
@@ -102,4 +102,3 @@ Please review the [Contributing guide](https://defguard.gitbook.io/defguard/for-
 
 # Legal
 WireGuard is [registered trademarks](https://www.wireguard.com/trademark-policy/) of Jason A. Donenfeld.
-
diff --git a/docker-compose.ldap.yaml b/docker-compose.ldap.yaml
@@ -81,4 +81,4 @@ services:
       - ./ldif/samba.ldif:/opt/bitnami/openldap/etc/schema/samba.ldif:ro
       - ./ldif/init.ldif:/ldifs/init.ldif:ro
       - ./ldif/custom.ldif:/schema/custom.ldif:ro
-      - ./.volumes_ldap/openldap:/bitnami/openldap
+      - ./.volumes_ldap/openldap:/bitnami/openldap
diff --git a/e2e/.nvmrc b/e2e/.nvmrc
@@ -1 +1 @@
-v19.9
+v19.9
diff --git a/migrations/20240214090709_add_mfa_authorization_timestamp.down.sql b/migrations/20240214090709_add_mfa_authorization_timestamp.down.sql
@@ -0,0 +1 @@
+ALTER TABLE wireguard_network_device DROP COLUMN authorized_at;
diff --git a/migrations/20240214090709_add_mfa_authorization_timestamp.up.sql b/migrations/20240214090709_add_mfa_authorization_timestamp.up.sql
@@ -0,0 +1 @@
+ALTER TABLE wireguard_network_device ADD COLUMN authorized_at timestamp without time zone NULL;
diff --git a/src/db/models/device.rs b/src/db/models/device.rs
@@ -171,6 +171,7 @@ pub struct WireguardNetworkDevice {
     pub device_id: i64,
     pub preshared_key: Option<String>,
     pub is_authorized: bool,
+    pub authorized_at: Option<NaiveDateTime>,
 }
 
 #[derive(Serialize, Deserialize, Debug)]
@@ -194,6 +195,7 @@ impl WireguardNetworkDevice {
             device_id,
             preshared_key: None,
             is_authorized: false,
+            authorized_at: None,
         }
     }
 
@@ -203,14 +205,16 @@ impl WireguardNetworkDevice {
     {
         query!(
             "INSERT INTO wireguard_network_device \
-                (device_id, wireguard_network_id, wireguard_ip, is_authorized) \
-                VALUES ($1, $2, $3, $4) \
+                (device_id, wireguard_network_id, wireguard_ip, is_authorized, authorized_at, preshared_key) \
+                VALUES ($1, $2, $3, $4, $5, $6) \
                 ON CONFLICT ON CONSTRAINT device_network \
                 DO UPDATE SET wireguard_ip = $3, is_authorized = $4",
             self.device_id,
             self.wireguard_network_id,
             IpNetwork::from(self.wireguard_ip.clone()),
             self.is_authorized,
+            self.authorized_at,
+            self.preshared_key
         )
         .execute(executor)
         .await?;
@@ -223,12 +227,14 @@ impl WireguardNetworkDevice {
     {
         query!(
             "UPDATE wireguard_network_device \
-            SET wireguard_ip = $3, is_authorized = $4 \
+            SET wireguard_ip = $3, is_authorized = $4, authorized_at = $5, preshared_key = $6 \
             WHERE device_id = $1 AND wireguard_network_id = $2",
             self.device_id,
             self.wireguard_network_id,
             IpNetwork::from(self.wireguard_ip.clone()),
             self.is_authorized,
+            self.authorized_at,
+            self.preshared_key,
         )
         .execute(executor)
         .await?;
@@ -260,7 +266,7 @@ impl WireguardNetworkDevice {
     {
         let res = query_as!(
             Self,
-            "SELECT device_id, wireguard_network_id, wireguard_ip as \"wireguard_ip: IpAddr\", preshared_key, is_authorized \
+            "SELECT device_id, wireguard_network_id, wireguard_ip as \"wireguard_ip: IpAddr\", preshared_key, is_authorized, authorized_at \
             FROM wireguard_network_device \
             WHERE device_id = $1 AND wireguard_network_id = $2",
             device_id,
@@ -277,7 +283,7 @@ impl WireguardNetworkDevice {
     ) -> Result<Option<Vec<Self>>, SqlxError> {
         let result = query_as!(
             Self,
-            "SELECT device_id, wireguard_network_id, wireguard_ip as \"wireguard_ip: IpAddr\", preshared_key, is_authorized \
+            "SELECT device_id, wireguard_network_id, wireguard_ip as \"wireguard_ip: IpAddr\", preshared_key, is_authorized, authorized_at \
             FROM wireguard_network_device WHERE device_id = $1",
             device_id
         )
@@ -298,7 +304,7 @@ impl WireguardNetworkDevice {
     {
         let res = query_as!(
             Self,
-            "SELECT device_id, wireguard_network_id, wireguard_ip as \"wireguard_ip: IpAddr\", preshared_key, is_authorized \
+            "SELECT device_id, wireguard_network_id, wireguard_ip as \"wireguard_ip: IpAddr\", preshared_key, is_authorized, authorized_at \
             FROM wireguard_network_device \
             WHERE wireguard_network_id = $1",
             network_id

diff --git a/src/grpc/desktop_client_mfa.rs b/src/grpc/desktop_client_mfa.rs
@@ -11,6 +11,7 @@ use crate::{
     handlers::mail::send_email_mfa_code_email,
     mail::Mail,
 };
+use chrono::Utc;
 use std::collections::HashMap;
 use tokio::sync::{broadcast::Sender, mpsc::UnboundedSender};
 use tonic::Status;
@@ -232,6 +233,7 @@ impl ClientMfaServer {
 
         // authorize device for given location
         network_device.is_authorized = true;
+        network_device.authorized_at = Some(Utc::now().naive_utc());
 
         // save updated network config
         network_device

diff --git a/src/wireguard_peer_disconnect.rs b/src/wireguard_peer_disconnect.rs
@@ -71,6 +71,7 @@ pub async fn run_periodic_peer_disconnect(
             JOIN wireguard_network_device wnd ON wnd.device_id = d.id \
             LEFT JOIN stats on d.id = stats.device_id \
             WHERE wnd.wireguard_network_id = $1 AND wnd.is_authorized = true AND \
+            (wnd.authorized_at IS NULL OR (NOW() - wnd.authorized_at) > $2 * interval '1 second') AND \
             (stats.latest_handshake IS NULL OR (NOW() - stats.latest_handshake) > $2 * interval '1 second')",
             location_id,
                 location.peer_disconnect_threshold as f64

diff --git a/templates/mail_enrollment_admin_notification.tera b/templates/mail_enrollment_admin_notification.tera
@@ -6,4 +6,4 @@ macros::paragraph(content="Dear " ~ admin_first_name ~ " " ~ admin_last_name),
 macros::paragraph(content=first_name ~ " " ~ last_name ~ " just completed their enrollment process."),
 macros::paragraph(content="Have a good day!")] %}
 {{ macros::text_section(content_array=section_content) }}
-{% endblock %}
+{% endblock %}
diff --git a/templates/mail_enrollment_welcome.tera b/templates/mail_enrollment_welcome.tera
@@ -3,4 +3,4 @@
 {% block mail_content %}
 {% set section_content = [macros::paragraph(content=welcome_message_content)] %}
 {{ macros::text_section(content_array=section_content)}}
-{% endblock %}
+{% endblock %}
diff --git a/templates/mail_gateway_disconnected.tera b/templates/mail_gateway_disconnected.tera
@@ -1,7 +1,7 @@
 {#
 Requires context:
 gateway_name ->  name of gateway
-gateway_ip -> gateway adress 
+gateway_ip -> gateway adress
 network_name ->  name of network
 #}
 {% extends "base.tera" %}
@@ -12,4 +12,3 @@ macros::paragraph(content="Your gateway: " ~ gateway_name ~ " (IP: " ~ gateway_i
 macros::paragraph(content="Please login to your gateway server and see the logs.")] %}
 {{ macros::text_section(content_array=section_content) }}
 {% endblock %}
-
diff --git a/templates/mail_mfa_configured.tera b/templates/mail_mfa_configured.tera
@@ -8,4 +8,4 @@ mfa_method -> what method was activated
 {% set section_content = [macros::paragraph(content="A Multi-Factor Authorization method: " ~ mfa_method ~ " has been
 activated in your account.")] %}
 {{ macros::text_section(content_array=section_content) }}
-{% endblock %}
+{% endblock %}
diff --git a/templates/mail_new_device_added.tera b/templates/mail_new_device_added.tera
@@ -30,4 +30,4 @@ locations_list ]
 %}
 {# render device section #}
 {{ macros::text_section(content_array=section_content) }}
-{% endblock %}
+{% endblock %}
diff --git a/templates/mail_new_device_login.tera b/templates/mail_new_device_login.tera
@@ -16,4 +16,4 @@ assigned_ip -> ip of device in location
 {{ macros::text_section(content_array=section_content) }}
 {{ macros::spacer(height="40px")}}
 {# render device section #}
-{% endblock %}
+{% endblock %}
diff --git a/templates/mail_new_device_ocid_login.tera b/templates/mail_new_device_ocid_login.tera
@@ -19,4 +19,4 @@ assigned_ip -> ip of device in location
 {{ macros::text_section(content_array=section_content) }}
 {{ macros::spacer(height="40px")}}
 {# render device section #}
-{% endblock %}
+{% endblock %}
diff --git a/templates/mail_support_data.tera b/templates/mail_support_data.tera
@@ -3,4 +3,4 @@
 {% block mail_content %}
 {% set section_content = [macros::paragraph(content="Support data in attachments.")] %}
 {{ macros::text_section(content_array=section_content)}}
-{% endblock %}
+{% endblock %}
diff --git a/templates/mail_test.tera b/templates/mail_test.tera
@@ -5,4 +5,4 @@
 macros::paragraph(content="This is test email from Defguard system."),
 macros::paragraph(content="If you received it, your SMTP configuration is ok.")] %}
 {{ macros::text_section(content_array=section_content)}}
-{% endblock %}
+{% endblock %}
diff --git a/user_agent_header_regexes.yaml b/user_agent_header_regexes.yaml
@@ -5860,4 +5860,4 @@ device_parsers:
   - regex: 'Mac OS'
     device_replacement: 'Mac'
     brand_replacement: 'Apple'
-    model_replacement: 'Mac'
+    model_replacement: 'Mac'
Original file line number	Diff line number	Diff line change
Expand Up		@@ -9,4 +9,3 @@ PublicKey = zGMeVGm9HV9I4wSKF9AXmYnnAIhDySyqLMuKpcfIaQo=\n\
		AllowedIPs = 10.1.1.0/24\n\
		Endpoint = gateway:50051" > /etc/wireguard/defguard.conf
		CMD wg-quick up defguard && ping -s 2000 10.1.1.1
Original file line number	Diff line number	Diff line change
Expand Up		@@ -81,4 +81,3 @@ grant_type=authorization_code
		XUVrWOLrLl0nx7RkKU8NXNHq-rvKMzqg"
		}
		```
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		ALTER TABLE wireguard_network_device DROP COLUMN authorized_at;
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		ALTER TABLE wireguard_network_device ADD COLUMN authorized_at timestamp without time zone NULL;