The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com

Feature #174

[PATCH] A native authorized_key type is available

Added by Redmine Admin about 8 years ago. Updated about 6 years ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Luke Kanies% Done:

0%

Category:newfeature
Target version:0.25.0
Affected Puppet version: Branch:
Keywords:

We've Moved!

Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com

This issue is currently not available for export. If you are experiencing the issue described below, please file a new ticket in JIRA. Once a new ticket has been created, please add a link to it that points back to this Redmine ticket.


Description

Hi,

When i tested the type sshkey it does not update my ~/authorizedkey file but the /etc/ssh/ssh_known_hosts file so it seem that there is an error there ? i am using .17.2




sshkey { backup01:
        ensure  => present,
        key     =>
'AAAABddfgdfgdorifjkshdkjflsdgmfgmsdiugfmiugsdmiufgmuisdgfmsodugfHLrwDE$
        type => ssh-rsa
}

i got:

core1:/root%(root)> more /etc/ssh/ssh_known_hosts
# HEADER: This file was autogenerated at Wed Jun 14 10:40:34 CEST 2006
# HEADER: by puppet.  While it can still be managed manually, it
# HEADER: is definitely not recommended.
backup01 ssh-rsa AAAAsdgfdsfsdfsdf.....

It seems then that the file updated is not the good one. or then the sshkey type should be named the sshknowhost type but there is a problem somewhere here.

Lets say we talk about SSHkeys for user authentification as i understand it.

I think it also needs a “user” parameters to specify which user will get the keyfile updated. The file which have the keys are

~/.ssh/authorized_keys

   user => ['root','backupuser']

Also, keys can have options like those from sshd manpage :

     The options (if present) consist of comma-separated option specifica-
     tions.  No spaces are permitted, except within double quotes.  The fol-
     lowing option specifications are supported (note that option keywords are
     case-insensitive):

     from="pattern-list"
             Specifies that in addition to public key authentication, the
             canonical name of the remote host must be present in the comma-
             separated list of patterns (@*' and @?' serve as wildcards).  The
             list may also contain patterns negated by prefixing them with
             @!'; if the canonical host name matches a negated pattern, the
             key is not accepted.  The purpose of this option is to optionally
             increase security: public key authentication by itself does not
             trust the network or name servers or anything (but the key); how-
             ever, if somebody somehow steals the key, the key permits an
             intruder to log in from anywhere in the world.  This additional
             option makes using a stolen key more difficult (name servers
             and/or routers would have to be compromised in addition to just
             the key).

     command="command"
             Specifies that the command is executed whenever this key is used
             for authentication.  The command supplied by the user (if any) is
             ignored.  The command is run on a pty if the client requests a
             pty; otherwise it is run without a tty.  If an 8-bit clean chan-
             nel is required, one must not request a pty or should specify
             no-pty.  A quote may be included in the command by quoting it
             with a backslash.  This option might be useful to restrict cer-
             tain public keys to perform just a specific operation.  An exam-
             ple might be a key that permits remote backups but nothing else.
             Note that the client may specify TCP and/or X11 forwarding unless
             they are explicitly prohibited.  Note that this option applies to
             shell, command or subsystem execution.

     environment="NAME=value"
             Specifies that the string is to be added to the environment when
             logging in using this key.  Environment variables set this way
             override other default environment values.  Multiple options of
             this type are permitted.  Environment processing is disabled by
             default and is controlled via the [[PermitUserEnvironment]] option.
             This option is automatically disabled if [[UseLogin]] is enabled.

     no-port-forwarding
             Forbids TCP forwarding when this key is used for authentication.
             Any port forward requests by the client will return an error.
             This might be used, e.g., in connection with the command option.

     no-X11-forwarding
             Forbids X11 forwarding when this key is used for authentication.
             Any X11 forward requests by the client will return an error.

     no-agent-forwarding
             Forbids authentication agent forwarding when this key is used for
             authentication.

     no-pty  Prevents tty allocation (a request to allocate a pty will fail).

     permitopen="host:port"
             Limit local @@ssh -L_ port forwarding such that it may only con-
             nect to the specified host and port.  IPv6 addresses can be spec-
             ified with an alternative syntax: host/port.  Multiple permitopen
             options may be applied separated by commas.  No pattern matching
             is performed on the specified hostnames, they must be literal
             domains or addresses.


etc…see man sshd for complete list :)

so i think an “option” parameters would be handy too.


options => ['from=192.168.1.1','no-agent-forwarding']

As a last thing the doc should be update as it is obviously a cut and past from ‘host’ ;)

also i do think that the ‘host’ parameters is in fact the “option” parameters that was copypasted without change :) So i think addind options support is easy by renaming the “host” name to “options”. One more tricky thing is the user part as it can be an array of user so it could affect the way puppet mmanage the entries in the key fileS.

regards, Ghislain.

History

#1 Updated by Luke Kanies over 6 years ago

  • Status changed from 1 to Closed
  • 7 set to fixed

Merged and pushed.

#2 Updated by Francois Deppierraz over 6 years ago

Tests are already included.

#3 Updated by Luke Kanies about 8 years ago

I’m converting this to an enhancement request. It would be nice if this class, or a subclass, also managed authorized keys files, but that’s clearly a feature request, not a bug.

#4 Updated by Francois Deppierraz over 6 years ago

Well, after messing a bit with git, everything is available in the ssh-authorized-key branch in my repository.

#5 Updated by Luke Kanies over 7 years ago

Renaming the ticket. Note there’s a recipe available at Authorized_keysRecipe.

#6 Updated by Francois Deppierraz over 6 years ago

Replying to [comment:18 anarcat]:

You must checkout the correct branch if you want the latest version of this type.

