Uploaded image for project: 'Puppet'
  1. Puppet
  2. PUP-1255

Default file mode is now 0600 instead of 0644

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Normal
    • Resolution: Done
    • Affects Version/s: PUP 2.7.24, PUP 3.4.1
    • Fix Version/s: PUP 2.7.25, PUP 3.4.2
    • Component/s: Types and Providers
    • Labels:
      None
    • Environment:

      puppet-3.4.1-1.el6.noarch

    • Template:
    • Story Points:
      1
    • Sprint:
      Week 2013-12-19 to 2014-1-2, Week 2014-1-2 to 2014-1-8

      Description

      As of Puppet 3.4.1 and 2.7.24, the default file mode when no mode is explicitly specified on a file resource has changed from 0644 to 0600. It appears from the commits (https://github.com/puppetlabs/puppet/commit/65909fbe and 691fbbeaa) that this was not the intention.

      # rpm -q puppet
      puppet-3.4.0-1.el6.noarch
      # puppet apply -e 'file { "/tmp/a": content => "foo" }'
      Notice: Compiled catalog for foreman.example.com in environment production in 0.04 seconds
      Notice: /Stage[main]/Main/File[/tmp/a]/ensure: defined content as '{md5}acbd18db4cc2f85cedef654fccc4a4d8'
      Notice: Finished catalog run in 0.07 seconds
      # ll /tmp/a
      -rw-r--r-- 1 root root 3 Dec 27 12:01 /tmp/a
       
      # rpm -q puppet
      puppet-3.4.1-1.el6.noarch
      # puppet apply -e 'file { "/tmp/a": content => "foo" }'
      Notice: Compiled catalog for foreman.example.com in environment production in 0.04 seconds
      Notice: /Stage[main]/Main/File[/tmp/a]/ensure: defined content as '{md5}acbd18db4cc2f85cedef654fccc4a4d8'
      Notice: Finished catalog run in 0.08 seconds
      # ll /tmp/a
      -rw------- 1 root root 3 Dec 27 11:58 /tmp/a
      

        Issue Links

          Activity

          Hide
          andy Andrew Parker added a comment -

          Kylo Ginsberg, Joshua Partlow, and I investigated this. It is happening because in the case when a mode it not specified the mode on the resulting file is now determined by the mode applied to the tempfile, which is moved into place. Since tempfiles receive fairly restrictive permissions we end up with 0600 on the resulting file.

          Show
          andy Andrew Parker added a comment - Kylo Ginsberg , Joshua Partlow , and I investigated this. It is happening because in the case when a mode it not specified the mode on the resulting file is now determined by the mode applied to the tempfile, which is moved into place. Since tempfiles receive fairly restrictive permissions we end up with 0600 on the resulting file.
          Hide
          andy Andrew Parker added a comment -

          Merged into stable in 6cabaa

          Show
          andy Andrew Parker added a comment - Merged into stable in 6cabaa
          Hide
          domcleal Dominic Cleal added a comment -

          Merged into 2.7.x in 6a11ab

          Show
          domcleal Dominic Cleal added a comment - Merged into 2.7.x in 6a11ab
          Hide
          josh Josh Cooper added a comment -

          Failing on windows 2012, investigating

          Show
          josh Josh Cooper added a comment - Failing on windows 2012, investigating
          Hide
          andy Andrew Parker added a comment -

          The Win2012 failure is

                  expected:   NT AUTHORITY\SYSTEM:(I)                       0x1f01ff
            BUILTIN\Administrators:(I)                    0x1f01ff
            BURI\Administrator:(I)                        0x1f01ff
           
               got:   NT AUTHORITY\SYSTEM:                          0x1f01ff
            BUILTIN\Administrators:                       0x1f01ff
            BURI\Administrator:                           0x1f01ff
            NT AUTHORITY\SYSTEM:(I)                       0x1f01ff
            BUILTIN\Administrators:(I)                    0x1f01ff
            BURI\Administrator:(I)                        0x1f01ff
           (using ==)
          Diff:
          @@ -1,3 +1,6 @@
          +  NT AUTHORITY\SYSTEM:                          0x1f01ff
          +  BUILTIN\Administrators:                       0x1f01ff
          +  BURI\Administrator:                           0x1f01ff
             NT AUTHORITY\SYSTEM:(I)                       0x1f01ff
             BUILTIN\Administrators:(I)                    0x1f01ff
             BURI\Administrator:(I)                        0x1f01ff
           
          ./spec/integration/util_spec.rb:73:in `block (3 levels) in <top (required)>'
          

          Show
          andy Andrew Parker added a comment - The Win2012 failure is expected: NT AUTHORITY\SYSTEM:(I) 0x1f01ff BUILTIN\Administrators:(I) 0x1f01ff BURI\Administrator:(I) 0x1f01ff   got: NT AUTHORITY\SYSTEM: 0x1f01ff BUILTIN\Administrators: 0x1f01ff BURI\Administrator: 0x1f01ff NT AUTHORITY\SYSTEM:(I) 0x1f01ff BUILTIN\Administrators:(I) 0x1f01ff BURI\Administrator:(I) 0x1f01ff (using ==) Diff: @@ -1,3 +1,6 @@ + NT AUTHORITY\SYSTEM: 0x1f01ff + BUILTIN\Administrators: 0x1f01ff + BURI\Administrator: 0x1f01ff NT AUTHORITY\SYSTEM:(I) 0x1f01ff BUILTIN\Administrators:(I) 0x1f01ff BURI\Administrator:(I) 0x1f01ff   ./spec/integration/util_spec.rb:73:in `block (3 levels) in <top (required)>'
          Hide
          josh Josh Cooper added a comment - - edited

          The issue is being triggered because for some reason, on 2012, files created in the temp directory (C:\Users\username\AppData\Local\Temp*) have pseudo-inherited ACLs. Using setacl, you can see:

          PS C:\users\josh\AppData\local> setacl -on .\temp\newdir -ot file -actn list  -lst "f:tab;w:d,s,o,g;i:y"
          .\temp\newdir 
             Owner: WIN-85MJMOPCCE6\josh
             Group: WIN-85MJMOPCCE6\None
             DACL(not_protected):
             NT AUTHORITY\SYSTEM    full   allow   pseudo_inherited+container_inherit+object_inherit
             BUILTIN\Administrators full   allow   pseudo_inherited+container_inherit+object_inherit
             WIN-85MJMOPCCE6\josh   full   allow   pseudo_inherited+container_inherit+object_inherit
           
             SACL:
             [NULL]
          

          Later when the Windows ReplaceFile API is called, it merges the source (an empty temp file) and destination ACLs, instead of just preserving the source ACLs. This has security implications and needs to be understood.

          Separately, the test should be made more resilient by protecting the playground directory so that we can be be sure what permissions to expect on the newly created file.

          Show
          josh Josh Cooper added a comment - - edited The issue is being triggered because for some reason, on 2012, files created in the temp directory (C:\Users\username\AppData\Local\Temp*) have pseudo-inherited ACLs . Using setacl , you can see: PS C:\users\josh\AppData\local> setacl -on .\temp\newdir -ot file -actn list -lst "f:tab;w:d,s,o,g;i:y" .\temp\newdir Owner: WIN-85MJMOPCCE6\josh Group: WIN-85MJMOPCCE6\None DACL(not_protected): NT AUTHORITY\SYSTEM full allow pseudo_inherited+container_inherit+object_inherit BUILTIN\Administrators full allow pseudo_inherited+container_inherit+object_inherit WIN-85MJMOPCCE6\josh full allow pseudo_inherited+container_inherit+object_inherit SACL: [NULL] Later when the Windows ReplaceFile API is called, it merges the source (an empty temp file) and destination ACLs, instead of just preserving the source ACLs. This has security implications and needs to be understood. Separately, the test should be made more resilient by protecting the playground directory so that we can be be sure what permissions to expect on the newly created file.
          Hide
          josh Josh Cooper added a comment -

          Forgot to mention that I reset the inherited permissions using {icacls local /reset /t and then the problem went away, and the test passes as expected.

          Show
          josh Josh Cooper added a comment - Forgot to mention that I reset the inherited permissions using { icacls local /reset /t and then the problem went away, and the test passes as expected.
          Hide
          rob Rob Reynolds added a comment - - edited

          Last merge (https://github.com/puppetlabs/puppet/pull/2211) merged into stable at a4af858 and master at d42c1e6.

          Show
          rob Rob Reynolds added a comment - - edited Last merge ( https://github.com/puppetlabs/puppet/pull/2211 ) merged into stable at a4af858 and master at d42c1e6 .
          Hide
          josh Josh Cooper added a comment - - edited

          merged to pe-puppet in a564fda

          Show
          josh Josh Cooper added a comment - - edited merged to pe-puppet in a564fda
          Hide
          kurt.wall Kurt Wall added a comment -

          Verified in 3.4.1.428:

          # dpkg -l puppet | grep -i puppet
          ii  puppet                              3.4.1.428-1puppetlabs1       Centralized configuration management - agent startup and compatibility scripts
          # puppet apply -e 'file { "/tmp/a": content => "foo" }'
          Notice: Compiled catalog for debian6-master.localdomain in environment production in 0.05 seconds
          Notice: /Stage[main]/Main/File[/tmp/a]/ensure: defined content as '{md5}acbd18db4cc2f85cedef654fccc4a4d8'
          Notice: Finished catalog run in 0.02 seconds
          root@debian6-master:~# ls -l /tmp/a
          -rw-r--r-- 1 root root 3 Jan  6 11:05 /tmp/a
          

          Show
          kurt.wall Kurt Wall added a comment - Verified in 3.4.1.428: # dpkg -l puppet | grep -i puppet ii puppet 3.4.1.428-1puppetlabs1 Centralized configuration management - agent startup and compatibility scripts # puppet apply -e 'file { "/tmp/a": content => "foo" }' Notice: Compiled catalog for debian6-master.localdomain in environment production in 0.05 seconds Notice: /Stage[main]/Main/File[/tmp/a]/ensure: defined content as '{md5}acbd18db4cc2f85cedef654fccc4a4d8' Notice: Finished catalog run in 0.02 seconds root@debian6-master:~# ls -l /tmp/a -rw-r--r-- 1 root root 3 Jan 6 11:05 /tmp/a
          Hide
          melissa Melissa Stone added a comment -

          Released in Puppet 3.4.2

          Show
          melissa Melissa Stone added a comment - Released in Puppet 3.4.2

            People

            • Assignee:
              Unassigned
              Reporter:
              domcleal Dominic Cleal
            • Votes:
              2 Vote for this issue
              Watchers:
              13 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Agile