diff --git a/dsc/tests/dsc_sshdconfig.tests.ps1 b/dsc/tests/dsc_sshdconfig.tests.ps1 index ccb5564f7..1cbf7883c 100644 --- a/dsc/tests/dsc_sshdconfig.tests.ps1 +++ b/dsc/tests/dsc_sshdconfig.tests.ps1 @@ -177,4 +177,197 @@ resources: $out.results.result.afterState.authorizedkeysfile | Should -Be @('./.ssh/authorized_keys', './.ssh/authorized_keys2') } } + + Context 'Subsystem and SubsystemList Tests' { + BeforeAll { + # Create a temporary test directory for sshd_config files + $TestDir = Join-Path $TestDrive "sshd_test" + New-Item -Path $TestDir -ItemType Directory -Force | Out-Null + $script:TestConfigPath = Join-Path $TestDir "sshd_config" + + # Define OS-specific paths with spaces + if ($IsWindows) { + $script:PathWithSpaces = "C:\Program Files\OpenSSH\sftp-server.exe" + $script:DefaultSftpPath = "sftp-server.exe" + $script:AlternatePath = "C:\OpenSSH\bin\sftp.exe" + } + else { + $script:PathWithSpaces = "/usr/local/lib/openssh server/sftp-server" + $script:DefaultSftpPath = "/usr/lib/openssh/sftp-server" + $script:AlternatePath = "/usr/libexec/sftp-server" + } + } + + BeforeEach { + # Create test config with existing subsystems + $initialContent = @" +Port 22 +subsystem sftp $script:DefaultSftpPath +Subsystem test2 /path/to/test2 +PasswordAuthentication yes +"@ + Set-Content -Path $script:TestConfigPath -Value $initialContent + } + + AfterEach { + # Clean up test config file after each test + if (Test-Path $script:TestConfigPath) { + Remove-Item -Path $script:TestConfigPath -Force -ErrorAction SilentlyContinue + } + if (Test-Path "$script:TestConfigPath.bak") { + Remove-Item -Path "$script:TestConfigPath.bak" -Force -ErrorAction SilentlyContinue + } + } + + It 'Should add a new subsystem that does not already exist' -Skip:($script:skipSubsystemTests) { + $config_yaml = @" +`$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json +metadata: + Microsoft.DSC: + securityContext: elevated +resources: +- name: newsub + type: Microsoft.OpenSSH.SSHD/Subsystem + metadata: + filepath: $($script:TestConfigPath -replace '\\', '/') + properties: + _exist: true + subsystem: + name: newsubsystem + value: /path/to/newsubsystem +"@ + $out = dsc config set -i "$config_yaml" | ConvertFrom-Json -Depth 10 + $LASTEXITCODE | Should -Be 0 + $out.hadErrors | Should -BeFalse + $out.results.Count | Should -Be 1 + $out.results[0].type | Should -BeExactly 'Microsoft.OpenSSH.SSHD/Subsystem' + $out.results[0].result.afterState._exist | Should -Be $true + $out.results[0].result.afterState.subsystem.name | Should -Be 'newsubsystem' + + # Verify file contains the new subsystem + $subsystems = Get-Content $script:TestConfigPath | Where-Object { $_ -match '^\s*subsystem\s+' } + $subsystems.Count | Should -Be 3 + $subsystems | Should -Contain "subsystem newsubsystem /path/to/newsubsystem" + } + + It 'Should remove a subsystem when _exist is false' -Skip:($script:skipSubsystemTests) { + $config_yaml = @" +`$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json +metadata: + Microsoft.DSC: + securityContext: elevated +resources: +- name: removesub + type: Microsoft.OpenSSH.SSHD/Subsystem + metadata: + filepath: $($script:TestConfigPath -replace '\\', '/') + properties: + _exist: false + subsystem: + name: sftp +"@ + $out = dsc config set -i "$config_yaml" | ConvertFrom-Json -Depth 10 + $LASTEXITCODE | Should -Be 0 + $out.hadErrors | Should -BeFalse + $out.results[0].result.afterState._exist | Should -Be $false + + # Verify subsystem was removed + $subsystems = Get-Content $script:TestConfigPath | Where-Object { $_ -match '^\s*subsystem\s+' } + $subsystems.Count | Should -Be 1 + $subsystems | Should -Not -Match 'sftp' + } + + It 'Should add multiple new subsystems with SubsystemList' -Skip:($script:skipSubsystemTests) { + $config_yaml = @" +`$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json +metadata: + Microsoft.DSC: + securityContext: elevated +resources: +- name: multisubsystem + type: Microsoft.OpenSSH.SSHD/SubsystemList + metadata: + filepath: $($script:TestConfigPath -replace '\\', '/') + properties: + _purge: false + subsystem: + - name: newsub1 + value: /path/to/newsub1 + - name: newsub2 + value: /path/to/newsub2 +"@ + $out = dsc config set -i "$config_yaml" | ConvertFrom-Json -Depth 10 + $LASTEXITCODE | Should -Be 0 + $out.hadErrors | Should -BeFalse + $out.results[0].type | Should -BeExactly 'Microsoft.OpenSSH.SSHD/SubsystemList' + $out.results[0].result.afterState.subsystem.Count | Should -Be 2 + + # Verify all subsystems are present (old + new) + $subsystems = Get-Content $script:TestConfigPath | Where-Object { $_ -match '^\s*subsystem\s+' } + $subsystems.Count | Should -Be 4 + $subsystems | Should -Contain "subsystem newsub1 /path/to/newsub1" + $subsystems | Should -Contain "subsystem newsub2 /path/to/newsub2" + } + + It 'Should preserve unlisted subsystems when _purge is false' { + $config_yaml = @" +`$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json +metadata: + Microsoft.DSC: + securityContext: elevated +resources: +- name: preservesubsystem + type: Microsoft.OpenSSH.SSHD/SubsystemList + metadata: + filepath: $($script:TestConfigPath -replace '\\', '/') + properties: + _purge: false + subsystem: + - name: onlythisone + value: /path/to/this +"@ + $out = dsc config set -i "$config_yaml" | ConvertFrom-Json -Depth 10 + $LASTEXITCODE | Should -Be 0 + $out.hadErrors | Should -BeFalse + + # Verify all existing subsystems are still present plus the new one + $subsystems = Get-Content $script:TestConfigPath | Where-Object { $_ -match '^\s*subsystem\s+' } + $subsystems.Count | Should -Be 3 + $subsystems | Should -Contain "subsystem onlythisone /path/to/this" + $subsystems | Should -Contain "subsystem sftp $script:DefaultSftpPath" + $subsystems | Should -Contain "Subsystem test2 /path/to/test2" + } + + It 'Should remove unlisted subsystems when _purge is true' { + $config_yaml = @" +`$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json +metadata: + Microsoft.DSC: + securityContext: elevated +resources: +- name: purgesubsystem + type: Microsoft.OpenSSH.SSHD/SubsystemList + metadata: + filepath: $($script:TestConfigPath -replace '\\', '/') + properties: + _purge: true + subsystem: + - name: sftp + value: $($script:AlternatePath -replace '\\', '/') + - name: newsub + value: /path/to/newsub +"@ + $out = dsc config set -i "$config_yaml" | ConvertFrom-Json -Depth 10 + $LASTEXITCODE | Should -Be 0 + $out.hadErrors | Should -BeFalse + + # Verify only specified subsystems remain + $subsystems = Get-Content $script:TestConfigPath | Where-Object { $_ -match '^\s*subsystem\s+' } + $subsystems.Count | Should -Be 2 + $sftpLine = $subsystems | Where-Object { $_ -match 'sftp' } + $sftpLine | Should -Match ([regex]::Escape($script:AlternatePath)) + $subsystems | Should -Contain "subsystem newsub /path/to/newsub" + $subsystems | Should -Not -Match 'test2' + } + } } diff --git a/resources/sshdconfig/.project.data.json b/resources/sshdconfig/.project.data.json index c47404b80..33510799e 100644 --- a/resources/sshdconfig/.project.data.json +++ b/resources/sshdconfig/.project.data.json @@ -7,7 +7,9 @@ ], "CopyFiles": { "All": [ - "sshd_config.dsc.resource.json" + "sshd_config.dsc.resource.json", + "sshd-subsystem.dsc.resource.json", + "sshd-subsystemList.dsc.resource.json" ], "Windows": [ "sshd-windows.dsc.resource.json" diff --git a/resources/sshdconfig/locales/en-us.toml b/resources/sshdconfig/locales/en-us.toml index 03df0c91e..754e36620 100644 --- a/resources/sshdconfig/locales/en-us.toml +++ b/resources/sshdconfig/locales/en-us.toml @@ -33,6 +33,7 @@ defaultShellEscapeArgsMustBe0Or1 = "'%{input}' must be a 0 or 1" defaultShellEscapeArgsMustBeDWord = "escapeArguments must be a DWord" defaultShellMustBeString = "shell must be a string" includeWarning = "Include directive found in sshd_config. This resource uses 'sshd -T' to process the overall configuration state, which merges all included files but does not return the Include directive itself" +invalidSetting = "Invalid setting: %{setting}" traceInput = "Get input:" windowsOnly = "Microsoft.OpenSSH.SSHD/Windows is only applicable to Windows" @@ -57,6 +58,8 @@ missingKeyInCriteria = "missing key in criteria node: '%{input}'" missingValueInCriteria = "missing value in criteria node: '%{input}'" missingValueInChildNode = "missing value in child node: '%{input}'" noArgumentsFound = "no arguments found in node: '%{input}'" +structuredKeywordCannotUseOperator = "structured keyword '%{keyword}' cannot use operator syntax" +structuredKeywordRequiresMinArgs = "structured keyword '%{keyword}' requires at least a name and a value" valueDebug = "Parsed argument value:" unknownNode = "unknown node: '%{kind}'" unknownNodeType = "unknown node type: '%{node}'" @@ -67,9 +70,15 @@ backupCreated = "Backup created at: %{path}" cleanupFailed = "Failed to clean up temporary file %{path}: %{error}" configDoesNotExist = "sshd_config file does not exist, no backup created" defaultShellDebug = "default_shell: %{shell}" +entryNameRequired = "Entry 'name' field is required and cannot be empty" +entryValueRequired = "Entry '%{name}' requires a 'value' field" +expectedArrayForKeyword = "Expected array for keyword '%{keyword}'" +failedToParse = "failed to parse: '%{input}'" failedToParseDefaultShell = "failed to parse input for DefaultShell with error: '%{error}'" +multipleKeywordsNotAllowed = "Multiple keywords not allowed in input (found %{count} keywords)" +nameValueEntryRequiresValue = "Name-value keyword entry requires 'value' field when _exist is true" +noKeywordFoundInInput = "No keyword found in input (expected one keyword like 'subsystem')" purgeFalseRequiresExistingFile = "_purge=false requires an existing sshd_config file. Use _purge=true to create a new configuration file." -purgeFalseUnsupported = "_purge=false is not supported for keywords that can have multiple values" settingDefaultShell = "Setting default shell" settingSshdConfig = "Setting sshd_config" shellPathDoesNotExist = "shell path does not exist: '%{shell}'" diff --git a/resources/sshdconfig/src/args.rs b/resources/sshdconfig/src/args.rs index 56c32b91a..03a2f4349 100644 --- a/resources/sshdconfig/src/args.rs +++ b/resources/sshdconfig/src/args.rs @@ -73,5 +73,7 @@ pub struct DefaultShell { #[derive(Clone, Debug, Eq, PartialEq, ValueEnum)] pub enum Setting { SshdConfig, - WindowsGlobal -} \ No newline at end of file + SshdConfigRepeat, + SshdConfigRepeatList, + WindowsGlobal, +} diff --git a/resources/sshdconfig/src/formatter.rs b/resources/sshdconfig/src/formatter.rs index 14c91fb5c..cef377226 100644 --- a/resources/sshdconfig/src/formatter.rs +++ b/resources/sshdconfig/src/formatter.rs @@ -8,7 +8,7 @@ use std::{fmt, fmt::Write}; use tracing::warn; use crate::error::SshdConfigError; -use crate::metadata::{MULTI_ARG_KEYWORDS_COMMA_SEP, REPEATABLE_KEYWORDS}; +use crate::metadata::{KeywordInfo, ValueSeparator}; #[derive(Debug, Deserialize)] struct MatchBlock { @@ -19,22 +19,23 @@ struct MatchBlock { #[derive(Clone, Debug)] pub struct SshdConfigValue<'a> { - is_repeatable: bool, + keyword_info: KeywordInfo, key: &'a str, - separator: ValueSeparator, value: &'a Value, -} - -#[derive(Clone, Copy, Debug)] -pub enum ValueSeparator { - Comma, - Space, + use_quotes: bool, } impl<'a> SshdConfigValue<'a> { /// Create a new SSHD config value, returning an error if the value is empty/invalid - pub fn try_new(key: &'a str, value: &'a Value, override_separator: Option) -> Result { - if matches!(value, Value::Null | Value::Object(_)) { + pub fn try_new(key: &'a str, value: &'a Value, override_separator: Option, use_quotes: bool) -> Result { + // Structured keywords (objects with name/value) are allowed + let is_structured_value = if let Value::Object(obj) = value { + obj.contains_key("name") && obj.contains_key("value") + } else { + false + }; + + if matches!(value, Value::Null) || (matches!(value, Value::Object(_)) && !is_structured_value) { return Err(SshdConfigError::ParserError( t!("formatter.invalidValue", key = key).to_string() )); @@ -48,32 +49,26 @@ impl<'a> SshdConfigValue<'a> { } } - let separator = match override_separator { - Some(separator) => separator, - None => { - if MULTI_ARG_KEYWORDS_COMMA_SEP.contains(&key) { - ValueSeparator::Comma - } else { - ValueSeparator::Space - } - } - }; + let mut keyword_info = KeywordInfo::from_keyword(key); - let is_repeatable = REPEATABLE_KEYWORDS.contains(&key); + // Allow separator override + if let Some(separator) = override_separator { + keyword_info.separator = separator; + } Ok(Self { - is_repeatable, + keyword_info, key, - separator, value, + use_quotes, }) } pub fn write_to_config(&self, config_text: &mut String) -> Result<(), SshdConfigError> { - if self.is_repeatable { + if self.keyword_info.is_repeatable { if let Value::Array(arr) = self.value { for item in arr { - let item = SshdConfigValue::try_new(self.key, item, Some(self.separator))?; + let item = SshdConfigValue::try_new(self.key, item, Some(self.keyword_info.separator), self.use_quotes)?; writeln!(config_text, "{} {item}", self.key)?; } } else { @@ -89,19 +84,34 @@ impl<'a> SshdConfigValue<'a> { impl fmt::Display for SshdConfigValue<'_> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self.value { + Value::Object(obj) => { + // Handle structured keywords (e.g., subsystem with name/value) + if let (Some(name), Some(value)) = (obj.get("name"), obj.get("value")) { + if let Ok(name_formatted) = SshdConfigValue::try_new(self.key, name, Some(self.keyword_info.separator), false) { + write!(f, "{name_formatted} ")?; + } + if let Ok(value_formatted) = SshdConfigValue::try_new(self.key, value, Some(self.keyword_info.separator), false) { + write!(f, "{value_formatted}")?; + } + Ok(()) + } else { + // Shouldn't happen for valid structured values + Ok(()) + } + }, Value::Array(arr) => { if arr.is_empty() { return Ok(()); } - let separator = match self.separator { + let separator = match self.keyword_info.separator { ValueSeparator::Comma => ",", ValueSeparator::Space => " ", }; let mut first = true; for item in arr { - if let Ok(sshd_config_value) = SshdConfigValue::try_new(self.key, item, Some(self.separator)) { + if let Ok(sshd_config_value) = SshdConfigValue::try_new(self.key, item, Some(self.keyword_info.separator), self.use_quotes) { let formatted = sshd_config_value.to_string(); if !formatted.is_empty() { if !first { @@ -119,13 +129,13 @@ impl fmt::Display for SshdConfigValue<'_> { Value::Bool(b) => write!(f, "{}", if *b { "yes" } else { "no" }), Value::Number(n) => write!(f, "{n}"), Value::String(s) => { - if s.contains(char::is_whitespace) { + if self.use_quotes && s.contains(char::is_whitespace) { write!(f, "\"{s}\"") } else { write!(f, "{s}") } }, - Value::Null | Value::Object(_) => Ok(()), + Value::Null => Ok(()), } } } @@ -149,7 +159,7 @@ fn format_match_block(match_obj: &Value) -> Result { for (key, value) in &match_block.criteria { // all match criteria values are comma-separated - let sshd_config_value = SshdConfigValue::try_new(key, value, Some(ValueSeparator::Comma))?; + let sshd_config_value = SshdConfigValue::try_new(key, value, Some(ValueSeparator::Comma), true)?; match_parts.push(format!("{key} {sshd_config_value}")); } @@ -158,7 +168,7 @@ fn format_match_block(match_obj: &Value) -> Result { // Format other keywords in the match block for (key, value) in &match_block.contents { - let sshd_config_value = SshdConfigValue::try_new(key, value, None)?; + let sshd_config_value = SshdConfigValue::try_new(key, value, None, true)?; result.push(format!("\t{key} {sshd_config_value}")); } @@ -181,7 +191,7 @@ pub fn write_config_map_to_text(global_map: &Map) -> Result, setting: &Setting) -> Result { get_default_shell()?; Ok(Map::new()) + }, + _ => { + Err(SshdConfigError::InvalidInput(t!("get.invalidSetting", setting = format!("{:?}", setting)).to_string())) } } } diff --git a/resources/sshdconfig/src/inputs.rs b/resources/sshdconfig/src/inputs.rs index 8c7bc8909..b70b4f8d2 100644 --- a/resources/sshdconfig/src/inputs.rs +++ b/resources/sshdconfig/src/inputs.rs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use serde_json::{Map, Value}; use std::path::PathBuf; @@ -44,7 +45,7 @@ impl CommandInfo { } } -#[derive(Debug, Default, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[derive(Debug, Default, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema)] pub struct Metadata { /// Filepath for the `sshd_config` file to be processed #[serde(skip_serializing_if = "Option::is_none")] @@ -69,3 +70,41 @@ pub struct SshdCommandArgs { #[serde(rename = "additionalArgs", skip_serializing_if = "Option::is_none")] pub additional_args: Option>, } + +/// A name-value entry for structured keywords like subsystem. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct NameValueEntry { + /// The entry name (e.g., subsystem name like "sftp") + pub name: String, + /// The entry value (e.g., subsystem command path and args) + #[serde(skip_serializing_if = "Option::is_none")] + pub value: Option, +} + +/// Input for name-value keyword single-entry operations (e.g., subsystem). +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] +pub struct RepeatInput { + /// Whether the entry should exist (true) or be removed (false) + #[serde(rename = "_exist", default)] + pub exist: bool, + /// Metadata for the operation + #[serde(rename = "_metadata", skip_serializing_if = "Option::is_none")] + pub metadata: Option, + /// The keyword and its entry (e.g., "subsystem": {"name": "sftp", "value": "/usr/bin/sftp"}) + #[serde(flatten)] + pub additional_properties: Map, +} + +/// Input for name-value keyword list operations with purge support (e.g., subsystem list). +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] +pub struct RepeatListInput { + /// Whether to remove entries not in the input list + #[serde(rename = "_purge", default)] + pub purge: bool, + /// Metadata for the operation + #[serde(rename = "_metadata", skip_serializing_if = "Option::is_none")] + pub metadata: Option, + /// The keyword and its array of entries (e.g., "subsystem": [{"name": "sftp", "value": "..."}]) + #[serde(flatten)] + pub additional_properties: Map, +} diff --git a/resources/sshdconfig/src/main.rs b/resources/sshdconfig/src/main.rs index 0ca12da5c..c127c2ada 100644 --- a/resources/sshdconfig/src/main.rs +++ b/resources/sshdconfig/src/main.rs @@ -8,6 +8,8 @@ use serde_json::Map; use std::process::exit; use tracing::{debug, error}; +use crate::inputs::{RepeatInput, RepeatListInput}; + use args::{Args, Command, DefaultShell, Setting}; use get::{get_sshd_settings, invoke_get}; use parser::SshdConfigParser; @@ -51,6 +53,12 @@ fn main() { Setting::SshdConfig => { schema_for!(SshdConfigParser) }, + Setting::SshdConfigRepeat => { + schema_for!(RepeatInput) + }, + Setting::SshdConfigRepeatList => { + schema_for!(RepeatListInput) + }, Setting::WindowsGlobal => { schema_for!(DefaultShell) } diff --git a/resources/sshdconfig/src/metadata.rs b/resources/sshdconfig/src/metadata.rs index 87c2b54a7..a5401d0c4 100644 --- a/resources/sshdconfig/src/metadata.rs +++ b/resources/sshdconfig/src/metadata.rs @@ -50,6 +50,63 @@ pub const REPEATABLE_KEYWORDS: [&str; 12] = [ "subsystem" ]; +// keywords that require structured name-value format (e.g., subsystem has a name and a command value). +pub const STRUCTURED_KEYWORDS: [&str; 1] = [ + "subsystem" +]; + +#[derive(Clone, Copy, Debug)] +pub enum ValueSeparator { + Comma, + Space, +} + +#[derive(Clone, Debug)] +pub struct KeywordInfo { + pub name: String, + pub is_repeatable: bool, + pub is_structured: bool, + pub separator: ValueSeparator, +} + +impl KeywordInfo { + /// Create a new `KeywordInfo` from a keyword string. + pub fn from_keyword(keyword: &str) -> Self { + let lowercase_key = keyword.to_lowercase(); + let is_repeatable = REPEATABLE_KEYWORDS.contains(&lowercase_key.as_str()); + let is_structured = STRUCTURED_KEYWORDS.contains(&lowercase_key.as_str()); + let separator = if MULTI_ARG_KEYWORDS_COMMA_SEP.contains(&lowercase_key.as_str()) { + ValueSeparator::Comma + } else { + ValueSeparator::Space + }; + + Self { + name: lowercase_key, + is_repeatable, + is_structured, + separator, + } + } + + /// Check if this keyword allows operator syntax (+, -, ^). + pub fn allows_operator(&self) -> bool { + !self.is_structured + } + + /// Check if this keyword requires structured name-value format. + pub fn requires_structured_format(&self) -> bool { + self.is_structured + } + + /// Check if this keyword can have multiple arguments. + pub fn is_multi_arg(&self) -> bool { + self.is_repeatable || + MULTI_ARG_KEYWORDS_COMMA_SEP.contains(&self.name.as_str()) || + MULTI_ARG_KEYWORDS_SPACE_SEP.contains(&self.name.as_str()) + } +} + pub const SSHD_CONFIG_HEADER: &str = "# This file is managed by the Microsoft.OpenSSH.SSHD/sshd_config DSC Resource"; pub const SSHD_CONFIG_HEADER_VERSION: &str = concat!("# The Microsoft.OpenSSH.SSHD/sshd_config DSC Resource version is ", env!("CARGO_PKG_VERSION")); pub const SSHD_CONFIG_HEADER_WARNING: &str = "# Please do not modify manually, as any changes may be overwritten"; diff --git a/resources/sshdconfig/src/parser.rs b/resources/sshdconfig/src/parser.rs index bb90f382c..5a5fa1d29 100644 --- a/resources/sshdconfig/src/parser.rs +++ b/resources/sshdconfig/src/parser.rs @@ -8,7 +8,14 @@ use tracing::debug; use tree_sitter::Parser; use crate::error::SshdConfigError; -use crate::metadata::{MULTI_ARG_KEYWORDS_COMMA_SEP, MULTI_ARG_KEYWORDS_SPACE_SEP, REPEATABLE_KEYWORDS}; +use crate::metadata::KeywordInfo; + +/// Unescape backslashes in strings. sshd -T outputs paths with escaped backslashes +/// (e.g., "c:\\openssh\\bin\\sftp.exe") but we want to normalize them to the original +/// form (e.g., "c:\openssh\bin\sftp.exe") for comparison and storage. +fn unescape_backslashes(s: &str) -> String { + s.replace("\\\\", "\\") +} #[derive(Debug, JsonSchema)] pub struct SshdConfigParser { @@ -114,7 +121,12 @@ impl SshdConfigParser { let values: Value; if let Some(value_node) = criteria_node.child_by_field_name("argument") { - values = parse_arguments_node(value_node, input, input_bytes, true)?; + // Match criteria are always treated as arrays (comma-separated), so we override + // the keyword_info to force multi-arg behavior + let mut keyword_info = KeywordInfo::from_keyword(&key); + // Force comma separator and multi-arg for match criteria + keyword_info.separator = crate::metadata::ValueSeparator::Comma; + values = parse_arguments_node_as_array(value_node, input, input_bytes, &keyword_info)?; } else { return Err(SshdConfigError::ParserError(t!("parser.missingValueInCriteria", input = input).to_string())); @@ -139,17 +151,15 @@ impl SshdConfigParser { let mut cursor = keyword_node.walk(); let mut key = None; let mut value = Value::Null; - let mut is_repeatable = false; - let mut is_vec = false; + let mut keyword_info: Option = None; let mut operator: Option = None; if let Some(keyword) = keyword_node.child_by_field_name("keyword") { let Ok(text) = keyword.utf8_text(input_bytes) else { return Err(SshdConfigError::ParserError(t!("parser.failedToParseNode", input = input).to_string())); }; - let lowercase_key = text.to_lowercase(); - is_repeatable = REPEATABLE_KEYWORDS.contains(&lowercase_key.as_str()); - is_vec = is_repeatable || MULTI_ARG_KEYWORDS_COMMA_SEP.contains(&lowercase_key.as_str()) || MULTI_ARG_KEYWORDS_SPACE_SEP.contains(&lowercase_key.as_str()); + let info = KeywordInfo::from_keyword(text); + keyword_info = Some(info); key = Some(text.to_string()); } @@ -163,13 +173,24 @@ impl SshdConfigParser { operator = Some(op_text.to_string()); } + // Validate that structured keywords cannot use operators + if let Some(ref info) = keyword_info { + if !info.allows_operator() && operator.is_some() { + return Err(SshdConfigError::ParserError( + t!("parser.structuredKeywordCannotUseOperator", keyword = &info.name).to_string() + )); + } + } + for node in keyword_node.named_children(&mut cursor) { if node.is_error() { return Err(SshdConfigError::ParserError(t!("parser.failedToParseNode", input = input).to_string())); } if node.kind() == "arguments" { - value = parse_arguments_node(node, input, input_bytes, is_vec)?; - debug!("{}: {:?}", t!("parser.valueDebug").to_string(), value); + if let Some(ref info) = keyword_info { + value = parse_arguments_node(node, input, input_bytes, info)?; + debug!("{}: {:?}", t!("parser.valueDebug").to_string(), value); + } } } @@ -188,6 +209,7 @@ impl SshdConfigParser { // If target_map is provided, insert the value with repeatability handling if let Some(map) = target_map { + let is_repeatable = keyword_info.as_ref().is_some_and(|i| i.is_repeatable); Self::insert_into_map(map, &key, value.clone(), is_repeatable)?; } @@ -237,14 +259,17 @@ impl SshdConfigParser { } } -fn parse_arguments_node(arg_node: tree_sitter::Node, input: &str, input_bytes: &[u8], is_vec: bool) -> Result { +fn parse_arguments_node(arg_node: tree_sitter::Node, input: &str, input_bytes: &[u8], keyword_info: &KeywordInfo) -> Result { let mut cursor = arg_node.walk(); let mut vec: Vec = Vec::new(); + let is_vec = keyword_info.is_multi_arg(); + // if there is more than one argument, but a vector is not expected for the keyword, throw an error let children: Vec<_> = arg_node.named_children(&mut cursor).collect(); if children.len() > 1 && !is_vec { return Err(SshdConfigError::ParserError(t!("parser.invalidMultiArgNode", input = input).to_string())); } + for node in &children { if node.is_error() { return Err(SshdConfigError::ParserError(t!("parser.failedToParseNode", input = input).to_string())); @@ -257,7 +282,9 @@ fn parse_arguments_node(arg_node: tree_sitter::Node, input: &str, input_bytes: & } "string" => { let arg_str = arg.trim(); - vec.push(Value::String(arg_str.to_string())); + // Unescape backslashes (sshd -T escapes them on Windows) + let unescaped = unescape_backslashes(arg_str); + vec.push(Value::String(unescaped)); }, "number" => { vec.push(Value::Number(arg.parse::()?.into())); @@ -265,6 +292,35 @@ fn parse_arguments_node(arg_node: tree_sitter::Node, input: &str, input_bytes: & _ => return Err(SshdConfigError::ParserError(t!("parser.unknownNode", kind = node.kind()).to_string())) } } + + // Handle structured keywords (e.g., subsystem) + if keyword_info.requires_structured_format() { + if vec.len() < 2 { + return Err(SshdConfigError::ParserError( + t!("parser.structuredKeywordRequiresMinArgs", keyword = &keyword_info.name).to_string() + )); + } + + // Extract name (first element) and value (remaining elements joined with space) + let name = vec[0].clone(); + let value_parts: Vec = vec[1..].iter() + .filter_map(|v| { + match v { + Value::String(s) => Some(s.clone()), + Value::Number(n) => Some(n.to_string()), + Value::Bool(b) => Some(if *b { "yes".to_string() } else { "no".to_string() }), + _ => None + } + }) + .collect(); + let value_str = value_parts.join(" "); + + let mut structured = Map::new(); + structured.insert("name".to_string(), name); + structured.insert("value".to_string(), Value::String(value_str)); + return Ok(Value::Object(structured)); + } + // Always return array if is_vec is true (for MULTI_ARG_KEYWORDS, and REPEATABLE_KEYWORDS) if is_vec { Ok(Value::Array(vec)) @@ -275,6 +331,41 @@ fn parse_arguments_node(arg_node: tree_sitter::Node, input: &str, input_bytes: & } } +/// Parse arguments node for match criteria, always returning an array. +/// Match criteria are always comma-separated and should be arrays. +fn parse_arguments_node_as_array(arg_node: tree_sitter::Node, input: &str, input_bytes: &[u8], _keyword_info: &KeywordInfo) -> Result { + let mut cursor = arg_node.walk(); + let mut vec: Vec = Vec::new(); + + let children: Vec<_> = arg_node.named_children(&mut cursor).collect(); + + for node in &children { + if node.is_error() { + return Err(SshdConfigError::ParserError(t!("parser.failedToParseNode", input = input).to_string())); + } + let arg = node.utf8_text(input_bytes)?; + match node.kind() { + "boolean" => { + let arg_str = arg.trim(); + vec.push(Value::Bool(arg_str.eq_ignore_ascii_case("yes"))); + } + "string" => { + let arg_str = arg.trim(); + // Unescape backslashes (sshd -T escapes them on Windows) + let unescaped = unescape_backslashes(arg_str); + vec.push(Value::String(unescaped)); + }, + "number" => { + vec.push(Value::Number(arg.parse::()?.into())); + }, + _ => return Err(SshdConfigError::ParserError(t!("parser.unknownNode", kind = node.kind()).to_string())) + } + } + + // Always return array for match criteria + Ok(Value::Array(vec)) +} + /// Parse `sshd_config` to map. /// /// # Arguments @@ -308,6 +399,53 @@ mod tests { assert_eq!(result.get("port").unwrap(), &Value::Array(expected)); } + #[test] + fn subsystem_keyword() { + let input = r#" + subsystem sftp /usr/bin/sftp + subsystem pwsh c:/progra~1/powershell/7/pwsh.exe -sshs -NoLogo -NoProfile + "#.trim_start(); + let result: Map = parse_text_to_map(input).unwrap(); + let subsystems = result.get("subsystem").unwrap().as_array().unwrap(); + assert_eq!(subsystems.len(), 2); + + // Check first subsystem + let sftp = subsystems[0].as_object().unwrap(); + assert_eq!(sftp.get("name").unwrap(), &Value::String("sftp".to_string())); + assert_eq!(sftp.get("value").unwrap(), &Value::String("/usr/bin/sftp".to_string())); + + // Check second subsystem with multiple arguments + let pwsh = subsystems[1].as_object().unwrap(); + assert_eq!(pwsh.get("name").unwrap(), &Value::String("pwsh".to_string())); + assert_eq!(pwsh.get("value").unwrap(), &Value::String("c:/progra~1/powershell/7/pwsh.exe -sshs -NoLogo -NoProfile".to_string())); + } + + #[test] + fn subsystem_single_entry() { + let input = "subsystem sftp /usr/lib/openssh/sftp-server\n"; + let result: Map = parse_text_to_map(input).unwrap(); + let subsystems = result.get("subsystem").unwrap().as_array().unwrap(); + assert_eq!(subsystems.len(), 1); + + let sftp = subsystems[0].as_object().unwrap(); + assert_eq!(sftp.get("name").unwrap(), &Value::String("sftp".to_string())); + assert_eq!(sftp.get("value").unwrap(), &Value::String("/usr/lib/openssh/sftp-server".to_string())); + } + + #[test] + fn subsystem_name_only_should_error() { + let input = "subsystem sftp\n"; + let result = parse_text_to_map(input); + assert!(result.is_err()); + } + + #[test] + fn subsystem_with_operator_should_error() { + let input = "subsystem +sftp /usr/bin/sftp\n"; + let result = parse_text_to_map(input); + assert!(result.is_err()); + } + #[test] fn multiarg_string_keyword() { let input = "hostkeyalgorithms ssh-ed25519-cert-v01@openssh.com,ecdsa-sha2-nistp256-cert-v01@openssh.com\r\n"; @@ -570,4 +708,67 @@ match group administrators assert_eq!(value_array.len(), 1); assert_eq!(value_array[0], Value::String("ssh-rsa".to_string())); } + + #[test] + fn subsystem_in_match_block() { + let input = r#" +match user alice + subsystem sftp /usr/bin/sftp +"#; + let result: Map = parse_text_to_map(input).unwrap(); + let match_array = result.get("match").unwrap().as_array().unwrap(); + let match_obj = match_array[0].as_object().unwrap(); + + let subsystems = match_obj.get("subsystem").unwrap().as_array().unwrap(); + assert_eq!(subsystems.len(), 1); + + let sftp = subsystems[0].as_object().unwrap(); + assert_eq!(sftp.get("name").unwrap(), &Value::String("sftp".to_string())); + assert_eq!(sftp.get("value").unwrap(), &Value::String("/usr/bin/sftp".to_string())); + } + + #[test] + fn subsystem_multiple_in_match_block() { + let input = r#" +match user developer + subsystem sftp /usr/bin/sftp + subsystem pwsh /usr/bin/pwsh -sshs +"#; + let result: Map = parse_text_to_map(input).unwrap(); + let match_array = result.get("match").unwrap().as_array().unwrap(); + let match_obj = match_array[0].as_object().unwrap(); + + let subsystems = match_obj.get("subsystem").unwrap().as_array().unwrap(); + assert_eq!(subsystems.len(), 2); + + let sftp = subsystems[0].as_object().unwrap(); + assert_eq!(sftp.get("name").unwrap(), &Value::String("sftp".to_string())); + assert_eq!(sftp.get("value").unwrap(), &Value::String("/usr/bin/sftp".to_string())); + + let pwsh = subsystems[1].as_object().unwrap(); + assert_eq!(pwsh.get("name").unwrap(), &Value::String("pwsh".to_string())); + assert_eq!(pwsh.get("value").unwrap(), &Value::String("/usr/bin/pwsh -sshs".to_string())); + } + + #[test] + fn subsystem_roundtrip() { + use crate::formatter::write_config_map_to_text; + + let input = r#"subsystem sftp /usr/bin/sftp +subsystem pwsh c:/progra~1/powershell/7/pwsh.exe -sshs -NoLogo +"#; + let parsed = parse_text_to_map(input).unwrap(); + let formatted = write_config_map_to_text(&parsed).unwrap(); + + // Parse again to verify round-trip + let reparsed = parse_text_to_map(&formatted).unwrap(); + + let subsystems1 = parsed.get("subsystem").unwrap().as_array().unwrap(); + let subsystems2 = reparsed.get("subsystem").unwrap().as_array().unwrap(); + + assert_eq!(subsystems1.len(), subsystems2.len()); + assert_eq!(subsystems1[0], subsystems2[0]); + assert_eq!(subsystems1[1], subsystems2[1]); + } } + diff --git a/resources/sshdconfig/src/set.rs b/resources/sshdconfig/src/set.rs index 2950b795d..bfb92043b 100644 --- a/resources/sshdconfig/src/set.rs +++ b/resources/sshdconfig/src/set.rs @@ -10,15 +10,15 @@ use { use rust_i18n::t; use serde_json::{Map, Value}; -use std::string::String; +use std::{path::PathBuf, string::String}; use tracing::{debug, info, warn}; use crate::args::{DefaultShell, Setting}; use crate::error::SshdConfigError; use crate::formatter::write_config_map_to_text; use crate::get::get_sshd_settings; -use crate::inputs::{CommandInfo, SshdCommandArgs}; -use crate::metadata::{MULTI_ARG_KEYWORDS_COMMA_SEP, MULTI_ARG_KEYWORDS_SPACE_SEP, REPEATABLE_KEYWORDS, SSHD_CONFIG_HEADER, SSHD_CONFIG_HEADER_VERSION, SSHD_CONFIG_HEADER_WARNING}; +use crate::inputs::{CommandInfo, NameValueEntry, RepeatInput, RepeatListInput, SshdCommandArgs}; +use crate::metadata::{SSHD_CONFIG_HEADER, SSHD_CONFIG_HEADER_VERSION, SSHD_CONFIG_HEADER_WARNING}; use crate::util::{build_command_info, get_default_sshd_config_path, invoke_sshd_config_validation}; /// Invoke the set command. @@ -36,6 +36,16 @@ pub fn invoke_set(input: &str, setting: &Setting) -> Result, Err(e) => Err(e), } }, + Setting::SshdConfigRepeat => { + debug!("{} {:?}", t!("set.settingSshdConfig").to_string(), setting); + let cmd_info = build_command_info(Some(&input.to_string()), false)?; + set_sshd_config_repeat(input, &cmd_info) + }, + Setting::SshdConfigRepeatList => { + debug!("{} {:?}", t!("set.settingSshdConfig").to_string(), setting); + let cmd_info = build_command_info(Some(&input.to_string()), false)?; + set_sshd_config_repeat_list(input, &cmd_info) + }, Setting::WindowsGlobal => { debug!("{} {:?}", t!("set.settingDefaultShell").to_string(), setting); match serde_json::from_str::(input) { @@ -53,6 +63,65 @@ pub fn invoke_set(input: &str, setting: &Setting) -> Result, } } +/// Handle single name-value keyword entry operations (add or remove). +fn set_sshd_config_repeat(input: &str, cmd_info: &CommandInfo) -> Result, SshdConfigError> { + let keyword_input: RepeatInput = serde_json::from_str(input) + .map_err(|e| SshdConfigError::InvalidInput(t!("set.failedToParse", input = e.to_string()).to_string()))?; + + let (keyword, entry_value) = extract_single_keyword(keyword_input.additional_properties)?; + + let mut existing_config = get_existing_config(cmd_info)?; + + // parses entry for name-value keywords, like subsystem, for now + // different keywords will likely need to be serialized into different structs + // and likely need to have different add/update/remove functions + let entry: NameValueEntry = serde_json::from_value(entry_value) + .map_err(|e| SshdConfigError::InvalidInput(t!("set.failedToParse", input = e.to_string()).to_string()))?; + + if keyword_input.exist { + add_or_update_entry(&mut existing_config, &keyword, &entry)?; + } else { + remove_entry(&mut existing_config, &keyword, &entry.name); + } + + write_and_validate_config(&mut existing_config, cmd_info.metadata.filepath.as_ref())?; + Ok(Map::new()) +} + +/// Handle list name-value keyword operations with purge support. +fn set_sshd_config_repeat_list(input: &str, cmd_info: &CommandInfo) -> Result, SshdConfigError> { + let list_input: RepeatListInput = serde_json::from_str(input) + .map_err(|e| SshdConfigError::InvalidInput(t!("set.failedToParse", input = e.to_string()).to_string()))?; + + let (keyword, entries_value) = extract_single_keyword(list_input.additional_properties)?; + + let mut existing_config = get_existing_config(cmd_info)?; + + // Ensure it's an array + let Value::Array(ref entries_array) = entries_value else { + return Err(SshdConfigError::InvalidInput( + t!("set.expectedArrayForKeyword", keyword = keyword).to_string() + )); + }; + + // Apply the changes based on _purge flag + if list_input.purge { + if entries_array.is_empty() { + existing_config.remove(&keyword); + } else { + existing_config.insert(keyword, entries_value); + } + } else { + let entries = parse_and_validate_entries(entries_array)?; + for entry in entries { + add_or_update_entry(&mut existing_config, &keyword, &entry)?; + } + } + + write_and_validate_config(&mut existing_config, cmd_info.metadata.filepath.as_ref())?; + Ok(Map::new()) +} + #[cfg(windows)] fn set_default_shell(shell: String, cmd_option: Option, escape_arguments: Option) -> Result<(), SshdConfigError> { debug!("{}", t!("set.settingDefaultShell")); @@ -112,10 +181,8 @@ fn set_sshd_config(cmd_info: &mut CommandInfo) -> Result<(), SshdConfigError> { // this should be its own helper function that checks that the value makes sense for the key type // i.e. if the key can be repeated or have multiple values, etc. // or if the value is something besides a string (like an object to convert back into a comma-separated list) - debug!("{}", t!("set.writingTempConfig")); - let mut config_text = SSHD_CONFIG_HEADER.to_string() + "\n" + SSHD_CONFIG_HEADER_VERSION + "\n" + SSHD_CONFIG_HEADER_WARNING + "\n"; - if cmd_info.purge { - config_text.push_str(&write_config_map_to_text(&cmd_info.input)?); + let mut config_to_write = if cmd_info.purge { + cmd_info.input.clone() } else { let mut get_cmd_info = cmd_info.clone(); get_cmd_info.include_defaults = false; @@ -131,26 +198,27 @@ fn set_sshd_config(cmd_info: &mut CommandInfo) -> Result<(), SshdConfigError> { Err(e) => return Err(e), }; for (key, value) in &cmd_info.input { - let key_contains = key.as_str(); - - // TODO: remove when design for handling repeatable and multi-arg keywords is finalized - // and consider using SshdConfigValue instead of any remaining contains() checks - if REPEATABLE_KEYWORDS.contains(&key_contains) - || MULTI_ARG_KEYWORDS_COMMA_SEP.contains(&key_contains) - || MULTI_ARG_KEYWORDS_SPACE_SEP.contains(&key_contains) { - return Err(SshdConfigError::InvalidInput(t!("set.purgeFalseUnsupported").to_string())); - } - if value.is_null() { existing_config.remove(key); } else { existing_config.insert(key.clone(), value.clone()); } } - existing_config.remove("_metadata"); - existing_config.remove("_inheritedDefaults"); - config_text.push_str(&write_config_map_to_text(&existing_config)?); - } + existing_config + }; + + write_and_validate_config(&mut config_to_write, cmd_info.metadata.filepath.as_ref()) +} + +/// Write configuration to file after validation. +fn write_and_validate_config(config: &mut Map, filepath: Option<&PathBuf>) -> Result<(), SshdConfigError> { + debug!("{}", t!("set.writingTempConfig")); + config.remove("_purge"); + config.remove("_exist"); + config.remove("_inheritedDefaults"); + config.remove("_metadata"); + let mut config_text = SSHD_CONFIG_HEADER.to_string() + "\n" + SSHD_CONFIG_HEADER_VERSION + "\n" + SSHD_CONFIG_HEADER_WARNING + "\n"; + config_text.push_str(&write_config_map_to_text(config)?); // Write input to a temporary file and validate it with SSHD -T let temp_file = tempfile::Builder::new() @@ -180,7 +248,7 @@ fn set_sshd_config(cmd_info: &mut CommandInfo) -> Result<(), SshdConfigError> { // Propagate failure, if any result?; - let sshd_config_path = get_default_sshd_config_path(cmd_info.metadata.filepath.clone())?; + let sshd_config_path = get_default_sshd_config_path(filepath.cloned())?; if sshd_config_path.exists() { let mut sshd_config_content = String::new(); @@ -208,3 +276,110 @@ fn set_sshd_config(cmd_info: &mut CommandInfo) -> Result<(), SshdConfigError> { Ok(()) } + +/// Extract and validate a single keyword from `additional_properties`. +fn extract_single_keyword(additional_properties: Map) -> Result<(String, Value), SshdConfigError> { + let mut keywords: Vec<(String, Value)> = additional_properties.into_iter().collect(); + + if keywords.is_empty() { + return Err(SshdConfigError::InvalidInput(t!("set.noKeywordFoundInInput").to_string())); + } + + if keywords.len() > 1 { + return Err(SshdConfigError::InvalidInput( + t!("set.multipleKeywordsNotAllowed", count = keywords.len()).to_string() + )); + } + + Ok(keywords.remove(0)) +} + +/// Find the index of a name-value entry in a keyword array by matching the name field (case-sensitive). +fn find_name_value_entry_index(keyword_array: &[Value], entry_name: &str) -> Option { + keyword_array.iter().position(|item| { + if let Value::Object(obj) = item { + if let Some(Value::String(name)) = obj.get("name") { + return name == entry_name; + } + } + false + }) +} + +/// Add or update a name-value entry in the config map. +fn add_or_update_entry(config: &mut Map, keyword: &str, entry: &NameValueEntry) -> Result<(), SshdConfigError> { + if entry.value.is_none() { + return Err(SshdConfigError::InvalidInput( + t!("set.nameValueEntryRequiresValue").to_string() + )); + } + + let entry_value = serde_json::to_value(entry)?; + + if let Some(existing) = config.get_mut(keyword) { + if let Value::Array(arr) = existing { + if let Some(index) = find_name_value_entry_index(arr, &entry.name) { + // Entry exists, update it + arr[index] = entry_value; + } else { + // Entry doesn't exist, append it + arr.push(entry_value); + } + } else { + *existing = Value::Array(vec![entry_value]); + } + } else { + let new_array = Value::Array(vec![entry_value]); + config.insert(keyword.to_string(), new_array); + } + Ok(()) +} + +/// Remove a keyword entry based on the keyword's name field. +fn remove_entry(config: &mut Map, keyword: &str, entry_name: &str) { + if let Some(Value::Array(arr)) = config.get_mut(keyword) { + if let Some(index) = find_name_value_entry_index(arr, entry_name) { + arr.remove(index); + } + } +} + +/// Get existing config from file or return empty map if file doesn't exist. +fn get_existing_config(cmd_info: &CommandInfo) -> Result, SshdConfigError> { + match get_sshd_settings(cmd_info, false) { + Ok(config) => Ok(config), + Err(SshdConfigError::FileNotFound(_)) => { + // If file doesn't exist, create empty config + Ok(Map::new()) + } + Err(e) => Err(e), + } +} + +/// Parse and validate an array of name-value entries. +fn parse_and_validate_entries(entries_array: &[Value]) -> Result, SshdConfigError> { + let mut entries: Vec = Vec::new(); + + for entry_value in entries_array { + let entry: NameValueEntry = serde_json::from_value(entry_value.clone()) + .map_err(|e| SshdConfigError::InvalidInput(t!("set.failedToParse", input = e.to_string()).to_string()))?; + + // Validate required name field + if entry.name.is_empty() { + return Err(SshdConfigError::InvalidInput( + t!("set.entryNameRequired").to_string() + )); + } + + // Validate value field is present + if entry.value.is_none() || entry.value.as_ref().unwrap().is_empty() { + return Err(SshdConfigError::InvalidInput( + t!("set.entryValueRequired", name = entry.name).to_string() + )); + } + + entries.push(entry); + } + + Ok(entries) +} diff --git a/resources/sshdconfig/sshd-subsystem.dsc.resource.json b/resources/sshdconfig/sshd-subsystem.dsc.resource.json new file mode 100644 index 000000000..36997ecdb --- /dev/null +++ b/resources/sshdconfig/sshd-subsystem.dsc.resource.json @@ -0,0 +1,58 @@ +{ + "$schema": "https://aka.ms/dsc/schemas/v3/bundled/resource/manifest.json", + "type": "Microsoft.OpenSSH.SSHD/Subsystem", + "description": "Manage SSH Server Subsystem keyword (single entry)", + "version": "0.1.0", + "set": { + "executable": "sshdconfig", + "args": [ + "set", + "-s", + "sshd-config-repeat", + { + "jsonInputArg": "--input", + "mandatory": true + } + ] + }, + "schema": { + "embedded": { + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "sshdconfig.subsystem", + "type": "object", + "properties": { + "_exist": { + "type": "boolean", + "description": "Whether the specified subsystem entry should exist (true) or be removed (false)", + "default": true + }, + "_metadata": { + "type": "object", + "properties": { + "filepath": { + "type": "string", + "description": "Path to sshd_config file" + } + } + }, + "subsystem": { + "type": "object", + "description": "Subsystem definition with name (subsystem identifier) and value (command path and arguments)", + "properties": { + "name": { + "type": "string", + "description": "The subsystem name (e.g., 'sftp', 'powershell')" + }, + "value": { + "type": "string", + "description": "The subsystem command path and arguments (e.g., '/usr/bin/sftp' or 'c:/progra~1/powershell/7/pwsh.exe -sshs -NoLogo')" + } + }, + "required": ["name"], + "additionalProperties": false + } + }, + "additionalProperties": false + } + } +} diff --git a/resources/sshdconfig/sshd-subsystemList.dsc.resource.json b/resources/sshdconfig/sshd-subsystemList.dsc.resource.json new file mode 100644 index 000000000..0487b399b --- /dev/null +++ b/resources/sshdconfig/sshd-subsystemList.dsc.resource.json @@ -0,0 +1,62 @@ +{ + "$schema": "https://aka.ms/dsc/schemas/v3/bundled/resource/manifest.json", + "type": "Microsoft.OpenSSH.SSHD/SubsystemList", + "description": "Manage SSH Server Subsystem keyword (accepts multiple entries)", + "version": "0.1.0", + "set": { + "executable": "sshdconfig", + "args": [ + "set", + "-s", + "sshd-config-repeat-list", + { + "jsonInputArg": "--input", + "mandatory": true + } + ] + }, + "schema": { + "embedded": { + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "sshdconfig.subsystem", + "type": "object", + "properties": { + "_purge": { + "type": "boolean", + "description": "Whether to clear any existing subsystem entries not specified in the 'subsystem' array", + "default": false + }, + "_metadata": { + "type": "object", + "properties": { + "filepath": { + "type": "string", + "description": "Path to sshd_config file" + } + } + }, + "subsystem": { + "type": "array", + "description": "Array of subsystem definitions with name and command value", + "items": { + "type": "object", + "description": "Subsystem definition with name (subsystem identifier) and value (command path and arguments)", + "properties": { + "name": { + "type": "string", + "description": "The subsystem name (e.g., 'sftp', 'powershell')" + }, + "value": { + "type": "string", + "description": "The subsystem command path and arguments (e.g., '/usr/bin/sftp' or 'c:/progra~1/powershell/7/pwsh.exe -sshs -NoLogo')" + } + }, + "required": ["name"], + "additionalProperties": false + } + } + }, + "additionalProperties": false + } + } +} diff --git a/resources/sshdconfig/tests/sshdconfig.set.tests.ps1 b/resources/sshdconfig/tests/sshdconfig.set.tests.ps1 index 49e80728b..e90261bad 100644 --- a/resources/sshdconfig/tests/sshdconfig.set.tests.ps1 +++ b/resources/sshdconfig/tests/sshdconfig.set.tests.ps1 @@ -201,24 +201,6 @@ Describe 'sshd_config Set Tests' -Skip:($skipTest) { Remove-Item -Path $stderrFile -Force -ErrorAction SilentlyContinue } - It 'Should fail with purge set to false for repeatable keywords' { - $inputConfig = @{ - _metadata = @{ - filepath = $TestConfigPath - } - _purge = $false - Port = "8888" - } | ConvertTo-Json - - $logFile = Join-Path $TestDrive "purge_error.log" - sshdconfig set --input $inputConfig -s sshd-config 2>$logFile - $LASTEXITCODE | Should -Not -Be 0 - - # Read log file and check for error message - $logContent = Get-Content $logFile -Raw - $logContent | Should -Match "purge=false is not supported for keywords that can have multiple values" - } - It 'Should fail with invalid keyword and not modify file' { # Get original content $getInput = @{ @@ -361,4 +343,36 @@ Match Group administrators } } } + + Context 'Set overwrites repeatable keywords' { + BeforeEach { + $initialContent = @" + Port 2222 + AddressFamily inet + MaxAuthTries 5 + PermitRootLogin yes + PasswordAuthentication no + Match Group administrators + GSSAPIAuthentication yes +"@ + Set-Content -Path $TestConfigPath -Value $initialContent + } + + It 'Should overwrite all instances of a repeatable keyword' { + $inputConfig = @{ + _metadata = @{ + filepath = $TestConfigPath + } + _purge = $false + Port = @(8888, 9999) + } | ConvertTo-Json + + sshdconfig set --input $inputConfig -s sshd-config 2>$logFile + $LASTEXITCODE | Should -Be 0 + $sshdConfigContents = Get-Content $TestConfigPath + $sshdConfigContents | Should -Contain "port 8888" + $sshdConfigContents | Should -Contain "port 9999" + $sshdConfigContents | Should -Not -Contain "port 2222" + } + } } diff --git a/resources/sshdconfig/tests/sshdconfigRepeat.tests.ps1 b/resources/sshdconfig/tests/sshdconfigRepeat.tests.ps1 new file mode 100644 index 000000000..750df092c --- /dev/null +++ b/resources/sshdconfig/tests/sshdconfigRepeat.tests.ps1 @@ -0,0 +1,216 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +BeforeDiscovery { + if ($IsWindows) { + $identity = [System.Security.Principal.WindowsIdentity]::GetCurrent() + $principal = [System.Security.Principal.WindowsPrincipal]::new($identity) + $isElevated = $principal.IsInRole([System.Security.Principal.WindowsBuiltInRole]::Administrator) + } + else { + $isElevated = (id -u) -eq 0 + } + + $sshdExists = ($null -ne (Get-Command sshd -CommandType Application -ErrorAction Ignore)) + $skipTest = !$isElevated -or !$sshdExists +} + +Describe 'sshd-config-repeat Set Tests' -Skip:($skipTest) { + BeforeAll { + # Create a temporary test directory for sshd_config files + $TestDir = Join-Path $TestDrive "sshd_test" + New-Item -Path $TestDir -ItemType Directory -Force | Out-Null + $TestConfigPath = Join-Path $TestDir "sshd_config" + + # Define OS-specific paths with spaces + if ($IsWindows) { + $script:PathWithSpaces = "C:\Program Files\OpenSSH\sftp-server.exe" + $script:DefaultSftpPath = "sftp-server.exe" + $script:AlternatePath = "C:\OpenSSH\bin\sftp.exe" + } + else { + $script:PathWithSpaces = "/usr/local/lib/openssh server/sftp-server" + $script:DefaultSftpPath = "/usr/lib/openssh/sftp-server" + $script:AlternatePath = "/usr/libexec/sftp-server" + } + } + + AfterEach { + # Clean up test config file after each test + if (Test-Path $TestConfigPath) { + Remove-Item -Path $TestConfigPath -Force -ErrorAction SilentlyContinue + } + if (Test-Path "$TestConfigPath.bak") { + Remove-Item -Path "$TestConfigPath.bak" -Force -ErrorAction SilentlyContinue + } + } + + Context 'Subsystem keyword' { + BeforeEach { + # Create test config with existing subsystems + $initialContent = @" +Port 22 +Subsystem sftp $script:DefaultSftpPath +Subsystem test2 /path/to/test2 +PasswordAuthentication yes +"@ + Set-Content -Path $TestConfigPath -Value $initialContent + } + + It 'Should add a new subsystem that does not already exist' { + $inputConfig = @{ + _metadata = @{ + filepath = $TestConfigPath + } + _exist = $true + subsystem = @{ + name = "newsubsystem" + value = "/path/to/newsubsystem" + } + } | ConvertTo-Json + + $output = sshdconfig set --input $inputConfig -s sshd-config-repeat 2>$null + $LASTEXITCODE | Should -Be 0 + + # Verify file contains the new subsystem + $subsystems = Get-Content $TestConfigPath | Where-Object { $_ -match '^\s*subsystem\s+' } + $subsystems.Count | Should -Be 3 + $subsystems | Should -Contain "subsystem newsubsystem /path/to/newsubsystem" + # Verify existing subsystems are preserved + $subsystems | Should -Contain "subsystem sftp $script:DefaultSftpPath" + $subsystems | Should -Contain "Subsystem test2 /path/to/test2" + } + + It 'Should treat subsystem names as case-sensitive (SFTP is different from sftp)' { + $inputConfig = @{ + _metadata = @{ + filepath = $TestConfigPath + } + _exist = $true + subsystem = @{ + name = "SFTP" # Uppercase - should be treated as different from lowercase sftp + value = $script:AlternatePath + } + } | ConvertTo-Json + + $output = sshdconfig set --input $inputConfig -s sshd-config-repeat 2>$null + $LASTEXITCODE | Should -Be 0 + + # Verify SFTP was added as a new entry (not updating existing sftp) using get + $getInput = @{ + _metadata = @{ + filepath = $TestConfigPath + } + } | ConvertTo-Json + $result = sshdconfig get --input $getInput -s sshd-config 2>$null | ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 + + $result.subsystem.Count | Should -Be 3 # Original sftp + test2 + new SFTP + + # Verify both sftp and SFTP exist as separate entries + $sftpEntry = $result.subsystem | Where-Object { $_.name -ceq 'sftp' } + $sftpEntry | Should -Not -BeNullOrEmpty + $sftpEntry.value | Should -Be $script:DefaultSftpPath + + $SFTPEntry = $result.subsystem | Where-Object { $_.name -ceq 'SFTP' } + $SFTPEntry | Should -Not -BeNullOrEmpty + $SFTPEntry.value | Should -Be $script:AlternatePath + + $test2Entry = $result.subsystem | Where-Object { $_.name -ceq 'test2' } + $test2Entry | Should -Not -BeNullOrEmpty + $test2Entry.value | Should -Be '/path/to/test2' + } + + It 'Should remove a subsystem when _exist is false' { + $inputConfig = @{ + _metadata = @{ + filepath = $TestConfigPath + } + _exist = $false + subsystem = @{ + name = "sftp" + } + } | ConvertTo-Json + + $output = sshdconfig set --input $inputConfig -s sshd-config-repeat 2>$null + $LASTEXITCODE | Should -Be 0 + + # Verify subsystem was removed + $subsystems = Get-Content $TestConfigPath | Where-Object { $_ -match '^\s*subsystem\s+' } + $subsystems.Count | Should -Be 1 + $subsystems | Should -Not -Match 'sftp' + # Verify other subsystem is still present + $subsystems | Should -Contain "Subsystem test2 /path/to/test2" + } + + It 'Should preserve case when adding subsystem with mixed case name' { + $inputConfig = @{ + _metadata = @{ + filepath = $TestConfigPath + } + _exist = $true + subsystem = @{ + name = "MyCustomSubsystem" + value = "/path/to/custom" + } + } | ConvertTo-Json + + $output = sshdconfig set --input $inputConfig -s sshd-config-repeat 2>$null + $LASTEXITCODE | Should -Be 0 + + # Verify exact case is preserved in file + $content = Get-Content $TestConfigPath -Raw + $content | Should -Match "subsystem MyCustomSubsystem /path/to/custom" + } + + It 'Should handle paths with spaces correctly' { + $inputConfig = @{ + _metadata = @{ + filepath = $TestConfigPath + } + _exist = $true + subsystem = @{ + name = "spacepath" + value = $script:PathWithSpaces + } + } | ConvertTo-Json + + $output = sshdconfig set --input $inputConfig -s sshd-config-repeat 2>$null + $LASTEXITCODE | Should -Be 0 + + # Verify subsystem with space in path is present + $subsystems = Get-Content $TestConfigPath | Where-Object { $_ -match '^\s*subsystem\s+' } + $spacePathLine = $subsystems | Where-Object { $_ -match 'spacepath' } + $spacePathLine | Should -Not -BeNullOrEmpty + $spacePathLine | Should -Match ([regex]::Escape($script:PathWithSpaces)) + } + + It 'Should fail when subsystem name is missing' { + $inputConfig = @{ + _metadata = @{ + filepath = $TestConfigPath + } + _exist = $true + subsystem = @{ + value = "/path/to/something" + } + } | ConvertTo-Json + + $stderrFile = Join-Path $TestDrive "stderr_missing_name.txt" + sshdconfig set --input $inputConfig -s sshd-config-repeat 2>$stderrFile + $LASTEXITCODE | Should -Not -Be 0 + + Remove-Item -Path $stderrFile -Force -ErrorAction SilentlyContinue + } + + It 'Should fail with invalid JSON structure' { + $invalidJson = "{ invalid json }" + + $stderrFile = Join-Path $TestDrive "stderr_invalid_json.txt" + sshdconfig set --input $invalidJson -s sshd-config-repeat 2>$stderrFile + $LASTEXITCODE | Should -Not -Be 0 + + Remove-Item -Path $stderrFile -Force -ErrorAction SilentlyContinue + } + } +} diff --git a/resources/sshdconfig/tests/sshdconfigRepeatList.tests.ps1 b/resources/sshdconfig/tests/sshdconfigRepeatList.tests.ps1 new file mode 100644 index 000000000..b233e3044 --- /dev/null +++ b/resources/sshdconfig/tests/sshdconfigRepeatList.tests.ps1 @@ -0,0 +1,324 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +BeforeDiscovery { + if ($IsWindows) { + $identity = [System.Security.Principal.WindowsIdentity]::GetCurrent() + $principal = [System.Security.Principal.WindowsPrincipal]::new($identity) + $isElevated = $principal.IsInRole([System.Security.Principal.WindowsBuiltInRole]::Administrator) + } + else { + $isElevated = (id -u) -eq 0 + } + + $sshdExists = ($null -ne (Get-Command sshd -CommandType Application -ErrorAction Ignore)) + $skipTest = !$isElevated -or !$sshdExists +} + +Describe 'sshd-config-repeat-list Set Tests' -Skip:($skipTest) { + BeforeAll { + # Create a temporary test directory for sshd_config files + $TestDir = Join-Path $TestDrive "sshd_test" + New-Item -Path $TestDir -ItemType Directory -Force | Out-Null + $TestConfigPath = Join-Path $TestDir "sshd_config" + + # Define OS-specific paths with spaces + if ($IsWindows) { + $script:PathWithSpaces = "C:\Program Files\OpenSSH\sftp-server.exe" + $script:DefaultSftpPath = "sftp-server.exe" + $script:AlternatePath = "C:\OpenSSH\bin\sftp.exe" + } + else { + $script:PathWithSpaces = "/usr/local/lib/openssh server/sftp-server" + $script:DefaultSftpPath = "/usr/lib/openssh/sftp-server" + $script:AlternatePath = "/usr/libexec/sftp-server" + } + } + + AfterEach { + # Clean up test config file after each test + if (Test-Path $TestConfigPath) { + Remove-Item -Path $TestConfigPath -Force -ErrorAction SilentlyContinue + } + if (Test-Path "$TestConfigPath.bak") { + Remove-Item -Path "$TestConfigPath.bak" -Force -ErrorAction SilentlyContinue + } + } + + Context 'Subsystem keyword' { + BeforeEach { + # Create test config with existing subsystems + $initialContent = @" +Port 22 +subsystem sftp $script:DefaultSftpPath +Subsystem test2 /path/to/test2 +PasswordAuthentication yes +"@ + Set-Content -Path $TestConfigPath -Value $initialContent + } + + It 'Should add multiple new subsystems' { + $inputConfig = @{ + _metadata = @{ + filepath = $TestConfigPath + } + _purge = $false + subsystem = @( + @{ + name = "powershell" + value = "/usr/bin/pwsh -sshs -NoLogo" + }, + @{ + name = "newsub2" + value = "/path/to/newsub2" + } + ) + } | ConvertTo-Json -Depth 10 + + $output = sshdconfig set --input $inputConfig -s sshd-config-repeat-list 2>$null + $LASTEXITCODE | Should -Be 0 + + # Verify all subsystems are present (old + new) using get + $getInput = @{ + _metadata = @{ + filepath = $TestConfigPath + } + } | ConvertTo-Json + $result = sshdconfig get --input $getInput -s sshd-config 2>$null | ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 + + $result.subsystem.Count | Should -Be 4 # 2 existing + 2 new + + # Verify new subsystems were added + $newsub1 = $result.subsystem | Where-Object { $_.name -ceq 'powershell' } + $newsub1 | Should -Not -BeNullOrEmpty + $newsub1.value | Should -Be '/usr/bin/pwsh -sshs -NoLogo' + + $newsub2 = $result.subsystem | Where-Object { $_.name -ceq 'newsub2' } + $newsub2 | Should -Not -BeNullOrEmpty + $newsub2.value | Should -Be '/path/to/newsub2' + + # Verify existing subsystems are preserved + ($result.subsystem | Where-Object { $_.name -ceq 'sftp' }) | Should -Not -BeNullOrEmpty + ($result.subsystem | Where-Object { $_.name -ceq 'test2' }) | Should -Not -BeNullOrEmpty + } + + It 'Should update existing subsystems and add new ones' { + $inputConfig = @{ + _metadata = @{ + filepath = $TestConfigPath + } + _purge = $false + subsystem = @( + @{ + name = "sftp" # Update existing + value = $script:AlternatePath + }, + @{ + name = "newsub" # Add new + value = "/path/to/newsub" + } + ) + } | ConvertTo-Json -Depth 10 + + $output = sshdconfig set --input $inputConfig -s sshd-config-repeat-list 2>$null + $LASTEXITCODE | Should -Be 0 + + # Verify subsystems + $subsystems = Get-Content $TestConfigPath | Where-Object { $_ -match '^\s*subsystem\s+' } + + # Should have updated sftp, kept test2, and added newsub + $subsystems.Count | Should -Be 3 + + # Verify sftp was updated + $sftpLine = $subsystems | Where-Object { $_ -match 'sftp' } + $sftpLine | Should -Match ([regex]::Escape($script:AlternatePath)) + + # Verify new subsystem was added + $subsystems | Should -Contain "subsystem newsub /path/to/newsub" + + # Verify unlisted subsystems are preserved (_purge: false) + $subsystems | Should -Contain "Subsystem test2 /path/to/test2" + } + + It 'Should preserve unlisted subsystems when _purge is false' { + $inputConfig = @{ + _metadata = @{ + filepath = $TestConfigPath + } + _purge = $false + subsystem = @( + @{ + name = "onlythisone" + value = "/path/to/this" + } + ) + } | ConvertTo-Json -Depth 10 + + $output = sshdconfig set --input $inputConfig -s sshd-config-repeat-list 2>$null + $LASTEXITCODE | Should -Be 0 + + # Verify all existing subsystems are still present plus the new one using get + $getInput = @{ + _metadata = @{ + filepath = $TestConfigPath + } + } | ConvertTo-Json + $result = sshdconfig get --input $getInput -s sshd-config 2>$null | ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 + + $result.subsystem.Count | Should -Be 3 # 2 existing + 1 new + + # Verify new subsystem + $newEntry = $result.subsystem | Where-Object { $_.name -ceq 'onlythisone' } + $newEntry | Should -Not -BeNullOrEmpty + $newEntry.value | Should -Be '/path/to/this' + + # Verify all existing subsystems preserved + ($result.subsystem | Where-Object { $_.name -ceq 'sftp' }) | Should -Not -BeNullOrEmpty + ($result.subsystem | Where-Object { $_.name -ceq 'test2' }) | Should -Not -BeNullOrEmpty + } + + It 'Should remove unlisted subsystems when _purge is true' { + $inputConfig = @{ + _metadata = @{ + filepath = $TestConfigPath + } + _purge = $true + subsystem = @( + @{ + name = "sftp" + value = $script:AlternatePath + }, + @{ + name = "newsub" + value = "/path/to/newsub" + } + ) + } | ConvertTo-Json -Depth 10 + + $output = sshdconfig set --input $inputConfig -s sshd-config-repeat-list 2>$null + $LASTEXITCODE | Should -Be 0 + + # Verify only specified subsystems remain using get + $getInput = @{ + _metadata = @{ + filepath = $TestConfigPath + } + } | ConvertTo-Json + $result = sshdconfig get --input $getInput -s sshd-config 2>$null | ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 + + $result.subsystem.Count | Should -Be 2 + + # Verify specified subsystems are present + $sftpEntry = $result.subsystem | Where-Object { $_.name -ceq 'sftp' } + $sftpEntry | Should -Not -BeNullOrEmpty + $sftpEntry.value | Should -Be $script:AlternatePath + + $newsubEntry = $result.subsystem | Where-Object { $_.name -ceq 'newsub' } + $newsubEntry | Should -Not -BeNullOrEmpty + $newsubEntry.value | Should -Be '/path/to/newsub' + + # Verify unlisted subsystems were removed + ($result.subsystem | Where-Object { $_.name -ceq 'test2' }) | Should -BeNullOrEmpty + ($result.subsystem | Where-Object { $_.name -ceq 'internal-sftp' }) | Should -BeNullOrEmpty + } + + It 'Should preserve case in subsystem names' { + $inputConfig = @{ + _metadata = @{ + filepath = $TestConfigPath + } + _purge = $false + subsystem = @( + @{ + name = "MixedCase" + value = "/path/to/mixed" + }, + @{ + name = "UPPERCASE" + value = "/path/to/upper" + } + ) + } | ConvertTo-Json -Depth 10 + + $output = sshdconfig set --input $inputConfig -s sshd-config-repeat-list 2>$null + $LASTEXITCODE | Should -Be 0 + + # Verify exact case is preserved in file + $content = Get-Content $TestConfigPath -Raw + $content | Should -Match "subsystem MixedCase /path/to/mixed" + $content | Should -Match "subsystem UPPERCASE /path/to/upper" + } + + It 'Should handle paths with spaces in list' { + $inputConfig = @{ + _metadata = @{ + filepath = $TestConfigPath + } + _purge = $false + subsystem = @( + @{ + name = "spacepath1" + value = $script:PathWithSpaces + }, + @{ + name = "spacepath2" + value = "/another/path with spaces/binary" + } + ) + } | ConvertTo-Json -Depth 10 + + $output = sshdconfig set --input $inputConfig -s sshd-config-repeat-list 2>$null + $LASTEXITCODE | Should -Be 0 + + # Verify subsystems with spaces in paths are present + $subsystems = Get-Content $TestConfigPath | Where-Object { $_ -match '^\s*subsystem\s+' } + $spacePath1Line = $subsystems | Where-Object { $_ -match 'spacepath1' } + $spacePath2Line = $subsystems | Where-Object { $_ -match 'spacepath2' } + + $spacePath1Line | Should -Not -BeNullOrEmpty + $spacePath2Line | Should -Not -BeNullOrEmpty + $spacePath1Line | Should -Match ([regex]::Escape($script:PathWithSpaces)) + $spacePath2Line | Should -Match '/another/path with spaces/binary' + } + + It 'Should fail with missing required name property' { + $inputConfig = @{ + _metadata = @{ + filepath = $TestConfigPath + } + _purge = $false + subsystem = @( + @{ + value = "/path/to/something" + } + ) + } | ConvertTo-Json -Depth 10 + + $stderrFile = Join-Path $TestDrive "stderr_missing_property.txt" + sshdconfig set --input $inputConfig -s sshd-config-repeat-list 2>$stderrFile + $LASTEXITCODE | Should -Not -Be 0 + + Remove-Item -Path $stderrFile -Force -ErrorAction SilentlyContinue + } + + It 'Should handle empty subsystem array when _purge is true' { + $inputConfig = @{ + _metadata = @{ + filepath = $TestConfigPath + } + _purge = $true + subsystem = @() + } | ConvertTo-Json -Depth 10 + + $output = sshdconfig set --input $inputConfig -s sshd-config-repeat-list 2>$null + $LASTEXITCODE | Should -Be 0 + + # Verify all subsystems were removed + $subsystems = Get-Content $TestConfigPath | Where-Object { $_ -match '^\s*subsystem\s+' } + $subsystems.Count | Should -Be 0 + } + } +}