git clone git://francois.ctrlaltdel.ch/puppet.git
cd puppet
git checkout origin/ssh-authorized-key
cp lib/puppet/type/ssh_authorized_key.rb /usr/lib/ruby/1.8/puppet/type/
mkdir /usr/lib/ruby/1.8/puppet/provider/ssh_authorized_key
cp lib/puppet/provider/ssh_authorized_key/parsed.rb /usr/lib/ruby/1.8/puppet/provider/ssh_authorized_key/parsed.rb
  • if no type is defined, it is set to “none” and then is not parseable anymore and breaks puppet:

Thanks for the report, this is fixed in my ssh-authorized-key branch.

  • @target@ should probably be named @path@ to follow the native @File@ type convention

Many other types based on parsedfile (@host@, @interface@, etc.) are using a @target@ parameter. I think we should keep this name coherent with those types.

  1. Puppet manages /root/.ssh/id_dsa{,.pub} on server N and provides facts ($ssh_pubkey_N) for those keys

This could be provided by another custom type calling ssh-keygen and a fact which returns the public key of each user key.

I can use the native File type or this patch to manage (3).

The advantage of this patch over a File resource is that you can easily handle multiple keys in a single user account.

But I don’t feel this is the revolution I was waiting for, what am I missing?

I had no revolutionary goals on this one ;) But it might a piece toward easier ssh keys management.

Oh, and if I might add, the ssh_authorized_keys type should at least set proper permissions when you set the user parameter (which is chown user).

Yes, this is a good point.

I’m trying to found out how this could be implemented using the parsedfile provider.

Should I better create file resources to set that or call File.chown myself ? Any hint is welcome !

#7 Updated by anarcat - over 6 years ago

For those that want to try this out and that are not tracking puppet through git directly, it’s fairly simple, provided you can clone a git repository, you don’t need to deploy the whole puppet from git:

git clone http://francois.ctrlaltdel.ch/dev/puppet.git
cd puppet
cp lib/puppet/type/ssh_authorized_key.rb /usr/lib/ruby/1.8/puppet/type/
mkdir /usr/lib/ruby/1.8/puppet/provider/ssh_authorized_key
cp lib/puppet/provider/ssh_authorized_key/parsed.rb /usr/lib/ruby/1.8/puppet/provider/ssh_authorized_key/parsed.rb

It seems to work generally well, for its intended function (namely to manage .ssh/authorized_keys), although there’s a few caveats that should probably be fixed before being merged in the main tree:

  • if no type is defined, it is set to “none” and then is not parseable anymore and breaks puppet:
    err: //Node[mumia]/Ssh_authorized_key[/tmp/testkey]: Failed to retrieve current state of resource: Could not parse line “none AAAA…”
    
  • @target@ should probably be named @path@ to follow the native @File@ type convention

I also feel that there’s some part of the puzzle missing. Now I can write /home/foo/.ssh/authorized_keys, great. But I could already write it before (through the native @File@ type). Sure I can set options and whatnot, but i still can’t manage my ssh key on box A and make sure that box can log on box B through authorized_keys because I can’t read /home/bar/.ssh/id_dsa.pub on box B.

My use case is this:

  1. I have a bunch of servers (N) that are managed under puppet and need to be backed up to backup server B
  2. Puppet manages /root/.ssh/id_dsa{,.pub} on server N and provides facts ($ssh_pubkey_N) for those keys
  3. Puppet manages /home/backup-N/.ssh/authorized_keys which is created with the $ssh_pubkey_N content

I can do (1) with a string of exec and file directives (which I hacked over DavidS’s ssh module in a so awful way I won’t even show it here).

I would have to write a facter (I guess) for (2).

I can use the native File type or this patch to manage (3).

But I don’t feel this is the revolution I was waiting for, what am I missing?

#8 Updated by James Turnbull over 6 years ago

  • Status changed from 4 to 1

#9 Updated by Luke Kanies over 6 years ago

You are correct, thanks.

I’ll look at it tonight, hopefully.

#10 Updated by anarcat - over 6 years ago

Oh, and if I might add, the ssh_authorized_keys type should at least set proper permissions when you set the @user@ parameter (which is chown @user@).

#11 Updated by Luke Kanies over 6 years ago

I just tried pulling from your repo and got this:

luke@phage(0) $ git remote add ctraltdel http://francois.ctrlaltdel.ch/dev/puppet.git
luke@phage(0) $ git fetch ctrlaltdel
fatal: 'ctrlaltdel': unable to chdir or not a git archive
fatal: The remote end hung up unexpectedly
Cannot get the repository state from ctrlaltdel
luke@phage(0) $

#12 Updated by Francois Deppierraz over 6 years ago

You can pull a working version with tests included from my repository at http://francois.ctrlaltdel.ch/dev/puppet.git on branch 0.24.x.

Do you have some advice to have this merged in next release ?

#13 Updated by Luke Kanies over 6 years ago

  • Status changed from Closed to 4
  • 7 deleted (wontfix)

#14 Updated by Francois Deppierraz over 6 years ago

Mmmh, sounds like a typo: ctraltdel != ctrlaltdel ;)

#15 Updated by Luke Kanies over 6 years ago

Any chance I could get some tests for this? I’m trying to hold a strict line on accepting types into core that don’t have tests, because of how poorly the interface type has worked out.

#16 Updated by Luke Kanies almost 7 years ago

  • Status changed from 1 to Closed
  • 7 set to wontfix

I agree it would be nice to have, but at this point, I’m not sure it belongs in the ticket db.

#17 Updated by Luke Kanies about 8 years ago

I agree that this functionality should exist, and it can probably be done within the same type, but this is neither a major flaw nor a high-priority one.

#18 Updated by James Turnbull about 6 years ago

  • Target version changed from 4 to 0.25.0

Also available in: Atom PDF