Purging of ssh_authorized_keys

[Felix Frank]

Hi,

I'm forking this thread from a cron discussion on the development list.
I feel that the exchange of design ideas regarding the much requested
cleaning of authorized ssh public keys is of interest to the base of
(potential) users and is not (yet) closely related to implementation
details.

On 11/21/2013 09:10 PM, John Bollinger wrote:
>
> There is a similar request for ssh_authorized_keys, which is
> just about
> at the top of the highly voted issues (see [2]).
>
>
>
> There are indeed similarities here with Ssh_authorized_key, but also
> some important differences. Consider that Ssh_authorized_key can
> manage any file as a keystore (see the 'target' parameter). As such,
> it is flatly impossible for Puppet to reliably determine from which
> files it must purge keys to zap them all without collateral damage.
> Puppet could be conservative by only purging from files having the
> standard name, but if Ssh_authorized_key.target has any use in the
> first place then a conservative purge would miss some keys in some
> environments. I think it would be worse to provide a key purging
> feature that doesn't do the full job than to omit key purging altogether.

I think we can agree that "clearing all public keys from all files" is
out of the question. It cannot and should not be the goal in my opinion.

For the vast majority of users, it will be important that the "active"
keys are accounted for, i.e. such keys that are installed in
authorized_keys files which will be read by an sshd to find trusted
public keys.

Taking this perspective, using the default location would be a fair
start indeed. I can think of two ways of extending that approach.

1. Try to locate and interpret the system-wide sshd configuration. Find
the actual location(s) used for authorized keys files.

2. Add a parameter (to the resources type?) that allows to override the
default location of a user's authorized keys file. Think resources {
"ssh_authorized_key": purge => true, location => "%h/.ssh_hidden/keys" }.

Both approaches could also be combined, of course.

There will always be limitations, e.g. we cannot protect the admin from
users (or other admins) running additional sshd instances with different
configurations. But doing so would be well beyond the scope of what I,
as a user, would expect from puppet.

> I think a more feasible reference model for key purging is the one
> provided by recursively-managed Files. Essentially, there is a
> container (a File resource) that establishes the scope of the purge.
> There is not currently a suitable container type for authorized keys,
> but one could certainly be created, say Ssh_authorized_keyfile or
> Ssh_authorized_keys.

I can see how this would be more in the spirit of puppet design than
what I sketched above. But wouldn't this require the manifest designer
to enumerate all key files after all?

That's quite a limitation, because usually (see user, group) purging
resources is expected to act specifically upon resources that are not
covered by the manifest at all.

I can see scenarios where such a feature would be useful, e.g. in a
defined type that manages all aspect of a user account, their authorized
keys file could be added to the list. However, if the manifest design
allows for such convenience, it will also easily allow workarounds such
as handling the key files via the concat module or just adding a dummy
for each user in order to allow the current purging mechanism to apply.
This makes me question the value of such a container type.

Best regards,
Felix

[Jo Rhett]

I agree with much of what Felix said here. Most importantly, John's "every possible file everywhere" is more than a bit extreme. (sorry, John :D ) I actually think that something even more limited than what Felix suggested would solve most desires. I believe that purging unlisted SSH keys for all users puppet is explicitly set to manage would solve 90% if not much more of the problem.

In essence, manage SSH keys for users which Puppet has defined. This fits cleanly within the Puppet model and doesn't cause unexpected behavior. This is a perfectly reasonable target and would solve most complaints. People who want all users to have their SSH keys purged would put all users in their manifests :D

> --
> You received this message because you are subscribed to the Google Groups "Puppet Users" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to puppet-users...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-users/528E6F92.6050601%40Alumni.TU-Berlin.de.
> For more options, visit https://groups.google.com/groups/opt_out.

--
Jo Rhett
Net Consonance : net philanthropy to improve open source and internet projects.

Author of Instant Puppet 3 Starter: http://www.netconsonance.com/instant-puppet-3-starter-book/

[jcbollinger]

On Thursday, November 21, 2013 3:30:56 PM UTC-6, Jo wrote:

I agree with much of what Felix said here. Most importantly, John's "every possible file everywhere" is more than a bit extreme. (sorry, John :D ) I actually think that something even more limited than what Felix suggested would solve most desires. I believe that purging unlisted SSH keys for all users puppet is explicitly set to manage would solve 90% if not much more of the problem.

Yes, it's indeed my point that purging keys from every file is not feasible.  I also agree that something more narrowly scoped would be useful, but I am arguing that it should not be achieved via Resources.purge, because the logical expectation for that approach is more than it can actually deliver.  For example, suppose I initially have these active declarations:

ssh_authorized_key { 'example':
  target => '/non/standard/location'
  key => '...',
  type => 'rsa',
  ensure => 'present',
}

