Project

General

Profile

Actions

User story #4439

closed

Technique 'ssh keys distribution': Have several keys per users

Added by Alex Tkachenko about 10 years ago. Updated about 9 years ago.

Status:
Released
Priority:
2
Category:
Techniques
Target version:
UX impact:
Suggestion strength:
User visibility:
Effort required:
Name check:
Fix check:
Regression:

Description

I've been trying to implement an ssh key distribution policy using the stock technique and run into some limitations.
To make the long story short, I ended up modifying the technique considerably, hence there is no patch, just a new set of files. With these files I hope the following has been addressed:
  • The key definitions in the old technique were used verbatim, which made it impossible to update say, key comment, which would result into two key definitions for essentially the same key hash;
  • classes used to record the outcome would be defined globally, but the names are not specific enough (i.e. line_1_*) which may have created confusion if several rules are in effect on the same host (think of line_1_ok);
  • multiple keys for the same user within the same directive were not possible - I had to introduce a new component variable to work around that;
  • a special case for SuSE, which differs only in the gid for file ownership has been folded in using an array built conditionally depending on the OS;
  • a class name denoting existence for a user was renamed from index_*_exist to user_*_exist for clarity;
  • reports have been replaced with methods, which, I hope, makes it simpler to read;
  • classes to denote outcomes were also rewritten to use rudder_common_classes;
  • an additional edit has been introduced to ensure uniqueness of the defs in the files - this is done with a bundle remove_duplicate_lines (attached) which I have in my site library. If nobody thinks it is useful - just drop the third files promise, otherwise please feel free to include it either into the library or along with the technique (but probably in the latter case it should be given a more specific name, i.e. sshkey_remove_duplicate_lines.

Also note that I have changed the agent version to 3.5.3 - the only reason for that is the use of some cfengine functions (specifically ifelse, regextract and escape) which were not used in the original version, and I am not sure which cfengine version they have been introduced.

Please let me know if this looks useful or you have any questions or concerns.


Files

sshKeyDistribution.st (6.42 KB) sshKeyDistribution.st Alex Tkachenko, 2014-02-04 00:19
metadata.xml (2.75 KB) metadata.xml Alex Tkachenko, 2014-02-04 00:19
remove_duplicate_lines.st (1.16 KB) remove_duplicate_lines.st Alex Tkachenko, 2014-02-04 00:19
metadata.xml (3.18 KB) metadata.xml Alex Tkachenko, 2014-02-05 02:30
sshKeyDistribution.st (7.22 KB) sshKeyDistribution.st Alex Tkachenko, 2014-02-05 02:30
metadata.xml (3.15 KB) metadata.xml Alex Tkachenko, 2014-02-15 01:40
sshKeyDistribution.st (7.42 KB) sshKeyDistribution.st Alex Tkachenko, 2014-02-15 01:40

Subtasks 1 (0 open1 closed)

User story #4478: Improve the sshKeyDistribution Technique to have several keys per users, and overall technique improvementReleasedNicolas CHARLES2014-02-18Actions
Actions #1

Updated by Alex Tkachenko about 10 years ago

And I guess one edge case still remains, which is creating the keys for the user defined in the network database (i.e. LDAP) for which there is no home directory yet.

The ssh key technique would create the missing directories/files; the problem is - the home directory for the user is created with root:root ownership, and no standard template files are copied (i.e. .bashrc, etc).

While it is not appropriate for this technique to handle proper creation of user homes (which should rather be implemented elsewhere), the question is - should this technique warn about non-existent home directory and stop trying or it should do what it does currently?

Actions #2

Updated by François ARMAND about 10 years ago

  • Project changed from 34 to 24
  • Category set to Techniques
  • Assignee set to Nicolas CHARLES
  • Priority changed from N/A to 3
  • Target version set to 2.10.0~beta1

Thanks Alex for that,

I will let Nicolas look at the actual code :)

Updated by Alex Tkachenko about 10 years ago

