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
      

        Attachments

          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:

                  Zendesk Support