+1

I've know there is a small (but possible) window for awful messes with
the current implementation, nice to see you fixed it :)

cheers,
/Martin

On Apr 8, 4:06 pm, Daniel Pittman <[email protected]> wrote:
> The user_role_add provider for user management previously open-coded the safe,
> atomic replacement of /etc/shadow after in modified it.
>
> We replace that with the standard, central replace_file API, which ensures
> that is done in a safe, correct, and standard fashion.
>
> On the way through this removes the window where this could previously have
> lost the content of /etc/shadow due to an unfortunately timed crash.
> ---
>  lib/puppet/provider/user/user_role_add.rb |   19 ++++++++++---------
>  1 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/lib/puppet/provider/user/user_role_add.rb 
> b/lib/puppet/provider/user/user_role_add.rb
> index aa01f8e..b4728db 100644
> --- a/lib/puppet/provider/user/user_role_add.rb
> +++ b/lib/puppet/provider/user/user_role_add.rb
> @@ -1,3 +1,4 @@
> +require 'puppet/util'
>  require 'puppet/util/user_attr'
>
>  Puppet::Type.type(:user).provide :user_role_add, :parent => :useradd, 
> :source => :useradd do
> @@ -165,28 +166,28 @@ Puppet::Type.type(:user).provide :user_role_add, 
> :parent => :useradd, :source =>
>          pass
>      end
>
> -    #Read in /etc/shadow, find the line for our used and rewrite it with the 
> new pw
> -    #Smooth like 80 grit
> +    # Read in /etc/shadow, find the line for our used and rewrite it with 
> the new pw
> +    # Smooth like 80 grit
> +    #
> +    # We use the replace_file atomic file API to ensure this is done safely,
> +    # since corrupting /etc/shadow would make some *very* unhappy people.
>      def password=(cryptopw)
>          begin
>              File.open("/etc/shadow", "r") do |shadow|
> -                File.open("/etc/shadow_tmp", "w", 0600) do |shadow_tmp|
> +                Puppet::Util.replace_file("/etc/shadow", 0600) do
> +                    |replacement|
>                      while line = shadow.gets do
>                          line_arr = line.split(':')
>                          if line_arr[0] == @resource[:name]
>                              line_arr[1] = cryptopw
>                              line = line_arr.join(':')
>                          end
> -                        shadow_tmp.print line
> +                        replacement.print line
>                      end
>                  end
>              end
> -            File.rename("/etc/shadow_tmp", "/etc/shadow")
>          rescue => detail
> -            fail "Could not write temporary shadow file: %s" % detail
> -        ensure
> -            # Make sure this *always* gets deleted
> -            File.unlink("/etc/shadow_tmp") if File.exist?("/etc/shadow_tmp")
> +            fail "Could not replace /etc/shadow: %s" % detail
>          end
>      end
>  end
> --
> 1.7.0.4

-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en.

Reply via email to