It looks like I have submitted too soon - sorry about that.
I was deploying this technique in a real world scenario and was also designing a similar one for removing deprecated keys (which I'm gonna submit later) and run into a couple of issues.

First being the authorized_keys file naming. Historically we have authorized_keys2 deployed on a number of hosts, hence the introduction of a unique variable holding that name.

Second is a number of cfengine bugs, all related to limited size static buffer use throughout the code, specifically

I happened to inherit some legacy 2048 bit long DSA keys, which have the hash in the keyspec exceeding 1024 bytes. Using these keys for the class prefix (via canonify) caused the agent to blow up spectacularly, taking it completely down, so I came up with another unique prefix constructed out of key tag (user-defined) and tracking key (defined automatically). Hopefully it will serve the purpose.

The second bug affected the regextract use in edit_line bundle; unless limited to 1000 chars the hash for such a long key does not get extracted (although everything else does - go figure).

Also some cosmetic changes to improve readability.

A complete set of the files is attached.

Once again, the bugs have been triggered by using the keys with the hash exceeding 1024 bytes (which, I suspect have been generated on RHEL 4 with openssh v3.9p1), which is not usual these days - recommended (and default) RSA keys have a much shorter hash and I believe that since openssh v4 the length of DSA keys is limited to 1024 bits as required by FIPS, and these produce the hash of about 600-700 bytes long.

Actions #4

Updated by Nicolas CHARLES about 10 years ago

Thank you very much Alex,

This is a really neat Technique, and your explanation are very clear.
We've been away most of the week for conferences, hence the delay in the answer.
I'll be reviewing this in more details in the next days, but I already have one question and one remark. Is it necessary to define the SSH_KEY_DISTRIBUTION_CONFIG_BASENAME ?
I'm asking because it is treated as a unique variable (no iteration on the template) which is sensible, BUT if you have several Directives based on this technique, the file name will become a list of filename, and not doing what you expect.

And we are not using CFEngine 3.5.3 because of bugs introduced by this version on path/file management. But if/else is working fine with 3.5.2 (the current version used in Rudder)

Updated by Alex Tkachenko about 10 years ago

Indeed, the use of SSH_KEY_DISTRIBUTION_CONFIG_BASENAME introduces problems.

I have tested it by creating a second test directive and tried to play with different ways of templating it. I even tried to tie it to the TRACKINGKEY, which did not work either because of the differences in cardinality.

It's a shame, as I hoped I could address the local (historical) issue with omnipresent authorized_keys2, but it looks like it won't work.

Frankly speaking, I did not expect different instances to be translated into a single file (I thought each directive translates into a separate file) - maybe it is worth considering doing it this way?

Another way I considered was using genericVariableDefinition, but this would result in an additional unnecessary dependency.

In either case, unless somebody could advise on a proper way of defining global technique settings, I think we are better of just removing this variable.

Updated versions of the files are attached.

Please note, that I have also added an ifvarclass conditions (in files and methods sections) to prevent cfengine from generating duplicate messages confusing the report parser (maybe the methods section does not need it, but it makes the intention clearer and should not hurt).

Actions #6

Updated by Nicolas CHARLES about 10 years ago

Hi Alex,

Thank you for your updated version,

Due to historical reason, and constraint in the way we use CFEngine, we were forced to have all data based on one technique in only one file (namely, we can't have several bundle with the same name, and same for bodies that may be in technique)
This is an old constraint, and we are working to lift it, but as you can imagine, this a pretty big change.

I'll have a look at your Technique, thank you so much for them

Actions #7

Updated by Nicolas CHARLES about 10 years ago

  • Tracker changed from Bug to User story
  • Target version changed from 2.10.0~beta1 to 2.6.11

I'm switching this ticket to user story, as it is a new version of a technique (as per our internal processes)

Actions #8

Updated by Nicolas CHARLES about 10 years ago

  • Target version changed from 2.6.11 to 2.8.3

as the technique uses features that exists only in CFEngine 3.5, i'm targeting to 2.8

Actions #9

Updated by Nicolas CHARLES about 10 years ago

  • Status changed from New to 13
  • Target version changed from 2.8.3 to 2.6.11

i've removed all the 3.5 specific code, so targeting to 2.6

Actions #10

Updated by Alex Tkachenko about 10 years ago

I see. Thanks for the explanation. As templates are instantiated just by replacing the variable contents (i.e. no changes to bundle/body names) it would be quite difficult to overcome the name clash.

Maybe the newly introduced namespaces could help? Just a thought...

I am also curious, what have you determined to be 3.5 specific (so I could keep this in mind for the future).

Actions #11

Updated by Nicolas CHARLES about 10 years ago

  • Status changed from 13 to Pending release
Actions #12

Updated by Vincent MEMBRÉ about 10 years ago

  • Subject changed from Enhanced version of the sshKeyDistribution technique to Improve the sshKeyDistribution Technique to have several keys per users
Actions #13

Updated by Vincent MEMBRÉ about 10 years ago

  • Subject changed from Improve the sshKeyDistribution Technique to have several keys per users to Technique 'ssh keys distribution': Have several keys per users
Actions #14

Updated by Vincent MEMBRÉ about 10 years ago

  • Status changed from Pending release to Released

This bug has been fixed in Rudder 2.6.11, which was released today.
Check out:

Actions #15

Updated by Benoît PECCATTE about 9 years ago

  • Project changed from 24 to Rudder
  • Category changed from Techniques to Techniques
Actions

Also available in: Atom PDF