resources { 'purge_authorized_keys':
  name => 'ssh_authorized_key',
  purge => true
}

Suppose further that some time after those resources have been applied to my nodes, I remove the declaration of Ssh_authorized_key['example'].  When my nodes next sync, shouldn't I expect Ssh_authorized_key['example'] to be purged?  It was previously under management, and now it's not, so how is it logical that it escapes the purge?

 

In essence, manage SSH keys for users which Puppet has defined. This fits cleanly within the Puppet model and doesn't cause unexpected behavior. This is a perfectly reasonable target and would solve most complaints. People who want all users to have their SSH keys purged would put all users in their manifests :D

If it is sufficient to restrict this behavior to users under management, then perhaps it makes sense to hang the functionality on the User resource type.  That would make User serve as exactly the sort of 'container' that I suggested over in puppet-dev for establishing the scope of the purge.  Example:

user {
  'alice':
    ensure => 'present',
    purge_ssh_keys => true;

  # And maybe even
  'bob':
    ensure => 'present',
    home => $bob_home,
    purge_ssh_keys => [
        "$bob_home/.ssh/authorized_keys",
        "$bob_home/extra_authorized_keys"
      ]
}



John

[Felix Frank]

On 11/22/2013 03:28 PM, jcbollinger wrote:
> ssh_authorized_key { 'example':
> target => '/non/standard/location'
> key => '...',
> type => 'rsa',
> ensure => 'present',
> }
>
> resources { 'purge_authorized_keys':
> name => 'ssh_authorized_key',
> purge => true
> }
>
> Suppose further that some time after those resources have been applied
> to my nodes, I remove the declaration of Ssh_authorized_key['example'].
> When my nodes next sync, shouldn't I expect
> Ssh_authorized_key['example'] to be purged? It was previously under
> management, and now it's not, so how is it logical that it escapes the
> purge?

Yes, that is indeed unfortunate.

I feel that this is actually a problem with the target parameter,
though. Below, you suggest to tie the key purging to user resources.
That is a great idea, and usually, the target file for a key is obtained
from the owning user only.

Therefor, it may be sensible to build a purging mechanism and add a
warning to the documentation for the target parameter, that it will
interfere with purging.

> In essence, manage SSH keys for users which Puppet has defined. This
> fits cleanly within the Puppet model and doesn't cause unexpected
> behavior. This is a perfectly reasonable target and would solve most
> complaints. People who want all users to have their SSH keys purged
> would put all users in their manifests :D

Oh I'm sure someone will eventually pipe up and notice that the key they
authorized for the rsync user (who is "somehow" already present on all
their boxen) won't get purged...but agreed, we can't catch all edge
cases here. You make a good case for a compromise.

> If it is sufficient to restrict this behavior to users under management,
> then perhaps it makes sense to hang the functionality on the User
> resource type. That would make User serve as exactly the sort of
> 'container' that I suggested over in puppet-dev for establishing the
> scope of the purge. Example:

This is elegant and sounds like it could be implemented without much hassle.

It's a little counter-intuitive though that the user type should have
parameters that control key removal, but no means for adding such keys.
I pondered wether it should be possible to manage a single "special" key
for each user via the user resource itself. But that would add much
unnecessary complexity to the user type, and code duplication.

> user {
> 'alice':
> ensure => 'present',
> purge_ssh_keys => true;

Hmm, hmm. Generally I like it.

It sort of feels like this should actually be a property, because it may
cause the agent to take action. But then, there would only be one state,
ssh_keys => purged, and other states like ssh_keys => present wouldn't
make any sense.

And I guess most users don't know or care about the distinction anyway ;-)

> # And maybe even
> 'bob':
> ensure => 'present',
> home => $bob_home,
> purge_ssh_keys => [
> "$bob_home/.ssh/authorized_keys",
> "$bob_home/extra_authorized_keys"
> ]
> }

Ah, good workaround for the original problem. How about even

user { 'charly':
ensure => present,
purge_ssh_keys => true,
ssh_key_files => [
"~/.ssh/authorized_keys",
"~/.ssh-intern/authorized_keys",
]
}

I.e., only accept true/false for "purge_ssh_keys", add a parameter
"ssh_key_files" (which is utterly useless when not purging) and allow a
glob character (~) that the agent will expand to the user's home directory.

But then, the additional parameter may lead users to believe that this
will choose a target implicitly:

user { 'daisy':
ensure => present,
ssh_key_files => [ "~/.openssh/authorized_keys" ],
}
ssh_authorized_key {
'daisy@gate":
ensure => present,
user => "daisy",
key => "...",
}

So John's initial suggestion may be safer.

Kind regards,
Felix