Jira (PUP-9295) Notify resource exposes Sensitive data when message is a Sensitive data

39 views
Skip to first unread message

Henrik Lindberg (JIRA)

unread,
Oct 31, 2018, 11:21:33 AM10/31/18
to puppe...@googlegroups.com
Henrik Lindberg updated an issue
 
Puppet / Bug PUP-9295
Notify resource exposes Sensitive data when message is a Sensitive data
Change By: Henrik Lindberg
Summary: Notify resource exposing exposes Sensitive data when message is raw a Sensitive data
Add Comment Add Comment
 
This message was sent by Atlassian JIRA (v7.7.1#77002-sha1:e75ca93)
Atlassian logo

Henrik Lindberg (JIRA)

unread,
Oct 31, 2018, 11:22:51 AM10/31/18
to puppe...@googlegroups.com
Henrik Lindberg updated an issue
Change By: Henrik Lindberg
Fix Version/s: PUP 5.5.9

Josh Cooper (JIRA)

unread,
Nov 1, 2018, 2:17:07 PM11/1/18
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Sprint: Coremunity Grooming

Josh Cooper (JIRA)

unread,
Jan 7, 2019, 4:39:04 PM1/7/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Fix Version/s: PUP 5.5.9
Fix Version/s: PUP 6.0.z
Fix Version/s: PUP 5.5.z

Josh Cooper (JIRA)

unread,
Jan 14, 2019, 6:07:03 PM1/14/19
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-9295
 
Re: Notify resource exposes Sensitive data when message is a Sensitive data

The Puppet::Pops::Types::PSensitiveType#to_s method is only called in the third case, I assume as a result of interpolating the sensitive message in a string.

One fix is to do the following, but it feels like a game of "whack a mole" as every type/provider needs to check if it's been given a sensitive property/parameter:

diff --git a/lib/puppet/type/notify.rb b/lib/puppet/type/notify.rb
 
index 96830aa91b..aa7cb5603b 100644
 
--- a/lib/puppet/type/notify.rb
 
+++ b/lib/puppet/type/notify.rb
 
@@ -11,11 +11,12 @@ module Puppet
 
     newproperty(:message, :idempotent => false) do
 
       desc "The message to be sent to the log."
 
       def sync
 
+        message = @sensitive ? 'Sensitive [value redacted]' : self.should
 
         case @resource["withpath"]
 
         when :true
 
-          send(@resource[:loglevel], self.should)
 
+          send(@resource[:loglevel], message)
 
         else
 
-          Puppet.send(@resource[:loglevel], self.should)
 
+          Puppet.send(@resource[:loglevel], message)
 
         end
 
         return
 
       end


$ bundle exec puppet apply manifest.pp
 
Notice: Compiled catalog for localhost in environment production in 0.02 seconds
 
Notice: Sensitive [value redacted]
 
Notice: /Stage[main]/Main/Notify[Sensitive message 1]/message: changed [redacted] to [redacted]
 
Notice: Sensitive [value redacted]
 
Notice: /Stage[main]/Main/Notify[Sensitive message 2]/message: changed [redacted] to [redacted]
 
Notice: Sensitive [value redacted]
 
Notice: /Stage[main]/Main/Notify[Sensitive message 3]/message: defined 'message' as 'Sensitive [value redacted]'
 
Notice: Applied catalog in 0.01 seconds

Henrik Lindberg (JIRA)

unread,
Jan 31, 2019, 7:44:04 AM1/31/19
to puppe...@googlegroups.com

I don't see how we can get around this for types/providers that do their own logging/output where values should be appear as redacted without actually making them recognize if they should redact. This because they are written to operate on a single clear text value.

Josh Cooper (JIRA)

unread,
Mar 1, 2019, 12:29:04 PM3/1/19
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-9295

But Henrik Lindberg why does interpolating the secret value correctly redact:

$ bx puppet apply -e "\$secret = Sensitive.new('foo'); notify { 'n': message => \"\${secret}\"}"
 
Notice: Compiled catalog for localhost in environment production in 0.03 seconds
 
Notice: Sensitive [value redacted]

But not when handled the secret value directly:

$ bx puppet apply -e "\$secret = Sensitive.new('foo'); notify { 'n': message => \$secret}"
 
Notice: Compiled catalog for localhost in environment production in 0.03 seconds
 
Notice: foo

Surely providers shouldn't be expected to handle those two cases differently?

Henrik Lindberg (JIRA)

unread,
Mar 1, 2019, 12:42:03 PM3/1/19
to puppe...@googlegroups.com

Because when the sensitive value is interpolated that takes place when the catalog is compiled and a Sensitive value turns it into a string by generating "redacted". When doing this the agent gets the string "redacted" not a sensitive value - and this is probably not what the user intended, so there really is no "other case" for a provider to deal with.

The support for sensitive on the agent side is partly in the harness - reporting values etc. checks if param is marked as sensitive. All types providers that do something else with values must be aware that a value may be sensitive and handle it appropriately. This is naturally not great.

The alternative is not great either: we could instead of setting the sensitive flag in the resource have the values actually be of type Sensitive. Then, any type provider getting this value would need to know how to unwrap it - and thus also be aware that it is handling a sensitive value. That has other issues naturally - and this was why the design with a separate sensitive flag was introduced. (If we had done this, it would be more secure, but would be a lot more painful).

Jorie Tappa (JIRA)

unread,
Jun 12, 2019, 4:29:03 PM6/12/19
to puppe...@googlegroups.com
Jorie Tappa updated an issue
 
Change By: Jorie Tappa
Component/s: Docs

Jorie Tappa (JIRA)

unread,
Jun 12, 2019, 4:30:03 PM6/12/19
to puppe...@googlegroups.com
Jorie Tappa commented on Bug PUP-9295
 
Re: Notify resource exposes Sensitive data when message is a Sensitive data

If it's an intended behavior, we should be explicit about it in the docs. If it's not, we can prioritize this for future Sensitive improvements. If it's an intended behavior that we now want to change, we need approval/priority from Products or a PM.

Kris Bosland (JIRA)

unread,
Jul 15, 2019, 6:32:03 PM7/15/19
to puppe...@googlegroups.com
Kris Bosland commented on Bug PUP-9295

The team is coming to the view that the least surprise for users is if only code specifically designed to handle Sensitive values can get the unredacted version.  It should not show up in even Debug logs by default, because that requires our users to manage their debug logs and reports as Sensitive data.  This is a larger scale engineering effort, and we will work towards tickets and epics to outline this work.

Jorie Tappa (JIRA)

unread,
Jul 22, 2019, 12:49:03 PM7/22/19
to puppe...@googlegroups.com
Jorie Tappa commented on Bug PUP-9295

Regardless of original intentions, we do not want to reveal any sensitive data in the notify resource. Josh Cooper's comment above should resolve this for the notify resource specifically, but there might be other issues we haven't come across yet. 

 

As Henrik mentioned above, we could've implemented another flag to force a reveal but we haven't, so redacting the data should always be the expected behavior, and the safest/least surprise for the user.

Jorie Tappa (JIRA)

unread,
Jul 22, 2019, 12:53:05 PM7/22/19
to puppe...@googlegroups.com
Jorie Tappa assigned an issue to Jorie Tappa
 
Change By: Jorie Tappa
Assignee: Jorie Tappa

Josh Cooper (JIRA)

unread,
Sep 16, 2019, 2:45:04 PM9/16/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Fix Version/s: PUP 6.0.z
Fix Version/s: PUP 6.4.z

Rob Braden (JIRA)

unread,
Sep 17, 2019, 2:27:04 PM9/17/19
to puppe...@googlegroups.com
Rob Braden assigned an issue to Unassigned
Change By: Rob Braden
Assignee: Jorie Tappa

Josh Cooper (JIRA)

unread,
Sep 17, 2019, 11:27:04 PM9/17/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Sprint: Coremunity Grooming Hopper

Josh Cooper (JIRA)

unread,
Oct 2, 2019, 12:51:04 AM10/2/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Sprint: Coremunity Hopper Platform Core KANBAN

Josh Cooper (JIRA)

unread,
Oct 2, 2019, 12:51:04 AM10/2/19
to puppe...@googlegroups.com
Josh Cooper assigned an issue to Josh Cooper
Change By: Josh Cooper
Assignee: Josh Cooper

Josh Cooper (JIRA)

unread,
Oct 3, 2019, 1:04:04 PM10/3/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Release Notes Summary: Redact sensitive values when interpolated in a notify resource's message.
Release Notes: Bug Fix

Josh Cooper (JIRA)

unread,
Oct 4, 2019, 6:02:04 PM10/4/19
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-9295
 
Re: Notify resource exposes Sensitive data when message is a Sensitive data

There are two issues going on. This ticket is about the notify resource printing messages 1 and 2 to the console, which ends up logs. The second issue is that the compiler evaluates these differently, which is surprising UX:

$secret = Sensitive('secret')
 
notify { 'a': message => $secret }
 
notify { 'b': message => "${secret}" }

The resulting catalog contains:

    {
 
      "type": "Notify",
 
      "title": "a",
 
      "tags": [
 
        "notify",
 
        "a",
 
        "class"
 
      ],
 
      "file": "/etc/puppetlabs/code/environments/production/manifests/site.pp",
 
      "line": 5,
 
      "exported": false,
 
      "parameters": {
 
        "message": "secret"
 
      },
 
      "sensitive_parameters": [
 
        "message"
 
      ]
 
    },
 
    {
 
      "type": "Notify",
 
      "title": "b",
 
      "tags": [
 
        "notify",
 
        "b",
 
        "class"
 
      ],
 
      "file": "/etc/puppetlabs/code/environments/production/manifests/site.pp",
 
      "line": 6,
 
      "exported": false,
 
      "parameters": {
 
        "message": "Sensitive [value redacted]"
 
      }
 
    }

The compiler preserves the sensitive value for resource "a" and marks it as sensitive, while resource "b" is coverted to the string "Sensitive [value redacted]", which is lossy.

Josh Cooper (JIRA)

unread,
Oct 4, 2019, 6:22:05 PM10/4/19
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-9295

Josh Cooper (JIRA)

unread,
Oct 4, 2019, 7:37:04 PM10/4/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Fix Version/s: PUP 6.4.z
Fix Version/s: PUP 5.5.z
Fix Version/s: PUP 6.4.4
Fix Version/s: PUP 5.5.18

Josh Cooper (JIRA)

unread,
Oct 4, 2019, 7:37:04 PM10/4/19
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-9295
 
Re: Notify resource exposes Sensitive data when message is a Sensitive data

This did not make 5.5.17, but should make 6.4.4 and 6.10.1

Josh Cooper (JIRA)

unread,
Oct 4, 2019, 7:37:05 PM10/4/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Fix Version/s: PUP 6.10.1

Josh Cooper (JIRA)

unread,
Oct 7, 2019, 7:20:03 PM10/7/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Fix Version/s: PUP 6.4.4
Fix Version/s: PUP 6.4.5

Kris Bosland (JIRA)

unread,
Oct 7, 2019, 10:18:03 PM10/7/19
to puppe...@googlegroups.com

George Mrejea (JIRA)

unread,
Oct 9, 2019, 7:59:04 AM10/9/19
to puppe...@googlegroups.com
George Mrejea commented on Bug PUP-9295

I believe this can be closed since the commit was promoted and tested in master.

Josh Cooper (JIRA)

unread,
Oct 9, 2019, 11:25:05 AM10/9/19
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-9295

Passed CI in 36c35404b8474b5949c87c49e0b360d426b64cbb

Jean Bond (JIRA)

unread,
Oct 10, 2019, 11:46:04 AM10/10/19
to puppe...@googlegroups.com
Jean Bond updated an issue
 
Change By: Jean Bond
Release Notes Summary:
Redact sensitive values when interpolated in a notify resource's message.


note added for 6.10.1-jb

Ciprian Badescu (JIRA)

unread,
Nov 14, 2019, 10:08:07 AM11/14/19
to puppe...@googlegroups.com
Ciprian Badescu updated an issue
Change By: Ciprian Badescu
Fix Version/s: PUP 6.11.0

Heston Hoffman (JIRA)

unread,
Nov 18, 2019, 12:58:05 PM11/18/19
to puppe...@googlegroups.com
Heston Hoffman updated an issue
Change By: Heston Hoffman
Labels: resolved-issue-added

Josh Cooper (JIRA)

unread,
Nov 19, 2019, 12:17:05 PM11/19/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Fix Version/s: PUP 6.11.0

Josh Cooper (JIRA)

unread,
Nov 19, 2019, 12:17:05 PM11/19/19
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-9295
 
Re: Notify resource exposes Sensitive data when message is a Sensitive data

Ciprian Badescu, Heston Hoffman This was fixed in 6.10.1, it doesn't need to be "fixed" again in 6.11.0
Reply all
Reply to author
Forward
0 new messages