Skip to content

Commit 19e6fb7

Browse files
j-chmielewskiwojcik91moubctez
authored
Release/1.5.1 (#169)
* Fixes pentest issue DG25-16 from 2025-09-02 (#159) * sanitize user agent to prevent html injection * add tests * Do not display sensitive data from protos (#167) * use the same phone regex as backend does (#168) * bump version to 1.5.1 * cargo update --------- Co-authored-by: Maciek <[email protected]> Co-authored-by: Adam <[email protected]>
1 parent 996cdb2 commit 19e6fb7

File tree

13 files changed

+472
-101
lines changed

13 files changed

+472
-101
lines changed

‎Cargo.lock‎

Lines changed: 317 additions & 72 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "defguard-proxy"
3-
version = "1.5.0"
3+
version = "1.5.1"
44
edition = "2021"
55
license = "Apache-2.0"
66
homepage = "https://github.com/DefGuard/proxy"
@@ -51,6 +51,7 @@ mime_guess = "2.0"
5151
base64 = "0.22"
5252
tower = "0.5"
5353
futures-util = "0.3"
54+
ammonia = "4.1.1"
5455

5556
[build-dependencies]
5657
tonic-prost-build = "0.14"

‎build.rs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,22 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
66
Emitter::default().add_instructions(&git2)?.emit()?;
77

88
tonic_prost_build::configure()
9+
// These types contain sensitive data.
10+
.skip_debug([
11+
"ActivateUserRequest",
12+
"AuthInfoResponse",
13+
"AuthenticateRequest",
14+
"AuthenticateResponse",
15+
"ClientMfaFinishResponse",
16+
"CodeMfaSetupStartResponse",
17+
"CodeMfaSetupFinishResponse",
18+
"CoreRequest",
19+
"CoreResponse",
20+
"DeviceConfigResponse",
21+
"InstanceInfoResponse",
22+
"NewDevice",
23+
"PasswordResetRequest",
24+
])
925
// Enable optional fields.
1026
.protoc_arg("--experimental_allow_proto3_optional")
1127
// Make all messages serde-serializable.

‎src/enterprise/handlers/desktop_client_mfa.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,10 @@ pub(super) async fn mfa_auth_callback(
9595
info!("MFA authentication callback completed successfully");
9696
Ok(private_cookies)
9797
} else {
98-
error!("Received invalid gRPC response type during handling the MFA OpenID authentication callback: {payload:#?}");
98+
error!(
99+
"Received invalid gRPC response type during handling the MFA OpenID authentication \
100+
callback"
101+
);
99102
Err(ApiError::InvalidResponseType)
100103
}
101104
}

‎src/enterprise/handlers/openid_login.rs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ async fn auth_info(
9494
.send(core_request::Payload::AuthInfo(request), device_info)?;
9595
let payload = get_core_response(rx).await?;
9696
if let core_response::Payload::AuthInfo(response) = payload {
97-
debug!("Received auth info {response:?}");
97+
debug!("Received auth info response");
9898

9999
let nonce_cookie = Cookie::build((NONCE_COOKIE_NAME, response.nonce))
100100
// .domain(cookie_domain)
@@ -117,7 +117,7 @@ async fn auth_info(
117117
let auth_info = AuthInfo::new(response.url, response.button_display_name);
118118
Ok((private_cookies, Json(auth_info)))
119119
} else {
120-
error!("Received invalid gRPC response type: {payload:#?}");
120+
error!("Received invalid gRPC response type");
121121
Err(ApiError::InvalidResponseType)
122122
}
123123
}
@@ -188,7 +188,10 @@ async fn auth_callback(
188188
debug!("Received auth callback response {url:?} {token:?}");
189189
Ok((private_cookies, Json(CallbackResponseData { url, token })))
190190
} else {
191-
error!("Received invalid gRPC response type during handling the OpenID authentication callback: {payload:#?}");
191+
error!(
192+
"Received invalid gRPC response type during handling the OpenID authentication \
193+
callback"
194+
);
192195
Err(ApiError::InvalidResponseType)
193196
}
194197
}

‎src/grpc.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use std::{
2+
any::Any,
23
collections::HashMap,
34
net::SocketAddr,
45
sync::{
@@ -21,7 +22,6 @@ use crate::{
2122
// connected clients
2223
type ClientMap = HashMap<SocketAddr, mpsc::UnboundedSender<Result<CoreRequest, Status>>>;
2324

24-
#[derive(Debug)]
2525
pub(crate) struct ProxyServer {
2626
current_id: Arc<AtomicU64>,
2727
clients: Arc<Mutex<ClientMap>>,
@@ -45,7 +45,7 @@ impl ProxyServer {
4545

4646
/// Sends message to the other side of RPC, with given `payload` and optional `device_info`.
4747
/// Returns `tokio::sync::oneshot::Reveicer` to let the caller await reply.
48-
#[instrument(name = "send_grpc_message", level = "debug", skip(self))]
48+
#[instrument(name = "send_grpc_message", level = "debug", skip(self, payload))]
4949
pub(crate) fn send(
5050
&self,
5151
payload: core_request::Payload,
@@ -127,13 +127,13 @@ impl proxy_server::Proxy for ProxyServer {
127127
loop {
128128
match stream.message().await {
129129
Ok(Some(response)) => {
130-
debug!("Received message from Defguard core: {response:?}");
130+
debug!("Received message from Defguard Core ID={}", response.id);
131131
connected.store(true, Ordering::Relaxed);
132132
// Discard empty payloads.
133133
if let Some(payload) = response.payload {
134134
if let Some(rx) = results.lock().unwrap().remove(&response.id) {
135135
if let Err(err) = rx.send(payload) {
136-
error!("Failed to send message to rx: {err:?}");
136+
error!("Failed to send message to rx {:?}", err.type_id());
137137
}
138138
} else {
139139
error!("Missing receiver for response #{}", response.id);

‎src/handlers/desktop_client_mfa.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ async fn start_client_mfa(
151151
info!("Started desktop client authorization {req:?}");
152152
Ok(Json(response))
153153
} else {
154-
error!("Received invalid gRPC response type: {payload:#?}");
154+
error!("Received invalid gRPC response type");
155155
Err(ApiError::InvalidResponseType)
156156
}
157157
}
@@ -170,7 +170,7 @@ async fn finish_client_mfa(
170170
if let core_response::Payload::ClientMfaFinish(response) = payload {
171171
Ok(Json(response))
172172
} else {
173-
error!("Received invalid gRPC response type: {payload:#?}");
173+
error!("Received invalid gRPC response type");
174174
Err(ApiError::InvalidResponseType)
175175
}
176176
}
@@ -210,7 +210,7 @@ async fn finish_remote_mfa(
210210
Err(ApiError::Unexpected(String::new()))
211211
}
212212
} else {
213-
error!("Received invalid gRPC response type: {payload:#?}");
213+
error!("Received invalid gRPC response type");
214214
Err(ApiError::InvalidResponseType)
215215
}
216216
}

‎src/handlers/enrollment.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,12 @@ async fn start_enrollment_process(
5959

6060
Ok((private_cookies.add(cookie), Json(response)))
6161
} else {
62-
error!("Received invalid gRPC response type: {payload:#?}");
62+
error!("Received invalid gRPC response type");
6363
Err(ApiError::InvalidResponseType)
6464
}
6565
}
6666

67-
#[instrument(level = "debug", skip(state))]
67+
#[instrument(level = "debug", skip(state, req))]
6868
async fn activate_user(
6969
State(state): State<AppState>,
7070
device_info: DeviceInfo,
@@ -94,12 +94,12 @@ async fn activate_user(
9494
}
9595
Ok(private_cookies)
9696
} else {
97-
error!("Received invalid gRPC response type: {payload:#?}");
97+
error!("Received invalid gRPC response type");
9898
Err(ApiError::InvalidResponseType)
9999
}
100100
}
101101

102-
#[instrument(level = "debug", skip(state))]
102+
#[instrument(level = "debug", skip(state, req))]
103103
async fn create_device(
104104
State(state): State<AppState>,
105105
device_info: DeviceInfo,
@@ -122,7 +122,7 @@ async fn create_device(
122122
info!("Added new device {name} {pubkey}");
123123
Ok(Json(response))
124124
} else {
125-
error!("Received invalid gRPC response type: {payload:#?}");
125+
error!("Received invalid gRPC response type");
126126
Err(ApiError::InvalidResponseType)
127127
}
128128
}
@@ -150,7 +150,7 @@ async fn get_network_info(
150150
info!("Got network info for device {pubkey}");
151151
Ok(Json(response))
152152
} else {
153-
error!("Received invalid gRPC response type: {payload:#?}");
153+
error!("Received invalid gRPC response type");
154154
Err(ApiError::InvalidResponseType)
155155
}
156156
}

‎src/handlers/mobile_client.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ pub(crate) async fn register_mobile_auth(
5858
info!("Registered mobile device for auth");
5959
Ok(())
6060
} else {
61-
error!("Received invalid gRPC response type: {payload:#?}");
61+
error!("Received invalid gRPC response type");
6262
Err(ApiError::InvalidResponseType)
6363
}
6464
}

‎src/handlers/mod.rs‎

Lines changed: 106 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ where
3535
let user_agent = TypedHeader::<UserAgent>::from_request_parts(parts, state)
3636
.await
3737
.map(|v| v.to_string())
38-
.ok();
38+
.ok()
39+
// sanitize user-agent
40+
.filter(|agent| !ammonia::is_html(agent));
3941

4042
let ip_address = forwarded_for_ip
4143
.or(insecure_ip)
@@ -55,7 +57,7 @@ where
5557
pub(crate) async fn get_core_response(rx: Receiver<Payload>) -> Result<Payload, ApiError> {
5658
debug!("Fetching core response...");
5759
if let Ok(core_response) = timeout(CORE_RESPONSE_TIMEOUT, rx).await {
58-
debug!("Got gRPC response from Defguard core: {core_response:?}");
60+
debug!("Got gRPC response from Defguard Core");
5961
if let Ok(Payload::CoreError(core_error)) = core_response {
6062
if core_error.status_code == Code::FailedPrecondition as i32
6163
&& core_error.message == "no valid license"
@@ -76,3 +78,105 @@ pub(crate) async fn get_core_response(rx: Receiver<Payload>) -> Result<Payload,
7678
Err(ApiError::CoreTimeout)
7779
}
7880
}
81+
82+
#[cfg(test)]
83+
mod tests {
84+
use super::*;
85+
use axum::{body::Body, http::Request};
86+
87+
static VALID_USER_AGENTS: &[&str] = &[
88+
// desktop
89+
"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.10 Safari/605.1.1 43.03",
90+
"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/113.0.0.0 Safari/537.3 21.05",
91+
"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/134.0.0.0 Safari/537.3 17.34",
92+
"Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/134.0.0.0 Safari/537.3 3.72",
93+
"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/134.0.0.0 Safari/537.36 Trailer/93.3.8652.5 2.48",
94+
"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/134.0.0.0 Safari/537.36 Edg/134.0.0. 2.48",
95+
"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/131.0.0.0 Safari/537.36 Edg/131.0.0. 2.48",
96+
"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/132.0.0.0 Safari/537.36 OPR/117.0.0. 2.48",
97+
"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/132.0.0.0 Safari/537.36 Edg/132.0.0. 1.24",
98+
"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.102 Safari/537.36 Edge/18.1958 1.24",
99+
"Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:136.0) Gecko/20100101 Firefox/136. 1.24",
100+
"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/107.0.0.0 Safari/537.3 1.24",
101+
102+
// mobile
103+
"Mozilla/5.0 (Linux; Android 10; K) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/134.0.0.0 Mobile Safari/537.3 63.11",
104+
"Mozilla/5.0 (iPhone; CPU iPhone OS 18_3_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/18.3.1 Mobile/15E148 Safari/604. 8.25",
105+
"Mozilla/5.0 (iPhone; CPU iPhone OS 18_3_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) GSA/360.1.737798518 Mobile/15E148 Safari/604. 5.83",
106+
"Mozilla/5.0 (iPhone; CPU iPhone OS 18_3_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) CriOS/134.0.6998.99 Mobile/15E148 Safari/604. 4.85",
107+
"Mozilla/5.0 (Linux; Android 10; K) AppleWebKit/537.36 (KHTML, like Gecko) SamsungBrowser/27.0 Chrome/125.0.0.0 Mobile Safari/537.3 3.88",
108+
"Mozilla/5.0 (iPhone; CPU iPhone OS 17_5_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.5 Mobile/15E148 Safari/604. 3.4",
109+
"Mozilla/5.0 (iPhone; CPU iPhone OS 18_3_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/18.3 Mobile/15E148 Safari/604. 1.94",
110+
"Mozilla/5.0 (iPhone; CPU iPhone OS 18_1_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/18.1.1 Mobile/15E148 Safari/604. 1.94",
111+
"Mozilla/5.0 (Linux; Android 10; K) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Mobile Safari/537.3 1.46",
112+
"Mozilla/5.0 (Android 14; Mobile; rv:136.0) Gecko/136.0 Firefox/136. 0.97",
113+
"Mozilla/5.0 (Linux; Android 10; K) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/133.0.0.0 Mobile Safari/537.3 0.97",
114+
"Mozilla/5.0 (Linux; Android 10; JNY-LX1; HMSCore 6.15.0.302) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/114.0.5735.196 HuaweiBrowser/15.0.4.312 Mobile Safari/537.3 0.97",
115+
"Mozilla/5.0 (Android 15; Mobile; rv:136.0) Gecko/136.0 Firefox/136. 0.49",
116+
"Mozilla/5.0 (iPhone; CPU iPhone OS 10_3 like Mac OS X) AppleWebKit/602.1.50 (KHTML, like Gecko) CriOS/56.0.2924.75 Mobile/14E5239e YisouSpider/5.0 Safari/602. 0.49",
117+
"Mozilla/5.0 (iPhone; CPU iPhone OS 16_7_10 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/16.6 Mobile/15E148 Safari/604. 0.49",
118+
"Mozilla/5.0 (Linux; Android 10; K) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Mobile Safari/537.3 0.49",
119+
"Mozilla/5.0 (Linux; Android 10; K) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/119.0.0.0 Mobile Safari/537.3 0.49",
120+
];
121+
122+
static INVALID_USER_AGENTS: &[&str] = &[
123+
"<h1><a href=\"//isec.pl\">CLICK HERE</a></h1>",
124+
"<html><script>alert(\"test\")</script></html>",
125+
"<h1><a href=\"//isec.pl\">CLICK HERE",
126+
];
127+
128+
struct DummyState;
129+
130+
#[tokio::test]
131+
async fn test_user_agent_sanitization_dg25_16() {
132+
let state = DummyState;
133+
134+
// valid user agents
135+
for agent in VALID_USER_AGENTS {
136+
let req = Request::builder()
137+
.header("User-Agent", *agent)
138+
.header("X-Forwarded-For", "10.0.0.1")
139+
.body(Body::empty())
140+
.unwrap();
141+
let (parts, _) = req.into_parts();
142+
let mut parts = parts;
143+
144+
let device_info = DeviceInfo::from_request_parts(&mut parts, &state)
145+
.await
146+
.expect("should succeed");
147+
148+
assert_eq!(device_info.user_agent, Some(agent.to_string()));
149+
}
150+
151+
// invalid user agents
152+
for agent in INVALID_USER_AGENTS {
153+
let req = Request::builder()
154+
.header("User-Agent", *agent)
155+
.header("X-Forwarded-For", "10.0.0.1")
156+
.body(Body::empty())
157+
.unwrap();
158+
let (parts, _) = req.into_parts();
159+
let mut parts = parts;
160+
161+
let device_info = DeviceInfo::from_request_parts(&mut parts, &state)
162+
.await
163+
.expect("should succeed");
164+
165+
assert!(device_info.user_agent.is_none());
166+
}
167+
168+
// no user agent
169+
let req = Request::builder()
170+
.header("X-Forwarded-For", "10.0.0.1")
171+
.body(Body::empty())
172+
.unwrap();
173+
let (parts, _) = req.into_parts();
174+
let mut parts = parts;
175+
176+
let device_info = DeviceInfo::from_request_parts(&mut parts, &state)
177+
.await
178+
.expect("should succeed");
179+
180+
assert!(device_info.user_agent.is_none());
181+
}
182+
}

0 commit comments

Comments
 (0)