[MODULES-7606] ssh_authorized_keys updating user => ... fails with Permission denied on previous user's .ssh/authorized_keys Created: 2014/09/11  Updated: 2020/05/14

Status: Needs Information
Project: Modules
Component/s: sshkeys_core
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Normal
Reporter: Tero Marttila Assignee: Branan Riley
Resolution: Unresolved Votes: 7
Labels: permissions, ssh, ssh_authorized_key, type_and_provider
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

Ubuntu 14.04
puppetmaster-passenger 3.6.2-1puppetlabs1
puppet agent 3.4.3-1


Issue Links:
Duplicate
is duplicated by PUP-4401 ssh_key purge on a authorized_key fil... Closed
Relates
relates to MODULES-7599 ssh_authorized_key key update will fa... Accepted
Template:
Team: Platform OS
QA Contact: Eric Thompson

 Description   

Changing the user for an ssh_authorized_key resource:

    ssh_authorized_key { "admin::sshkey:$name":
        ensure => present,
        key    => $sshkey,
        type   => $type,
        user   => $user,
    }

Leads to a Permission Denied error writing to the ~/.ssh/authorized_keys file of the previous user:

Notice: /Stage[main]/Admins/Admin[foo]/Admin::Sshkey[...]/Ssh_authorized_key[admin::sshkey:...]/user: user changed 'bar' to 'foo'
Notice: /Stage[main]/Admins/Admin[foo]/Admin::Sshkey[...]/Ssh_authorized_key[admin::sshkey:...]/target: target changed '/home/bar/.ssh/authorized_keys' to '/home/foo/.ssh/authorized_keys'
Error: Puppet::Util::FileType::FileTypeFlat could not write /home/bar/.ssh/authorized_keys: Permission denied - /home/bar/.ssh/authorized_keys
Error: /Stage[main]/Admins/Admin[foo]/Admin::Sshkey[...]/Ssh_authorized_key[admin::sshkey:...]: Could not evaluate: Puppet::Util::FileType::FileTypeFlat could not write /home/bar/.ssh/authorized_keys: Permission denied - /home/bar/.ssh/authorized_keys

The key for the new user is not written to ~foo/.ssh/authorized_keys, nor is the key removed from the old user's ~bar/.ssh/authorized_keys.



 Comments   
Comment by Thomas Kishel [ 2017/04/03 ]

This issue is the result of this PR:

https://github.com/puppetlabs/puppet/commit/b29b1785d543a3cea961fffa9b3c15f14ab7cce0

"Drop privileges before creating and chmodding SSH keys. Previously, potentially abusable chown and chmod calls were performed as root. This tries to moves as much as possible into code which is run after privileges have been dropped. Huge thanks to Ricky Zhou <ricky@fedoraproject.org> for discovering this and supplying the security fix. Awesome work. Fixes CVE-2011-3870."

The error occurs within the `super` call here:

https://github.com/puppetlabs/puppet/blame/master/lib/puppet/provider/ssh_authorized_key/parsed.rb#L65

To reproduce:

node test {
  user { 'test':
    ensure => present,
    gid => 'users',
    managehome => true,
    password => '!!',
    home => "/home/test",
    forcelocal => true,
    purge_ssh_keys => true,
  }
 
  ssh_authorized_key { 'test@home':
    user => 'test',
    type => 'ssh-rsa',
    key => 'ABC123',
  }
}

puppet agent -t
chown root:root /home/test/.ssh/authorized_keys
puppet agent -t

Results in:

Error: Puppet::Util::FileType::FileTypeFlat could not write /home/test/.ssh/authorized_keys: Permission denied @ rb_sysopen - /home/test/.ssh/authorized_keys

In one user's case, the authorized_keys file was created but kept its root:root ownership due to another (permissions on /tmp) error:

Error: Puppet::Util::FileType::FileTypeFlat could not write /home/test/.ssh/authorized_keys: could not find a temporary directory

Environment: OS: RHEL 7.3 / PE: 2016.4.x

Comment by Scott McClellan [ 2018/08/21 ]

Branan Riley to add some detail to what the desired behavior is and mark as Accepted.

Comment by Greg [ 2020/03/18 ]

We're seeing this, but with the root user.  Short version:

user { 'root':
  ensure         => present,
  name           => 'root',
  home           => '/root',
  purge_ssh_keys => true,
  password       => 'redacted';
}
 
@user { 'someone2':
  ensure     => 'present',
  uid        => '1234',
  shell      => '/bin/bash',
  comment    => 'someone2@work.com',
  home       => '/home/someone2',
  allowdupe  => true,
  purge_ssh_keys => true,
  groups     => 'users',
  managehome => true;
}
@ssh_authorized_key {
  'ldapuser_someone2_gen_1':
    user    => 'someone2',
    require => User['someone2'],
    type    => 'ssh-rsa',
    key     => 'alongkeygoeshere'
  'ldapuser_someone2_gen_2':
    user    => 'someone2',
    require => User['someone2'],
    type    => 'ssh-ed25519',
    key     => 'ashortkeygoeshere';
}
 
[...]
 
realize(User[$username])
Ssh_Authorized_Key <| user == $username |>

During the first puppet run, we get:

Notice: /Stage[main]/Base::Linux::All_distros::Root/Ssh_authorized_key[ldapuser_root_someone1_gen_3]/ensure: removed
Notice: /Stage[main]/Base::Linux::All_distros::Root/Ssh_authorized_key[ldapuser_root_someone2_gen_1]/ensure: removed
Notice: /Stage[main]/Base::Linux::All_distros::Root/Ssh_authorized_key[ldapuser_root_someone1_gen_4]/ensure: removed
Notice: /Stage[main]/Base::Linux::All_distros::Root/Ssh_authorized_key[ldapuser_root_someone3_gen_1]/ensure: removed
Notice: /Stage[main]/Base::Linux::All_distros::Root/Ssh_authorized_key[ldapuser_root_someone1_gen_1]/ensure: removed[...]Notice: /Stage[main]/Ldap_users::Admin/User[someone1]/ensure: created
Notice: /Stage[main]/Ldap_users::Admin/User[someone2]/ensure: created
Notice: /Stage[main]/Ldap_users::Admin/User[someone3]/ensure: created
Notice: /Stage[main]/Ldap_users::Admin/Ssh_authorized_key[ldapuser_someone1_gen_1]/ensure: created
Notice: /Stage[main]/Ldap_users::Admin/Ssh_authorized_key[ldapuser_someone1_gen_2]/ensure: created
Notice: /Stage[main]/Ldap_users::Admin/Ssh_authorized_key[ldapuser_someone1_gen_3]/ensure: created
Notice: /Stage[main]/Ldap_users::Admin/Ssh_authorized_key[ldapuser_someone2_gen_1]/ensure: created
Notice: /Stage[main]/Ldap_users::Admin/Ssh_authorized_key[ldapuser_someone2_gen_2]/user: user changed 'root' to 'someone2'
Notice: /Stage[main]/Ldap_users::Admin/Ssh_authorized_key[ldapuser_someone2_gen_2]/target: target changed '/root/.ssh/authorized_keys' to '/home/someone2/.ssh/authorized_keys'
Error: Puppet::Util::FileType::FileTypeFlat could not write /root/.ssh/authorized_keys: Permission denied @ rb_sysopen - /root/.ssh/authorized_keys
Error: /Stage[main]/Ldap_users::Admin/Ssh_authorized_key[ldapuser_someone2_gen_2]: Could not evaluate: Puppet::Util::FileType::FileTypeFlat could not write /root/.ssh/authorized_keys: Permission denied @ rb_sysopen - /root/.ssh/authorized_keys
Notice: /Stage[main]/Ldap_users::Admin/Ssh_authorized_key[ldapuser_someone3_gen_1]/ensure: created
Notice: /Stage[main]/Ldap_users::Admin/Ssh_authorized_key[ldapuser_someone3_gen_2]/ensure: created
Notice: /Stage[main]/Ldap_users::Admin/Ssh_authorized_key[ldapuser_someone3_gen_3]/ensure: created

Second puppet runs and beyond don't have this issue.

