Jira (PUP-10164) Agent incorrectly reporting corrective changes

15 views
Skip to first unread message

Josh Cooper (JIRA)

unread,
Dec 6, 2019, 4:17:04 PM12/6/19
to puppe...@googlegroups.com
Josh Cooper moved an issue
 
Puppet / Bug PUP-10164
Agent incorrectly reporting corrective changes
Change By: Josh Cooper
Affects Version/s: puppet-agent 6.11.0
Component/s: Releases
Key: PA PUP - 3024 10164
Project: Puppet Agent
Add Comment Add Comment
 
This message was sent by Atlassian JIRA (v7.7.1#77002-sha1:e75ca93)
Atlassian logo

Josh Cooper (JIRA)

unread,
Dec 6, 2019, 4:19:05 PM12/6/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
*Puppet Version: 6.11.1*
*Puppet Server Version: 6*
*OS Name/Version: Centos* 

Describe your issue in as much detail as possible…
Describe steps to reproduce…

Reproducing this requires using a file from an https server as the source for a local file. In our case it is things that are secret like /etc/shadow that we don't want committed to our puppet repo. 

*Desired Behavior:*

I want the agent to calculate the md5sum correctly when pulling a file from a remote https server. 

*Actual Behavior:*

Agent reports changing content from an MD5 with 128 bits to an 'MD5' with 196 bits. It doesn't change the file as no changes are needed. The 'changed' file's md5sum is the same before and  after the agent runs.

It seems to be using a 196 bit hash in place of an md5sum and reporting it needs changing as the 128 bit and 196 bit values are not equal.


Please take a moment and attach any relevant log output and/or manifests. This will help us immensely when troubleshooting the issue.

Examples:
Run puppet agent with --test --trace --debug

Relevant sections of {{/var/log/puppetlabs/puppetserver/puppetserver.log}} or any applicable logs from the same directory.

For more detailed information turn up the server logs by upping the log level in the server's logback.xml

Relevant sections of configurations files (puppet.conf, hiera.conf, Server's conf.d, defaults/sysconfig)

For memory issues with server heap dumps are also helpful.

Josh Cooper (JIRA)

unread,
Dec 11, 2019, 4:46:05 PM12/11/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
*Puppet Version: 6.11.1*
*Puppet Server Version: 6*
*OS Name/Version: Centos* 

Reproducing this requires using a file from an https server as the source for a local file. In our case it is things that are secret like /etc/shadow that we don't want committed to our puppet repo. 

*Local Reproducing Steps:*

* run the command below and copy the md5 resulted

{code:bash}[root@unpaved-song ttt]# printf '%s' "File Data" | md5sum
0ee8336a3589503bcb03c7885950c909{code}

* create a .rb file and copy the following inside to have a server with a get response with "File Data" text and a Content-MD5 header.

 
{code:ruby}# server.rb
require 'socket'
server = TCPServer.new 5678
while session = server.accept
  request = session.gets
  puts request

     session.print "HTTP/1.1 200\r\n"
  session.print "Content-Type: text/html\r\n"
  session.print "Content-MD5: 0ee8336a3589503bcb03c7885950c909\r\n"
  session.print "\r\n"
  session.print "File Data"
  session.close
end
{code}

*  run ruby server.rb to start the server

 
* run puppet apply -e 'file \{"/file_from_server": source => "http://localhost:5678"}' to get the data form the local server

 
{code:java}[root@unpaved-song ttt]# puppet apply -e 'file {"/file_from_server": source => "http://localhost:5678"}'
Notice: Compiled catalog for unpaved-song.delivery.puppetlabs.net in environment production in 0.01 seconds
Notice: /Stage[main]/Main/File[/file_from_server]/ensure: defined content as '{md5}f5d7fb75fe1beb6f1bedaefa6bddf7df9e7df797b9ef4d9c'
Notice: Applied catalog in 0.07 seconds
{code}

* Running the command again you will get

{code:java}Notice: /Stage[main]/Main/File[/file_from_server]/content: content changed '{md5}0ee8336a3589503bcb03c7885950c909' to '{md5}f5d7fb75fe1beb6f1bedaefa6bddf7df9e7df797b9ef4d9c'
{code}

 

*Desired Behavior:*

I want the agent to calculate the md5sum correctly when pulling a file from a remote https server. 

*Actual Behavior:*

Agent reports changing content from an MD5 with 128 bits to an 'MD5' with 196 bits. It doesn't change the file as no changes are needed. The 'changed' file's md5sum is the same before and  after the agent runs.

It seems to be using a 196 bit hash in place of an md5sum and reporting it needs changing as the 128 bit and 196 bit values are not equal.

Josh Cooper (JIRA)

unread,
Dec 11, 2019, 7:36:04 PM12/11/19
to puppe...@googlegroups.com

Josh Cooper (JIRA)

unread,
Dec 11, 2019, 7:50:04 PM12/11/19
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-10164
 
Re: Agent incorrectly reporting corrective changes

Thanks Tyler Hess I was able to repro using:

bx puppet apply -e 'file {"/tmp/file_from_server": source => "http://localhost:5678/file"}'  --http_debug
Notice: Compiled catalog for whatcom.wifi.pdx.corp.puppet.net in environment production in 0.01 seconds
opening connection to localhost:5678...
opened
<- "HEAD /file HTTP/1.1\r\nAccept: */*\r\nUser-Agent: Puppet/5.5.18 Ruby/2.5.3-p105 (x86_64-darwin18)\r\nAccept-Encoding: gzip;q=1.0,deflate;q=0.6,identity;q=0.3\r\nConnection: close\r\nHost: localhost:5678\r\n\r\n"
-> "HTTP/1.1 200\r\n"
-> "Content-Type: text/html\r\n"
-> "Content-MD5: 0ee8336a3589503bcb03c7885950c909\r\n"
-> "\r\n"
Conn close
opening connection to localhost:5678...
opened
<- "HEAD /file HTTP/1.1\r\nAccept: */*\r\nUser-Agent: Puppet/5.5.18 Ruby/2.5.3-p105 (x86_64-darwin18)\r\nAccept-Encoding: gzip;q=1.0,deflate;q=0.6,identity;q=0.3\r\nConnection: close\r\nHost: localhost:5678\r\n\r\n"
-> "HTTP/1.1 200\r\n"
-> "Content-Type: text/html\r\n"
-> "Content-MD5: 0ee8336a3589503bcb03c7885950c909\r\n"
-> "\r\n"
Conn close
opening connection to localhost:5678...
opened
<- "GET /file HTTP/1.1\r\nAccept: */*\r\nUser-Agent: Puppet/5.5.18 Ruby/2.5.3-p105 (x86_64-darwin18)\r\nAccept-Encoding: gzip;q=1.0,deflate;q=0.6,identity;q=0.3\r\nConnection: close\r\nHost: localhost:5678\r\n\r\n"
-> "HTTP/1.1 200\r\n"
-> "Content-Type: text/html\r\n"
-> "Content-MD5: 0ee8336a3589503bcb03c7885950c909\r\n"
-> "\r\n"
reading all...
-> "File Data"
read 9 bytes
Conn close
Notice: /Stage[main]/Main/File[/tmp/file_from_server]/ensure: defined content as '{md5}d1e7bcdf7e9adf9f3de74ddb71bd3773bf3ce7de7473dd3d'
Notice: Applied catalog in 0.04 seconds

So the problem is the Content-MD5 response header is supposed to be base64 encoded not hex. For example https://tools.ietf.org/html/rfc1864#section-2 gives the example:

         Content-MD5:  Q2hlY2sgSW50ZWdyaXR5IQ==

And the apache ContentDigest directive is similar: https://httpd.apache.org/docs/2.4/mod/core.html#contentdigest

The reason we end up with 196 bit hash, is puppet attempts to convert what it thinks is a base64 value to hex: https://github.com/puppetlabs/puppet/blob/e36a82e8f620a3e846a97c18d3d0bb6076812fce/lib/puppet/file_serving/http_metadata.rb#L21.

Josh Cooper (JIRA)

unread,
Dec 11, 2019, 11:56:04 PM12/11/19
to puppe...@googlegroups.com

Josh Cooper (JIRA)

unread,
Dec 12, 2019, 12:07:04 AM12/12/19
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-10164
 
Re: Agent incorrectly reporting corrective changes

The following can be used to generate the base64 encoded md5 value:

$ echo -n 'File Data' | openssl md5 -binary | base64 -
DugzajWJUDvLA8eIWVDJCQ==

When using that in server.rb, puppet correctly detects that the file is insync:

$ bx puppet apply -e 'file {"/tmp/file_from_server": source => "http://localhost:5678/file"}'  --http_debug
Notice: Compiled catalog for ... in environment production in 0.02 seconds
opening connection to localhost:5678...
opened
<- "HEAD /file HTTP/1.1\r\nAccept: */*\r\nUser-Agent: Puppet/6.12.0 Ruby/2.3.8-p459 (x86_64-darwin18)\r\nAccept-Encoding: gzip;q=1.0,deflate;q=0.6,identity;q=0.3\r\nHost: localhost:5678\r\nConnection: close\r\n\r\n"
-> "HTTP/1.1 200\r\n"
-> "Content-Type: text/html\r\n"
-> "Content-MD5: DugzajWJUDvLA8eIWVDJCQ==\r\n"
-> "\r\n"
Conn close
Notice: Applied catalog in 0.04 seconds

So I think we can either close this or update documentation to make it clear the Content-MD5 header needs to be the base64 encoded value.

Tyler Hess (JIRA)

unread,
Dec 12, 2019, 11:36:04 AM12/12/19
to puppe...@googlegroups.com
Tyler Hess commented on Bug PUP-10164

Josh Cooper Would it be possible to accept the md5sum both ways by only unpacking if the length of response is < 32 characters?

Josh Cooper (JIRA)

unread,
Dec 16, 2019, 3:04:04 PM12/16/19
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-10164

Ah, I see what's going on. Ruby has 2 methods for base64 decoding. One is strict (follows RFC 4648) and other is not strict (follows RFC 2045). Puppet calls unpack("m0") which calls the former, while unpack("m") would call the latter.

The = character is used to pad the base64 encoded value, see https://en.wikipedia.org/wiki/Base64#Output_padding. If the padding is omitted (which your server must be doing), then non-strict decode will succeed, but strict decode will fail:

irb(main):034:0> Base64.strict_decode64("DugzajWJUDvLA8eIWVDJCQ==")
=> "\x0E\xE83j5\x89P;\xCB\x03\xC7\x88YP\xC9\t"
irb(main):035:0> Base64.strict_decode64("DugzajWJUDvLA8eIWVDJCQ")
Traceback (most recent call last):
        4: from /usr/local/opt/rbenv/versions/2.5.3/bin/irb:11:in `<main>'
        3: from (irb):35
        2: from /usr/local/Cellar/rbenv/1.1.2/versions/2.5.3/lib/ruby/2.5.0/base64.rb:74:in `strict_decode64'
        1: from /usr/local/Cellar/rbenv/1.1.2/versions/2.5.3/lib/ruby/2.5.0/base64.rb:74:in `unpack1'
ArgumentError (invalid base64)

So we could change puppet to accept non-strict decode (RFC 2045). Or we could add 0-2 characters of padding, and then strictly decode as we do now.

I'm inclined to do the latter, as it seems consistent with https://tools.ietf.org/html/rfc4648#section-3.2, since in this case, we know the size exactly (24 bytes)

In some circumstances, the use of padding ("=") in base-encoded data
is not required or used. In the general case, when assumptions about
the size of transported data cannot be made, padding is required to
yield correct decoded data.

Reply all
Reply to author
Forward
0 new messages