Skip to content

Need a common way to declare a property is sensitive #71

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

Closed
SteveL-MSFT opened this issue Apr 25, 2023 · 10 comments
Closed

Need a common way to declare a property is sensitive #71

SteveL-MSFT opened this issue Apr 25, 2023 · 10 comments
Labels
Issue-Enhancement The issue is a feature or idea Resolution-Fixed The issue is fixed

Comments

@SteveL-MSFT
Copy link
Member

Summary of the new feature / enhancement

There may be a need to pass sensitive information (like an api key) to a resource that may be retrieved from AKV, for example. DSC itself needs to know this is sensitive so it doesn't log it or emit to console and resources also need to know it's sensitive for the same reasons.

It could be as simple as a common prefix: _sensitive so resources can themselves decide if it's _sensitiveString, _sensitiveKey, etc...

Proposed technical implementation details (optional)

No response

@SteveL-MSFT SteveL-MSFT added the Issue-Enhancement The issue is a feature or idea label Apr 25, 2023
@Bpoe
Copy link
Collaborator

Bpoe commented May 8, 2023

For a configuration, you could specify the parameter type like ARM templates do:
https://learn.microsoft.com/en-US/azure/azure-resource-manager/templates/data-types#secure-strings-and-objects

I'm hesitant to try to set a flag for the resource to know, since its up to the resource to handle it correctly. It might give users a false sense that the value is somehow protected by DSC, but we have no control over what the resource does with it.

@Bpoe
Copy link
Collaborator

Bpoe commented May 17, 2023

We could pass the sensitive value in via environment variables named "SENSITIVE_PARAMETERNAME". The json payload would need to reference this variable.

@michaeltlombardi
Copy link
Collaborator

I think we do need a way to declare in a manifest whether the resource (and especially the arbitrary caller) can/should treat a value as sensitive for redaction. It's not a guarantee of redaction - that always depends on the implementation of the resource/caller - but without it there's just no way for a caller to know whether or not to apply special handling to a value.

@mgreenegit
Copy link
Member

Agree with @michaeltlombardi on a way to make it obvious to the resource author. I also could see an integration point with machine config where we refer to the name and path of a secret in KeyVault via the SecretManagement module.

@SteveL-MSFT
Copy link
Member Author

Note that I expect actual secrets to be passed as parameters to the configuration. For PS users, they can certainly use SecretManagement module to do this, but we may need equivalent created for non-PS scenarios.

@SteveL-MSFT
Copy link
Member Author

It may make sense to have a special identifier for getting secrets by name (using ARM-like syntax):

resources:
- name: myTest
   type: Microsoft.Azure/VM
   properties:
     name: myVM
     user: foo
     password: [secret('fooPassword')]

In this case, we'd extend ARM to have a secret function that is tied to something like SecretManagement (or equivalent for non-PS users).

@Bpoe
Copy link
Collaborator

Bpoe commented Jul 5, 2023

I want to make sure that we keep consistency with ARM as a top priority. Here is some "prior art" to think about regarding passing sensitive settings to VM extensions:

https://learn.microsoft.com/en-us/azure/templates/microsoft.compute/virtualmachines/extensions?pivots=deployment-language-arm-template

@michaeltlombardi
Copy link
Collaborator

There's at least three different but related requirements here:

  1. A DSC Resource must have a way to indicate that a property is sensitive - it might be a password, a token, or some other secret. The DSC Resource must be able to report this information to consuming tools through its manifest or JSON Schema - without this information, calling tools and users have no reliable way of distinguishing between sensitive and non-sensitive properties without relying on fragile pattern matching for property names.

    The simplest implementation of this is to either introduce an annotation keyword like sensitive to the JSON Schema, or to add a new manifest section for indicating which properties are sensitive, if we don't want to extend the standard dialect. Both are feasible1.

  2. A DSC Configuration author must have a way to retrieve and specify sensitive values to DSC Resources without committing secrets into the configuration document.

    In this case, I think the ARM-isms of using a parameter or key vault reference are sensible.

  3. A DSC Configuration author should have a way to indicate that a property or set of properties should be treated as sensitive, even if the DSC Resource isn't implemented with that sensitivity. This would redact the values from any logging or reporting. I think this is broadly useful, but lower priority than 1 and 2.

