Merge pull request #1307 from tgauth/sshdconfig-fixes

Expand sshdconfig set capability

Author: SteveL-MSFT

dsc/tests/dsc_sshdconfig.tests.ps1

@@ -133,4 +133,44 @@ resources:
             }
         }
     }
+
+    Context 'Set Commands' {
+        It 'Set works with _clobber: true' {
+            $set_yaml = @"
+`$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json
+metadata:
+  Microsoft.DSC:
+    securityContext: elevated
+resources:
+- name: sshdconfig
+  type: Microsoft.OpenSSH.SSHD/sshd_config
+  metadata:
+    filepath: $filepath
+  properties:
+    _clobber: true
+    port: 1234
+    allowUsers:
+      - user1
+      - user2
+    passwordAuthentication: $false
+    ciphers:
+      - aes128-ctr
+      - aes192-ctr
+      - aes256-ctr
+    addressFamily: inet6
+    authorizedKeysFile:
+      - ./.ssh/authorized_keys
+      - ./.ssh/authorized_keys2
+"@
+            $out = dsc config set -i "$set_yaml" | ConvertFrom-Json -Depth 10
+            $LASTEXITCODE | Should -Be 0
+            $out.results.count | Should -Be 1
+            $out.results.result.afterState.port | Should -Be 1234
+            $out.results.result.afterState.passwordauthentication | Should -Be $false
+            $out.results.result.afterState.ciphers | Should -Be @('aes128-ctr', 'aes192-ctr', 'aes256-ctr')
+            $out.results.result.afterState.allowusers | Should -Be @('user1', 'user2')
+            $out.results.result.afterState.addressfamily | Should -Be 'inet6'
+            $out.results.result.afterState.authorizedkeysfile | Should -Be @('./.ssh/authorized_keys', './.ssh/authorized_keys2')
+        }
+    }
 }

resources/sshdconfig/locales/en-us.toml

@@ -30,6 +30,7 @@ windowsOnly = "Microsoft.OpenSSH.SSHD/Windows is only applicable to Windows"
 
 [main]
 export = "Export command: %{input}"
+invalidTraceLevel = "Invalid trace level"
 schema = "Schema command:"
 set = "Set command: '%{input}'"
 
@@ -67,13 +68,15 @@ shellPathMustNotBeRelative = "shell path must not be relative"
 sshdConfigReadFailed = "failed to read existing sshd_config file at path: '%{path}'"
 tempFileCreated = "temporary file created at: %{path}"
 validatingTempConfig = "Validating temporary sshd_config file"
-valueMustBeString = "value for key '%{key}' must be a string"
 writingTempConfig = "Writing temporary sshd_config file"
 
 [util]
 cleanupFailed = "Failed to clean up temporary file %{path}: %{error}"
+deserializeFailed = "Failed to deserialize match input: %{error}"
+getIgnoresInputFilters = "get command does not support filtering based on input settings, provided input will be ignored"
 inputMustBeBoolean = "value of '%{input}' must be true or false"
-inputMustBeEmpty = "get command does not support filtering based on input settings"
+invalidValue = "Key: '%{key}' cannot have empty value"
+matchBlockMissingCriteria = "Match block must contain 'criteria' field"
 sshdConfigNotFound = "sshd_config not found at path: '%{path}'"
 sshdConfigReadFailed = "failed to read sshd_config at path: '%{path}'"
 sshdElevation = "elevated security context required"

resources/sshdconfig/src/args.rs

@@ -6,10 +6,30 @@ use rust_i18n::t;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 
+#[derive(Debug, Clone, PartialEq, Eq, ValueEnum)]
+pub enum TraceFormat {
+    Default,
+    Plaintext,
+    Json,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq, ValueEnum)]
+pub enum TraceLevel {
+    Error,
+    Warn,
+    Info,
+    Debug,
+    Trace
+}
+
 #[derive(Parser)]
 pub struct Args {
     #[clap(subcommand)]
     pub command: Command,
+    #[clap(short = 'l', long, help = "Trace level to use", value_enum)]
+    pub trace_level: Option<TraceLevel>,
+    #[clap(short = 'f', long, help = "Trace format to use", value_enum, default_value = "json")]
+    pub trace_format: TraceFormat,
 }
 
 #[derive(Subcommand)]

resources/sshdconfig/src/main.rs

@@ -29,10 +29,10 @@ const EXIT_SUCCESS: i32 = 0;
 const EXIT_FAILURE: i32 = 1;
 
 fn main() {
-    enable_tracing();
-
     let args = Args::parse();
 
+    enable_tracing(args.trace_level.as_ref(), &args.trace_format);
+
     let result = match &args.command {
         Command::Export { input } => {
             debug!("{}: {:?}", t!("main.export").to_string(), input);

resources/sshdconfig/src/metadata.rs

@@ -1,34 +1,39 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-// note that it is possible for a keyword to be in one, neither, or both of the multi-arg and repeatable lists below.
+// the multi-arg comma-separated and space-separated lists are mutually exclusive, but the repeatable list can overlap with either of them.
+// the multi-arg lists are maintained for formatting arrays into the correct format when writing back to the config file.
 
 // keywords that can have multiple comma-separated arguments per line and should be represented as arrays.
-pub const MULTI_ARG_KEYWORDS: [&str; 22] = [
-    "acceptenv",
-    "allowgroups",
-    "allowusers",
+pub const MULTI_ARG_KEYWORDS_COMMA_SEP: [&str; 11] = [
     "authenticationmethods",
-    "authorizedkeysfile",
     "casignaturealgorithms",
-    "channeltimeout",
     "ciphers",
-    "denygroups",
-    "denyusers",
     "hostbasedacceptedalgorithms",
     "hostkeyalgorithms",
-    "ipqos",
     "kexalgorithms",
     "macs",
-    "permitlisten",
-    "permitopen",
     "permituserenvironment",
-    "persourcepenalties",
     "persourcepenaltyexemptlist",
     "pubkeyacceptedalgorithms",
     "rekeylimit" // first arg is bytes, second arg (optional) is amount of time
 ];
 
+// keywords that can have multiple space-separated arguments per line and should be represented as arrays.
+pub const MULTI_ARG_KEYWORDS_SPACE_SEP: [&str; 11] = [
+    "acceptenv",
+    "allowgroups",
+    "allowusers",
+    "authorizedkeysfile",
+    "channeltimeout",
+    "denygroups",
+    "denyusers",
+    "ipqos",
+    "permitlisten",
+    "permitopen",
+    "persourcepenalties",
+];
+
 // keywords that can be repeated over multiple lines and should be represented as arrays.
 pub const REPEATABLE_KEYWORDS: [&str; 12] = [
     "acceptenv",
@@ -45,7 +50,6 @@ pub const REPEATABLE_KEYWORDS: [&str; 12] = [
     "subsystem"
 ];
 
-
 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";

resources/sshdconfig/src/parser.rs

@@ -8,7 +8,7 @@ use tracing::debug;
 use tree_sitter::Parser;
 
 use crate::error::SshdConfigError;
-use crate::metadata::{MULTI_ARG_KEYWORDS, REPEATABLE_KEYWORDS};
+use crate::metadata::{MULTI_ARG_KEYWORDS_COMMA_SEP, MULTI_ARG_KEYWORDS_SPACE_SEP, REPEATABLE_KEYWORDS};
 
 #[derive(Debug, JsonSchema)]
 pub struct SshdConfigParser {
@@ -147,9 +147,9 @@ impl SshdConfigParser {
             let Ok(text) = keyword.utf8_text(input_bytes) else {
                 return Err(SshdConfigError::ParserError(t!("parser.failedToParseNode", input = input).to_string()));
             };
-
-            is_repeatable = REPEATABLE_KEYWORDS.contains(&text);
-            is_vec = is_repeatable || MULTI_ARG_KEYWORDS.contains(&text);
+            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());
             key = Some(text.to_string());
         }
 
@@ -467,12 +467,6 @@ match user testuser
         let result: Map<String, Value> = 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();
-        for (k, v) in match_obj.iter() {
-            eprintln!("  {}: {:?}", k, v);
-        }
-
-        // allowgroups is both MULTI_ARG and REPEATABLE
-        // Space-separated values should be parsed as array
         let allowgroups = match_obj.get("allowgroups").unwrap().as_array().unwrap();
         assert_eq!(allowgroups.len(), 2);
         assert_eq!(allowgroups[0], Value::String("administrators".to_string()));

resources/sshdconfig/src/set.rs

@@ -16,8 +16,8 @@ use tracing::{debug, info, warn};
 use crate::args::{DefaultShell, Setting};
 use crate::error::SshdConfigError;
 use crate::inputs::{CommandInfo, 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};
+use crate::metadata::{REPEATABLE_KEYWORDS, SSHD_CONFIG_HEADER, SSHD_CONFIG_HEADER_VERSION, SSHD_CONFIG_HEADER_WARNING};
+use crate::util::{build_command_info, format_sshd_value, get_default_sshd_config_path, invoke_sshd_config_validation};
 
 /// Invoke the set command.
 ///
@@ -114,10 +114,24 @@ fn set_sshd_config(cmd_info: &CommandInfo) -> Result<(), SshdConfigError> {
     let mut config_text = SSHD_CONFIG_HEADER.to_string() + "\n" + SSHD_CONFIG_HEADER_VERSION + "\n" + SSHD_CONFIG_HEADER_WARNING + "\n";
     if cmd_info.clobber {
         for (key, value) in &cmd_info.input {
-            if let Some(value_str) = value.as_str() {
-                writeln!(&mut config_text, "{key} {value_str}")?;
+            let key_lower = key.to_lowercase();
+
+            // Handle repeatable keywords - write multiple lines
+            if REPEATABLE_KEYWORDS.contains(&key_lower.as_str()) {
+                if let Value::Array(arr) = value {
+                    for item in arr {
+                        let formatted = format_sshd_value(key, item)?;
+                        writeln!(&mut config_text, "{key} {formatted}")?;
+                    }
+                } else {
+                    // Single value for repeatable keyword, write as-is
+                    let formatted = format_sshd_value(key, value)?;
+                    writeln!(&mut config_text, "{key} {formatted}")?;
+                }
             } else {
-                return Err(SshdConfigError::InvalidInput(t!("set.valueMustBeString", key = key).to_string()));
+                // Handle non-repeatable keywords - format and write single line
+                let formatted = format_sshd_value(key, value)?;
+                writeln!(&mut config_text, "{key} {formatted}")?;
             }
         }
     } else {