Skip to content

Add support for secret() function and secret extensions #908

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion dsc/src/subcommand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -632,9 +632,10 @@ fn list_extensions(dsc: &mut DscManager, extension_name: Option<&String>, format
let mut include_separator = false;
for manifest_resource in dsc.list_available(&DiscoveryKind::Extension, extension_name.unwrap_or(&String::from("*")), "", progress_format) {
if let ImportedManifest::Extension(extension) = manifest_resource {
let mut capabilities = "-".to_string();
let mut capabilities = "--".to_string();
let capability_types = [
(ExtensionCapability::Discover, "d"),
(ExtensionCapability::Secret, "s"),
];

for (i, (capability, letter)) in capability_types.iter().enumerate() {
Expand Down
119 changes: 119 additions & 0 deletions dsc/tests/dsc_functions_secret.tests.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.

Describe 'Tests for the secret() function and extensions' {
BeforeAll {
$oldPath = $env:PATH
$toolPath = Resolve-Path -Path "$PSScriptRoot/../../extensions/test/secret"
$env:PATH = "$toolPath" + [System.IO.Path]::PathSeparator + $oldPath
}

AfterAll {
$env:PATH = $oldPath
}

It 'Just a secret name' {
$configYaml = @'
$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json
resources:
- name: Echo
type: Microsoft.DSC.Debug/Echo
properties:
output: "[secret('MySecret')]"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not the point of this PR, but should we prevent leaking secrets in this way? Accidentally leaking a secure string (and the output of the secret function is always secure, right?) to the output seems dangerous.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

secureString internally is just a string (as it's a JSON type), the only control DSC has is that itself it won't write it to console or traces.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm specifically wondering whether we want to add an issue to prevent things like this - afaik there's only two ways to get a secret value into a configuration document:

  1. As a parameter.
  2. With the secret() function.

In both cases, DSC is aware that the string value is a secret and shouldn't be emitted (consider the case where the desired state for a resource requires a credential object) - should we mask the value even in the actually-emitted JSON output?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, internally we could keep it as a JSON object with a single secureString (or secureObject) property, but resources that receive it could unwrap it and DSC wouldn't know. I think it's best effort at best.

'@
$out = dsc -l trace config get -i $configYaml 2> $TestDrive/error.log | ConvertFrom-Json
$LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw -Path $TestDrive/error.log)
$out.results.Count | Should -Be 1
$out.results[0].result.actualState.Output | Should -BeExactly 'Hello'
}

It 'Name and vault' {
$configYaml = @'
$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json
resources:
- name: Echo
type: Microsoft.DSC.Debug/Echo
properties:
output: "[secret('DifferentSecret', 'VaultA')]"
'@
$out = dsc -l trace config get -i $configYaml 2> $TestDrive/error.log | ConvertFrom-Json
$LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw -Path $TestDrive/error.log)
$out.results.Count | Should -Be 1
$out.results[0].result.actualState.Output | Should -BeExactly 'Hello2'
}

It 'Name that does not exist' {
$configYaml = @'
$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json
resources:
- name: Echo
type: Microsoft.DSC.Debug/Echo
properties:
output: "[secret('NonExistentSecret')]"
'@
dsc -l trace config get -i $configYaml 2> $TestDrive/error.log | ConvertFrom-Json
$LASTEXITCODE | Should -Be 2
$errorMessage = Get-Content -Raw -Path $TestDrive/error.log
$errorMessage | Should -Match "Secret 'NonExistentSecret' not found"
}

It 'Vault that does not exist' {
$configYaml = @'
$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json
resources:
- name: Echo
type: Microsoft.DSC.Debug/Echo
properties:
output: "[secret('MySecret', 'NonExistentVault')]"
'@
dsc -l trace config get -i $configYaml 2> $TestDrive/error.log | ConvertFrom-Json
$LASTEXITCODE | Should -Be 2
$errorMessage = Get-Content -Raw -Path $TestDrive/error.log
$errorMessage | Should -Match "Secret 'MySecret' not found"
}

It 'Duplicate secret' {
$configYaml = @'
$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json
resources:
- name: Echo
type: Microsoft.DSC.Debug/Echo
properties:
output: "[secret('DuplicateSecret')]"
'@
dsc -l trace config get -i $configYaml 2> $TestDrive/error.log | ConvertFrom-Json
$LASTEXITCODE | Should -Be 2
$errorMessage = Get-Content -Raw -Path $TestDrive/error.log
$errorMessage | Should -Match "Multiple secrets with the same name 'DuplicateSecret' and different values was returned, try specifying a vault"
}

It 'Secret and vault to disambiguate' {
$configYaml = @'
$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json
resources:
- name: Echo
type: Microsoft.DSC.Debug/Echo
properties:
output: "[secret('DuplicateSecret', 'Vault1')]"
'@
$out = dsc -l trace config get -i $configYaml 2> $TestDrive/error.log | ConvertFrom-Json
$LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw -Path $TestDrive/error.log)
$out.results.Count | Should -Be 1
$out.results[0].result.actualState.Output | Should -BeExactly 'World'
}

It 'Same secret name and value in different extensions' {
$configYaml = @'
$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json
resources:
- name: Echo
type: Microsoft.DSC.Debug/Echo
properties:
output: "[secret('DuplicateSame')]"
'@
$out = dsc -l trace config get -i $configYaml 2> $TestDrive/error.log | ConvertFrom-Json
$LASTEXITCODE | Should -Be 0
$out.results.Count | Should -Be 1
$out.results[0].result.actualState.Output | Should -BeExactly 'SameSecret'
}
}
13 changes: 12 additions & 1 deletion dsc_lib/locales/en-us.toml
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,11 @@ resourceManifestSchemaDescription = "Defines the JSON Schema the resource manife
discoverNoResults = "No results returned for discovery extension '%{extension}'"
discoverNotAbsolutePath = "Resource path from extension '%{extension}' is not an absolute path: %{path}"
extensionReturned = "Extension '%{extension}' returned line: %{line}"
retrievingSecretFromExtension = "Retrieving secret '%{name}' from extension '%{extension}'"
secretExtensionReturnedInvalidJson = "Extension '%{extension}' returned invalid JSON: %{error}"
extensionReturnedSecret = "Extension '%{extension}' returned secret"
extensionReturnedNoSecret = "Extension '%{extension}' did not return a secret"
secretNoResults = "Extension '%{extension}' returned no output"

[extensions.extension_manifest]
extensionManifestSchemaTitle = "Extension manifest schema URI"
Expand Down Expand Up @@ -269,6 +274,13 @@ invalidFirstArgType = "Invalid argument type for first parameter"
incorrectNameFormat = "Name argument cannot contain a slash"
invalidSecondArgType = "Invalid argument type for second parameter"

[functions.secret]
notString = "Parameter secret name is not a string"
multipleSecrets = "Multiple secrets with the same name '%{name}' and different values was returned, try specifying a vault"
extensionReturnedError = "Extension '%{extension}': %{error}"
noExtensions = "No extensions supporting secrets was found"
secretNotFound = "Secret '%{name}' not found"

[functions.sub]
invoked = "sub function"

Expand Down Expand Up @@ -329,7 +341,6 @@ manifestDescription = "manifest description"
commandOperation = "Command: Operation"
forExecutable = "for executable"
function = "Function"
error = "error"
integerConversion = "Function integer argument conversion"
invalidConfiguration = "Invalid configuration"
unsupportedManifestVersion = "Unsupported manifest version"
Expand Down
4 changes: 3 additions & 1 deletion dsc_lib/src/configure/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Licensed under the MIT License.

use chrono::{DateTime, Local};
use crate::configure::config_doc::ExecutionKind;
use crate::{configure::config_doc::ExecutionKind, extensions::dscextension::DscExtension};
use security_context_lib::{get_security_context, SecurityContext};
use serde_json::{Map, Value};
use std::{collections::HashMap, path::PathBuf};
Expand All @@ -11,6 +11,7 @@ use super::config_doc::{DataType, SecurityContextKind};

pub struct Context {
pub execution_type: ExecutionKind,
pub extensions: Vec<DscExtension>,
pub references: Map<String, Value>,
pub system_root: PathBuf,
pub parameters: HashMap<String, (Value, DataType)>,
Expand All @@ -24,6 +25,7 @@ impl Context {
pub fn new() -> Self {
Self {
execution_type: ExecutionKind::Actual,
extensions: Vec::new(),
references: Map::new(),
system_root: get_default_os_system_root(),
parameters: HashMap::new(),
Expand Down
6 changes: 5 additions & 1 deletion dsc_lib/src/configure/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,11 +233,14 @@ impl Configurator {
json: json.to_owned(),
config: Configuration::new(),
context: Context::new(),
discovery,
discovery: discovery.clone(),
statement_parser: Statement::new()?,
progress_format,
};
config.validate_config()?;
for extension in discovery.extensions.values() {
config.context.extensions.push(extension.clone());
}
Ok(config)
}

Expand Down Expand Up @@ -641,6 +644,7 @@ impl Configurator {
let config = serde_json::from_str::<Configuration>(self.json.as_str())?;
self.set_parameters(parameters_input, &config)?;
self.set_variables(&config)?;
self.context.extensions = self.discovery.extensions.values().cloned().collect();
Ok(())
}

Expand Down
17 changes: 17 additions & 0 deletions dsc_lib/src/discovery/command_discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ pub enum ImportedManifest {
Extension(DscExtension),
}

#[derive(Clone)]
pub struct CommandDiscovery {
// use BTreeMap so that the results are sorted by the typename, the Vec is sorted by version
adapters: BTreeMap<String, Vec<DscResource>>,
Expand Down Expand Up @@ -79,6 +80,11 @@ impl CommandDiscovery {
}
}

#[must_use]
pub fn get_extensions(&self) -> &BTreeMap<String, DscExtension> {
&self.extensions
}

fn get_resource_path_setting() -> Result<ResourcePathSetting, DscError>
{
if let Ok(v) = get_setting("resourcePath") {
Expand Down Expand Up @@ -546,6 +552,13 @@ impl ResourceDiscovery for CommandDiscovery {
}
Ok(found_resources)
}

fn get_extensions(&mut self) -> Result<BTreeMap<String, DscExtension>, DscError> {
if self.extensions.is_empty() {
self.discover(&DiscoveryKind::Extension, "*")?;
}
Ok(self.extensions.clone())
}
}

// TODO: This should be a BTreeMap of the resource name and a BTreeMap of the version and DscResource, this keeps it version sorted more efficiently
Expand Down Expand Up @@ -709,6 +722,10 @@ fn load_extension_manifest(path: &Path, manifest: &ExtensionManifest) -> Result<
verify_executable(&manifest.r#type, "discover", &discover.executable);
capabilities.push(dscextension::Capability::Discover);
}
if let Some(secret) = &manifest.secret {
verify_executable(&manifest.r#type, "secret", &secret.executable);
capabilities.push(dscextension::Capability::Secret);
}

let extension = DscExtension {
type_name: manifest.r#type.clone(),
Expand Down
13 changes: 12 additions & 1 deletion dsc_lib/src/discovery/discovery_trait.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

use crate::{dscerror::DscError, dscresources::dscresource::DscResource};
use crate::{dscerror::DscError, extensions::dscextension::DscExtension, dscresources::dscresource::DscResource};
use std::collections::BTreeMap;

use super::command_discovery::ImportedManifest;
Expand Down Expand Up @@ -77,4 +77,15 @@ pub trait ResourceDiscovery {
///
/// This function will return an error if the underlying discovery fails.
fn find_resources(&mut self, required_resource_types: &[String]) -> Result<BTreeMap<String, DscResource>, DscError>;

/// Get the available extensions.
///
/// # Returns
///
/// A result containing a map of extension names to their corresponding `DscExtension` instances.
///
/// # Errors
///
/// This function will return an error if the underlying discovery fails.
fn get_extensions(&mut self) -> Result<BTreeMap<String, DscExtension>, DscError>;
}
8 changes: 6 additions & 2 deletions dsc_lib/src/discovery/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::discovery::discovery_trait::{DiscoveryKind, ResourceDiscovery};
use crate::extensions::dscextension::DscExtension;
use crate::{dscresources::dscresource::DscResource, dscerror::DscError, progress::ProgressFormat};
use std::collections::BTreeMap;
use command_discovery::ImportedManifest;
use command_discovery::{CommandDiscovery, ImportedManifest};
use tracing::error;

#[derive(Clone)]
Expand Down Expand Up @@ -80,8 +80,9 @@ impl Discovery {
///
/// * `required_resource_types` - The required resource types.
pub fn find_resources(&mut self, required_resource_types: &[String], progress_format: ProgressFormat) {
let command_discovery = CommandDiscovery::new(progress_format);
let discovery_types: Vec<Box<dyn ResourceDiscovery>> = vec![
Box::new(command_discovery::CommandDiscovery::new(progress_format)),
Box::new(command_discovery),
];
let mut remaining_required_resource_types = required_resource_types.to_owned();
for mut discovery_type in discovery_types {
Expand All @@ -98,6 +99,9 @@ impl Discovery {
self.resources.insert(resource.0.clone(), resource.1);
remaining_required_resource_types.retain(|x| *x != resource.0);
};
if let Ok(extensions) = discovery_type.get_extensions() {
self.extensions.extend(extensions);
}
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions dsc_lib/src/dscerror.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ pub enum DscError {
#[error("{0}")]
Extension(String),

#[error("{t} '{0}' {t2}: {1}", t = t!("dscerror.function"), t2 = t!("dscerror.error"))]
#[error("{t} '{0}': {1}", t = t!("dscerror.function"))]
Function(String, String),

#[error("{t} '{0}' {t2}: {1}", t = t!("dscerror.function"), t2 = t!("dscerror.error"))]
#[error("{t} '{0}': {1}", t = t!("dscerror.function"))]
FunctionArg(String, String),

#[error("{t}: {0}", t = t!("dscerror.integerConversion"))]
Expand Down
Loading
Loading