Footnotes

  1. Defining a new keyword normally means extending the JSON Schema standard dialect with a new vocabulary. Fortunately, JSON Schema validation ignores unrecognized keywords so we don't have to publish a vocabulary or dialect, especially in the short term. The value can still be used by our own tools and it's easy enough for me to document.

@michaeltlombardi
Copy link
Collaborator

After consideration, I think the simplest way forward for this is to define and advise the use of the custom keyword x-sensitive. The JSON Schema maintainers wrote a blog post recommending the use of the x- prefix for custom annotation keywords. As this functionality is annotative - it informs the users and tools interacting with the manifest how to handle the data, not how to validate it - this seems like the lowest effort solution.

The updated schema for the schema.embedded.properties keyword in the manifest would then move from:

title: Instance Properties
description: >-
  Defines the properties that DSC can retrieve and manage for the resource's instances.
  This keyword must define at least one property as a key-value pair. The key is the
  property's name. The value is a subschema that validates the property.
type: object
minProperties: 1
unevaluatedProperties:
  anyOf:
    - $ref: https://json-schema.org/draft/2020-12/schema
    - $ref: https://json-schema.org/draft/2019-09/schema
    - $ref: http://json-schema.org/draft-07/schema#

To:

title: Instance Properties
description: >-
  Defines the properties that DSC can retrieve and manage for the resource's instances.
  This keyword must define at least one property as a key-value pair. The key is the
  property's name. The value is a subschema that validates the property.
type: object
minProperties: 1
unevaluatedProperties:
  anyOf:
    - $ref: https://json-schema.org/draft/2020-12/schema
    - $ref: https://json-schema.org/draft/2019-09/schema
    - $ref: http://json-schema.org/draft-07/schema#
  properties:
    x-sensitive:
      type:    boolean
      default: false
      title:   Value is sensitive
      description: >-
        Defines whether the property value should be handled as sensitive data. If the value is
        sensitive, like an API token, DSC and higher order tools should redact the value in any
        logs or messages. By default, property values aren't considered sensitive.

The Markdown documentation for this property would be:

Defines whether the property value should be handled as sensitive data. If the value is sensitive, like an API token, DSC and higher order tools should redact the value in any logs or messages.

This keyword identifies to users and tools that the value is inherently sensitive. The resource itself should also redact the value in any logs or messages. Users can still use the sensitive data type for a configuration parameter to force DSC to redact the data, even when it's not used for a sensitive property value.

By default, property values aren't considered sensitive. To indicate that a property value is sensitive, set this keyword to true.

In the following example, the foo property is sensitive but the bar and baz properties aren't.

{
    // Rest of the resource manifest...
    "schema": {
        "embedded": {
            "$schema": "http://json-schema.org/draft-07/schema#",
            "title": "SensitiveExample",
            "type": "object",
            "required": ["foo"],
            "additionalProperties": false,
            "properties": {
                "foo": {
                    "type": "string",
                    "x-sensitive": true
                },
                "bar": {
                    "type": "string",
                    "x-sensitive": false
                },
                "baz": { "type": "integer" }
            }
        }
    }
}

This same pattern could be used for other cases where we need to introduce annotations for DSC Resource properties, like whether a property requires elevation/root.

@SteveL-MSFT
Copy link
Member Author

With PR #364 it describes a way to pass sensitive strings and objects.

@SteveL-MSFT SteveL-MSFT added Resolution-Fixed The issue is fixed and removed Need-Review labels Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Enhancement The issue is a feature or idea Resolution-Fixed The issue is fixed
Projects
None yet
Development

No branches or pull requests

4 participants