refactor: improve gp response parsing

Cargo.lock

@@ -564,7 +564,7 @@ dependencies = [
 
 [[package]]
 name = "common"
-version = "2.1.3"
+version = "2.1.4"
 dependencies = [
  "is_executable",
 ]
@@ -1430,7 +1430,7 @@ dependencies = [
 
 [[package]]
 name = "gpapi"
-version = "2.1.3"
+version = "2.1.4"
 dependencies = [
  "anyhow",
  "base64 0.21.5",
@@ -1462,7 +1462,7 @@ dependencies = [
 
 [[package]]
 name = "gpauth"
-version = "2.1.3"
+version = "2.1.4"
 dependencies = [
  "anyhow",
  "clap",
@@ -1483,7 +1483,7 @@ dependencies = [
 
 [[package]]
 name = "gpclient"
-version = "2.1.3"
+version = "2.1.4"
 dependencies = [
  "anyhow",
  "clap",
@@ -1505,7 +1505,7 @@ dependencies = [
 
 [[package]]
 name = "gpgui-helper"
-version = "2.1.3"
+version = "2.1.4"
 dependencies = [
  "anyhow",
  "clap",
@@ -1523,7 +1523,7 @@ dependencies = [
 
 [[package]]
 name = "gpservice"
-version = "2.1.3"
+version = "2.1.4"
 dependencies = [
  "anyhow",
  "axum",
@@ -2537,7 +2537,7 @@ dependencies = [
 
 [[package]]
 name = "openconnect"
-version = "2.1.3"
+version = "2.1.4"
 dependencies = [
  "cc",
  "common",

Cargo.toml

@@ -5,7 +5,7 @@ members = ["crates/*", "apps/gpclient", "apps/gpservice", "apps/gpauth", "apps/g
 
 [workspace.package]
 rust-version = "1.70"
-version = "2.1.3"
+version = "2.1.4"
 authors = ["Kevin Yue <k3vinyue@gmail.com>"]
 homepage = "https://github.com/yuezk/GlobalProtect-openconnect"
 edition = "2021"

apps/gpclient/src/connect.rs

@@ -6,7 +6,7 @@ use gpapi::{
   clap::args::Os,
   credential::{Credential, PasswordCredential},
   error::PortalError,
-  gateway::gateway_login,
+  gateway::{gateway_login, GatewayLogin},
   gp_params::{ClientOs, GpParams},
   portal::{prelogin, retrieve_config, Prelogin},
   process::{
@@ -154,7 +154,7 @@ impl<'a> ConnectHandler<'a> {
     let gateway = selected_gateway.server();
     let cred = portal_config.auth_cookie().into();
 
-    let cookie = match gateway_login(gateway, &cred, &gp_params).await {
+    let cookie = match self.login_gateway(gateway, &cred, &gp_params).await {
       Ok(cookie) => cookie,
       Err(err) => {
         info!("Gateway login failed: {}", err);
@@ -174,11 +174,28 @@ impl<'a> ConnectHandler<'a> {
     let prelogin = prelogin(gateway, &gp_params).await?;
     let cred = self.obtain_credential(&prelogin, gateway).await?;
 
-    let cookie = gateway_login(gateway, &cred, &gp_params).await?;
+    let cookie = self.login_gateway(gateway, &cred, &gp_params).await?;
 
     self.connect_gateway(gateway, &cookie).await
   }
 
+  async fn login_gateway(&self, gateway: &str, cred: &Credential, gp_params: &GpParams) -> anyhow::Result<String> {
+    let mut gp_params = gp_params.clone();
+
+    loop {
+      match gateway_login(gateway, cred, &gp_params).await? {
+        GatewayLogin::Cookie(cookie) => return Ok(cookie),
+        GatewayLogin::Mfa(message, input_str) => {
+          let otp = Text::new(&message).prompt()?;
+          gp_params.set_input_str(&input_str);
+          gp_params.set_otp(&otp);
+
+          info!("Retrying gateway login with MFA...");
+        }
+      }
+    }
+  }
+
   async fn connect_gateway(&self, gateway: &str, cookie: &str) -> anyhow::Result<()> {
     let mtu = self.args.mtu.unwrap_or(0);
     let csd_uid = get_csd_uid(&self.args.csd_user)?;

changelog.md

@@ -1,5 +1,10 @@
 # Changelog
 
+## 2.1.4 - 2024-04-10
+
+- Support MFA authentication (fix [#343](https://github.com/yuezk/GlobalProtect-openconnect/issues/343))
+- Improve the Gateway switcher UI
+
 ## 2.1.3 - 2024-04-07
 
 - Support CAS authentication (fix [#339](https://github.com/yuezk/GlobalProtect-openconnect/issues/339))

crates/gpapi/src/gateway/login.rs

@@ -8,10 +8,15 @@ use crate::{
   credential::Credential,
   error::PortalError,
   gp_params::GpParams,
-  utils::{normalize_server, parse_gp_error, remove_url_scheme},
+  utils::{normalize_server, parse_gp_response, remove_url_scheme},
 };
 
-pub async fn gateway_login(gateway: &str, cred: &Credential, gp_params: &GpParams) -> anyhow::Result<String> {
+pub enum GatewayLogin {
+  Cookie(String),
+  Mfa(String, String),
+}
+
+pub async fn gateway_login(gateway: &str, cred: &Credential, gp_params: &GpParams) -> anyhow::Result<GatewayLogin> {
   let url = normalize_server(gateway)?;
   let gateway = remove_url_scheme(&url);
 
@@ -36,23 +41,25 @@ pub async fn gateway_login(gateway: &str, cred: &Credential, gp_params: &GpParam
     .await
     .map_err(|e| anyhow::anyhow!(PortalError::NetworkError(e.to_string())))?;
 
-  let status = res.status();
+  let res = parse_gp_response(res).await.map_err(|err| {
+    warn!("{err}");
+    anyhow::anyhow!("Gateway login error: {}", err.reason)
+  })?;
 
-  if status.is_client_error() || status.is_server_error() {
+  // MFA detected
-    let (reason, res) = parse_gp_error(res).await;
+  if res.contains("Challenge") {
+    let Some((message, input_str)) = parse_mfa(&res) else {
+      bail!("Failed to parse MFA challenge: {res}");
+    };
 
-    warn!(
+    return Ok(GatewayLogin::Mfa(message, input_str));
-      "Gateway login error: reason={}, status={}, response={}",
-      reason, status, res
-    );
-
-    bail!("Gateway login error, reason: {}", reason);
   }
 
-  let res_xml = res.text().await?;
+  let doc = Document::parse(&res)?;
-  let doc = Document::parse(&res_xml)?;
 
-  build_gateway_token(&doc, gp_params.computer())
+  let cookie = build_gateway_token(&doc, gp_params.computer())?;
+
+  Ok(GatewayLogin::Cookie(cookie))
 }
 
 fn build_gateway_token(doc: &Document, computer: &str) -> anyhow::Result<String> {
@@ -86,3 +93,33 @@ fn read_args<'a>(args: &'a [String], index: usize, key: &'a str) -> anyhow::Resu
     .ok_or_else(|| anyhow::anyhow!("Failed to read {key} from args"))
     .map(|s| (key, s.as_ref()))
 }
+
+fn parse_mfa(res: &str) -> Option<(String, String)> {
+  let message = res
+    .lines()
+    .find(|l| l.contains("respMsg"))
+    .and_then(|l| l.split('"').nth(1).map(|s| s.to_string()))?;
+
+  let input_str = res
+    .lines()
+    .find(|l| l.contains("inputStr"))
+    .and_then(|l| l.split('"').nth(1).map(|s| s.to_string()))?;
+
+  Some((message, input_str))
+}
+
+#[cfg(test)]
+mod tests {
+  use super::*;
+
+  #[test]
+  fn mfa() {
+    let res = r#"var respStatus = "Challenge";
+var respMsg = "MFA message";
+thisForm.inputStr.value = "5ef64e83000119ed";"#;
+
+    let (message, input_str) = parse_mfa(res).unwrap();
+    assert_eq!(message, "MFA message");
+    assert_eq!(input_str, "5ef64e83000119ed");
+  }
+}

crates/gpapi/src/gp_params.rs

@@ -42,7 +42,7 @@ impl ClientOs {
   }
 }
 
-#[derive(Debug, Serialize, Deserialize, Type, Default)]
+#[derive(Debug, Serialize, Deserialize, Type, Default, Clone)]
 pub struct GpParams {
   is_gateway: bool,
   user_agent: String,
@@ -51,6 +51,9 @@ pub struct GpParams {
   client_version: Option<String>,
   computer: String,
   ignore_tls_errors: bool,
+  // Used for MFA
+  input_str: Option<String>,
+  otp: Option<String>,
 }
 
 impl GpParams {
@@ -90,6 +93,14 @@ impl GpParams {
     self.client_version.as_deref()
   }
 
+  pub fn set_input_str(&mut self, input_str: &str) {
+    self.input_str = Some(input_str.to_string());
+  }
+
+  pub fn set_otp(&mut self, otp: &str) {
+    self.otp = Some(otp.to_string());
+  }
+
   pub(crate) fn to_params(&self) -> HashMap<&str, &str> {
     let mut params: HashMap<&str, &str> = HashMap::new();
     let client_os = self.client_os.as_str();
@@ -100,11 +111,16 @@ impl GpParams {
     params.insert("ok", "Login");
     params.insert("direct", "yes");
     params.insert("ipv6-support", "yes");
-    params.insert("inputStr", "");
     params.insert("clientVer", "4100");
     params.insert("clientos", client_os);
     params.insert("computer", &self.computer);
 
+    // MFA
+    params.insert("inputStr", self.input_str.as_deref().unwrap_or_default());
+    if let Some(otp) = &self.otp {
+      params.insert("passwd", otp);
+    }
+
     if let Some(os_version) = &self.os_version {
       params.insert("os-version", os_version);
     }
@@ -187,6 +203,8 @@ impl GpParamsBuilder {
       client_version: self.client_version.clone(),
       computer: self.computer.clone(),
       ignore_tls_errors: self.ignore_tls_errors,
+      input_str: Default::default(),
+      otp: Default::default(),
     }
   }
 }

crates/gpapi/src/portal/config.rs

@@ -10,7 +10,7 @@ use crate::{
   error::PortalError,
   gateway::{parse_gateways, Gateway},
   gp_params::GpParams,
-  utils::{normalize_server, parse_gp_error, remove_url_scheme, xml},
+  utils::{normalize_server, parse_gp_response, remove_url_scheme, xml},
 };
 
 #[derive(Debug, Serialize, Type)]
@@ -108,24 +108,19 @@ pub async fn retrieve_config(portal: &str, cred: &Credential, gp_params: &GpPara
     .send()
     .await
     .map_err(|e| anyhow::anyhow!(PortalError::NetworkError(e.to_string())))?;
-  let status = res.status();
 
-  if status == StatusCode::NOT_FOUND {
+  let res_xml = parse_gp_response(res).await.or_else(|err| {
-    bail!(PortalError::ConfigError("Config endpoint not found".to_string()))
+    if err.status == StatusCode::NOT_FOUND {
-  }
+      bail!(PortalError::ConfigError("Config endpoint not found".to_string()));
+    }
 
-  if status.is_client_error() || status.is_server_error() {
+    if err.is_status_error() {
-    let (reason, res) = parse_gp_error(res).await;
+      warn!("{err}");
+      bail!("Portal config error: {}", err.reason);
+    }
 
-    warn!(
+    Err(anyhow::anyhow!(PortalError::ConfigError(err.reason)))
-      "Portal config error: reason={}, status={}, response={}",
+  })?;
-      reason, status, res
-    );
-
-    bail!("Portal config error, reason: {}", reason);
-  }
-
-  let res_xml = res.text().await.map_err(|e| PortalError::ConfigError(e.to_string()))?;
 
   if res_xml.is_empty() {
     bail!(PortalError::ConfigError("Empty portal config response".to_string()))

crates/gpapi/src/portal/prelogin.rs

@@ -8,7 +8,7 @@ use specta::Type;
 use crate::{
   error::PortalError,
   gp_params::GpParams,
-  utils::{base64, normalize_server, parse_gp_error, xml},
+  utils::{base64, normalize_server, parse_gp_response, xml},
 };
 
 const REQUIRED_PARAMS: [&str; 8] = [
@@ -126,23 +126,18 @@ pub async fn prelogin(portal: &str, gp_params: &GpParams) -> anyhow::Result<Prel
     .await
     .map_err(|e| anyhow::anyhow!(PortalError::NetworkError(e.to_string())))?;
 
-  let status = res.status();
+  let res_xml = parse_gp_response(res).await.or_else(|err| {
-  if status == StatusCode::NOT_FOUND {
+    if err.status == StatusCode::NOT_FOUND {
-    bail!(PortalError::PreloginError("Prelogin endpoint not found".to_string()))
+      bail!(PortalError::PreloginError("Prelogin endpoint not found".to_string()))
-  }
+    }
 
-  if status.is_client_error() || status.is_server_error() {
+    if err.is_status_error() {
-    let (reason, res) = parse_gp_error(res).await;
+      warn!("{err}");
+      bail!("Prelogin error: {}", err.reason)
+    }
 
-    warn!("Prelogin error: reason={}, status={}, response={}", reason, status, res);
+    Err(anyhow!(PortalError::PreloginError(err.reason)))
-
+  })?;
-    bail!("Prelogin error: {}", status)
-  }
-
-  let res_xml = res
-    .text()
-    .await
-    .map_err(|e| PortalError::PreloginError(e.to_string()))?;
 
   let prelogin = parse_res_xml(res_xml, is_gateway).map_err(|e| PortalError::PreloginError(e.to_string()))?;
 

crates/gpapi/src/utils/mod.rs

@@ -1,5 +1,3 @@
-use reqwest::{Response, Url};
-
 pub(crate) mod xml;
 
 pub mod base64;
@@ -15,8 +13,12 @@ pub mod window;
 
 mod shutdown_signal;
 
+use log::warn;
 pub use shutdown_signal::shutdown_signal;
 
+use reqwest::{Response, StatusCode, Url};
+use thiserror::Error;
+
 /// Normalize the server URL to the format `https://<host>:<port>`
 pub fn normalize_server(server: &str) -> anyhow::Result<String> {
   let server = if server.starts_with("https://") || server.starts_with("http://") {
@@ -42,7 +44,41 @@ pub fn remove_url_scheme(s: &str) -> String {
   s.replace("http://", "").replace("https://", "")
 }
 
-pub(crate) async fn parse_gp_error(res: Response) -> (String, String) {
+#[derive(Error, Debug)]
+#[error("GP response error: reason={reason}, status={status}, body={body}")]
+pub(crate) struct GpError {
+  pub status: StatusCode,
+  pub reason: String,
+  body: String,
+}
+
+impl GpError {
+  pub fn is_status_error(&self) -> bool {
+    self.status.is_client_error() || self.status.is_server_error()
+  }
+}
+
+pub(crate) async fn parse_gp_response(res: Response) -> anyhow::Result<String, GpError> {
+  let status = res.status();
+
+  if status.is_client_error() || status.is_server_error() {
+    let (reason, body) = parse_gp_error(res).await;
+
+    return Err(GpError { status, reason, body });
+  }
+
+  res.text().await.map_err(|err| {
+    warn!("Failed to read response: {}", err);
+
+    GpError {
+      status,
+      reason: "failed to read response".to_string(),
+      body: "<failed to read response>".to_string(),
+    }
+  })
+}
+
+async fn parse_gp_error(res: Response) -> (String, String) {
   let reason = res
     .headers()
     .get("x-private-pan-globalprotect")