Jira (PUP-11061) Two Sensitive instances with the same value are not equal

10 views
Skip to first unread message

Joshua Partlow (Jira)

unread,
May 13, 2021, 5:31:04 PM5/13/21
to puppe...@googlegroups.com
Joshua Partlow created an issue
 
Puppet / Bug PUP-11061
Two Sensitive instances with the same value are not equal
Issue Type: Bug Bug
Affects Versions: PUP 7.6.0
Assignee: Unassigned
Components: Type System
Created: 2021/05/13 2:30 PM
Priority: Normal Normal
Reporter: Joshua Partlow

Puppet Version: 7.6.0

Two instances of lib/puppet/pops/types/p_sensitive_type.rb Sensitive with the same value are not equal. The class is just using default object id for equality.

Desired Behavior:

jpartlow@work1804:~/work/src/puppet$ puppet apply -e '$a = Sensitive("secret"); $b = Sensitive("secret"); notice($a == $b)'
Notice: Scope(Class[main]): true
Notice: Compiled catalog for work1804.platform9.puppet.net in environment production in 0.03 seconds
Notice: Applied catalog in 0.08 seconds

Actual Behavior:

jpartlow@work1804:~/work/src/puppet$ puppet apply -e '$a = Sensitive("secret"); $b = Sensitive("secret"); notice($a == $b)'
Notice: Scope(Class[main]): false
Notice: Compiled catalog for work1804.platform9.puppet.net in environment production in 0.03 seconds
Notice: Applied catalog in 0.08 seconds

Add Comment Add Comment
 
This message was sent by Atlassian Jira (v8.13.2#813002-sha1:c495a97)
Atlassian logo

Gabriel Nagy (Jira)

unread,
May 18, 2021, 9:11:01 AM5/18/21
to puppe...@googlegroups.com
Gabriel Nagy commented on Bug PUP-11061
 
Re: Two Sensitive instances with the same value are not equal

From a Slack convo with Henrik: https://puppetcommunity.slack.com/archives/C0W1X7ZAL/p1621341048001500

gabi Today at 3:30 PM
@helindbe or whoever may have more context: I was curious if https://tickets.puppetlabs.com/browse/PUP-11061 was an implementation oversight or intended behavior

helindbe 15 minutes ago
The only thing that shpuld be allowed is to know the base type of the value (e.g String), but not specifics of that steing, such as the lenght or characters in it. From that it follows that comparison should not be allowed. One reason is that equality check can fail and the error message may reveal the sensitive value. However, since unwrap is available, it isn’t really a hard security concern, but unwrap shifts the responsibility not to spill the secret.
Hope this explains why it works the way it does.

helindbe 11 minutes ago
So, I consider this intended behavior, but I don’t see a real problem if an equality check was to return true if sensitive values are equal and false for all other cases (including possible errors). Problem is, where do you stop, also support in selector and case and other comparisons… Probably best to keep it the way it is.

Joshua Partlow (Jira)

unread,
May 19, 2021, 6:31:04 PM5/19/21
to puppe...@googlegroups.com

I updated the pr to test hash, so unwrap isn't involved.

For context, the case I tripped over was using bolt_spec to verify a task with a sensitive parameter. The spec runner couldn't match the expectation because it was matching the parameter hashes, and the sensitive parameter values were unequal...

Beth Glenfield (Jira)

unread,
May 20, 2021, 5:25:03 AM5/20/21
to puppe...@googlegroups.com

My one concern here would be if a Sensitive type was compromised, either through brute force or another method, would we be increasing the attack area with this change. Could we maybe add some tests in the PR for when the values don't match to ensure we're not leaking any info?

Josh Cooper (Jira)

unread,
Jun 3, 2021, 2:38:01 PM6/3/21
to puppe...@googlegroups.com

Josh Cooper (Jira)

unread,
Jun 3, 2021, 2:38:03 PM6/3/21
to puppe...@googlegroups.com
Josh Cooper updated an issue
 
Change By: Josh Cooper
Fix Version/s: PUP 7.8.0
Fix Version/s: PUP 6.23.0

Joshua Partlow (Jira)

unread,
Jun 3, 2021, 5:16:03 PM6/3/21
to puppe...@googlegroups.com
Joshua Partlow updated an issue
Change By: Joshua Partlow
Release Notes: Enhancement
Release Notes Summary: Previously two instances of type Sensitive would not compare as equal if their underlying strings were equal. This patch changed that behavior allowing comparisons such as '$a = Sensitive("secret"); $b = Sensitive("secret"); notice($a == $b)' to return true.

Christine Yoon (Jira)

unread,
Jun 21, 2021, 3:59:03 PM6/21/21
to puppe...@googlegroups.com
Reply all
Reply to author
Forward
0 new messages