I haven't been able to track this down because I don't understand the provider language, but what I've been able to figure out is, as noted above, this has something to do with the drop in privs from that 2011 era CVE.  sshkeys_core lib/puppet/provider/ssh_authorized_key/parsed.rb, the flush method drops privs by default.  In my case, because I have "all the users" getting their personal keys added on the heels of having root's pre-puppet default keys being purged, What feels like is happening is either a reuse in the provider, where nothing is guaranteeing that (once a flush has been called and privs have been dropped) a provider is operating on only one particular user and then destroying itself rather than staying around to be reused after a priv drop.

I could be grossly misinterpreting, and I'd welcome a handsmack on this.

Comment by Greg [ 2020/05/14 ]

I dug into this some more.

We have our sysadmins' pubkeys in /root/.ssh/authorized_keys on our pre-puppeted VM templates.  When we puppetize a VM from template, the root user has purge_ssh_keys = true, so we expect (and eventually get) the keys purged, and the sysadmin users created.  It always took 2 puppet runs.  From --debug on the first (flawed) run:

 

Notice: /Stage[main]/Ldap_users::Admin/Ssh_authorized_key[ldapuser_someone2_key]/target: target changed '/root/.ssh/authorized_keys' to '/home/someone2/.ssh/authorized_keys'
Debug: Flushing ssh_authorized_key provider target /home/someone2/.ssh/authorized_keys
Debug: Flushing ssh_authorized_key provider target /root/.ssh/authorized_keys

We've all seen the target changing, but not really noticed the particulars of 'why'.  The ssh key already exists (in our case, in root's authorized_keys).  And we've all been focusing on ssh_authorized_key.  But I think puppet:lib/puppet/type/user.rb is actually our culprit.  In unknown_keys_in_file it's finding the would-be-purged keys and they realized as {{ssh_authorized_key}}s, being ignored/kept because they're "managed by the present catalog".  I feel that there's a buried assumption there, that "in the catalog" is being used when "in the catalog and aimed at being in this user's file" would be more correct.

 

Before the DSL even has a chance to make changes, the key is in the 'wrong' place: puppet sees it under 'root'.  When it comes time to follow the catalog's instructions and create the key (with the same name), puppet then sees the key should be under 'someone2'.  There's a 'conflict' from the key being doubly defined, once by me in the catalog, and once by puppet as a pre-step to purging a key.

That's pretty close to the root cause.  Then, rolling forward from that...  puppet:lib/puppet/provider/parsedfile.rb - (the superclass to ssh_authorized_key/parsed) - its flush is happy to handle multiple modified items:

  @modified.sort_by(&:to_s).uniq.each do |target|

sshkeys_core:lib/puppet/provider/ssh_authorized_key/parsed.rb - the call to its flush is, as has been known, an issue.  The ssh_authorized_key provider's flush only imagined that a user would ever manipulate their own key.  The idea of 'the same key being in a shared account, or being moved' wasn't there.  The asuser call limits the privs, passes to super, which does the writing.  But at that point, having two modified files from two users and permissions to only one, of course it'll fail.

 

This is a case where humans and puppet are colliding.  In my mind, a key is a crypto line and I can add it in any file I want, multiple files even.  But to puppet, ssh_authorized_key is a specific key line in a specific file with a specific comment, known by a specific name.  Absent the weird provider, the expectation here (at least from me) is, "I didn't declare a key under root, and I did declare 'purge', so that should die because it's not catalog'ed." plus "I did declare a key under ~someone, so that should appear, because it IS catalog'ed."

It took us a long time to figure out our issue, but for us the workaround was "change all the root key comments in our templates" to avoid this.

I think AN issue (if not THE issue) would be to do deeper analysis on the .reject at the end of puppet's lib/puppet/type/user.rbunknown_keys_in_file: rather than comparing based on the object's ref, compare deeply on the ssh_authorized_key object.

"But wait, why would we change user like that?"  Well, because puppet is polluting the catalog.  If I created a dupe on a defined type, sure, I'd expect to lose.  But I'm not.  Reusing a defined type name against a moving target is fine, ala:

 

myfile { "myfile": filename => $::facts['system_uptime']['seconds']; }

 

Which is basically what's happening here: the key's comment-name is re-used, but puppet is conflating an old instance of the key that IT created, as being the same key used somewhere else.

Generated at Sat Sep 19 12:21:41 PDT 2020 using Jira 8.5.2#805002-sha1:a66f9354b9e12ac788984e5d84669c903a370049.