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

User provider password_max_age attribute is flawed under Solaris

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Normal
    • Resolution: Fixed
    • Affects Version/s: PUP 2.7.23
    • Fix Version/s: PUP 3.7.0
    • Component/s: Types and Providers
    • Labels:
    • Environment:

      PE 2.8.2 under Solaris 10 1/13

    • Template:
    • Story Points:
      1
    • Sprint:
      2014-08-20

      Description

      A user reported this, and did an excellent write up of the issue and a possible fix, so I'm just going to paste that:

      The password_max_age logic is flawed on Solaris. At first, it seems to work:

      # tail -1 /etc/passwd
      foobar:x:911:911::/home/foobar:/bin/sh
       
      # tail -1 /etc/shadow
      foobar:GI2XGzW0DitJk:15931::182:28:::
      

      Excerpt from Puppet manifest:

      user { 'foobar':
        uid => 911,
        gid => 911,
        password_max_age => -1,  <<<<<<<<<<
        ensure => present,
      }
      

      # /opt/puppet/bin/puppet agent --test
      ...
      notice: /Stage[main]/Site::Common/User[foobar]/password_max_age: password_max_age changed '182' to '-1'
       
      root@its-au-psnpdevap4:/export/home/e05593 # tail -1 /etc/shadow
      foobar:GI2XGzW0DitJk:15931::::::
      

      So far so good ... password expiration was removed properly and subsequent puppet runs are quiet. The problem occurs after a failed login attempt:

      root@its-au-psnpdevap4:/export/home/e05593 # tail -1 /etc/shadow
      foobar:GI2XGzW0DitJk:15931::::::1
      

      Notice the last field is now 1 indicating 1 failed login. Puppet will now wrongly claim that password expiration is enabled ... and all subsequent Puppet runs attempt to remove it:

      # /opt/puppet/bin/puppet agent --test
      ...
      notice: /Stage[main]/Site::Common/User[foobar]/password_max_age: password_max_age changed '' to '-1'
      

      The logic error is in user_role_add.rb. The following patch seems to fix the problem:

      --- /opt/puppet/lib/ruby/site_ruby/1.8/puppet/provider/user/user_role_add.rb   2013-06-08 07:47:48.000000000 +1000
      +++ user_role_add.rb    2013-08-14 16:55:46.347435848 +1000
      @@ -174,7 +174,7 @@
       
         def password_max_age
           return :absent unless shadow_entry
      -    shadow_entry[4] || -1
      +    (shadow_entry[4].nil? || shadow_entry[4].empty?) ? -1 : shadow_entry[4]
         end
       
         # Read in /etc/shadow, find the line for our used and rewrite it with the
      

      Personally, I find it confusing to have nil array elements, because shadow should always have 9 fields. I would probably have populated @shadow_entry as follows:

      --- /opt/puppet/lib/ruby/site_ruby/1.8/puppet/provider/user/user_role_add.rb   2013-06-08 07:47:48.000000000 +1000
      +++ user_role_add.rb    2013-08-14 17:01:14.141392541 +1000
      @@ -160,7 +160,7 @@
           return @shadow_entry if defined? @shadow_entry
           @shadow_entry = File.readlines(target_file_path).
             reject { |r| r =~ /^[^\w]/ }.
      -      collect { |l| l.chomp.split(':') }.
      +      collect { |l| l.chomp.split(':', -1) }.
             find { |user, _| user == @resource[:name] }
         end
      

      Note that the original logic error would have been obvious if this approach had been used. It would also render .nil? checks unnecessary ... assuming an array length sanity check after the file is read.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                Unassigned
                Reporter:
                kfj Ken Johnson
                QA Contact:
                Narmadha Perumal
              • Votes:
                2 Vote for this issue
                Watchers:
                9 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Zendesk Support