cloud-init team mailing list archive

[Scott Moser <smoser@xxxxxxxxxx>]

On Wed, 21 Dec 2016, Michael Felt wrote:

> I am working through cloud-init and shall pose questions as I go. I am hoping
> for enlightenment from any/all of you.
>
> Had a discussion about the /etc/cloud/cloud.cfg contents - just these two
> lines for now:
>
> Currently they are:
>
> # Delete existing SSH host keys
> ssh_deletekeys: true
>
> # Regen rsa and dsa host keys
> ssh_genkeytypes: ['rsa', 'dsa']
>
> IMHO - they should be: (because "straight dsa" is considered too "weak")
>
> # Delete existing SSH host keys
> ssh_deletekeys: true
>
> # Regen rsa and dsa host keys
> ssh_genkeytypes: ['rsa', 'ecdsa', 'ed25519']

Thats a reasonable suggestion.

> I have been told that ssh_deletekeys: true
> ONLY deletes the keys that are included in the "genkeytypes" list. certainly
> not what I expected from a "instance initialization machine". I was expecting
> it to delete all keys, regardless - in other words, by default: true means
> delete all from the default location.

Well, the code does this:
    # remove the static keys from the pristine image
    if cfg.get("ssh_deletekeys", True):
        key_pth = os.path.join("/etc/ssh/", "ssh_host_*key*")
        for f in glob.glob(key_pth):
            try:
                util.del_file(f)
            except Exception:
                util.logexc(log, "Failed deleting key file %s", f)

so effectively if /etc/ssh is the right path, then setting it to 'True'
does what you want. So i'm perfectly happy to allow that to mean:
  "Delete any found generated host keys in the system".

>
> What I see as a useability feature is an option to say something like: (I do
> not know json yet, so PLEASE, pick away - fix it!)
>
> ssh_deletekeys: true: { true, [path1, path2]} or
>
> ssh_deletekeys: true: {["key1", "key2"], ["path1", ..., "pathN"]}
>
> Again, the default, imho, should be, if true, to delete all keys from (all)
> default path(s)

I think we can punt on a more complex value than a boolean as the 'True'
does what you think it should, alhtough only for keys in /etc/ssh... i see
that as a bug in the implementation.

> The ssh_genkeytypes: ['rsa', 'ecdsa', 'ed25519'] would be very similiar -
> except we have no need for an additional 'true' - calling it implies 'true'.
> However, to make a path "override" easy to see and do have the call ALSO
> support:
>
> ssh_genkeytypes: { ['rsa', 'ecdsa', 'ed25519'], ["path1", ..., "pathN"]}

One thing to note is that we are generally moving to namespacing
configuration under a top level key.  Ie, many config modules are moving
from:
  ssh_genkeytypes: ..
  ssh_deletekeys: true

to the more sane and extensible:

  ssh:
    genkeytypes: ...
    deletekeys: true

And then generally, I prefer to use dictionaries over lists, so:
ssh:
  genkeytypes:
    rsa:
      path: /etc/ssh/ssh_host_key_rsa
      type: rsa
    ecdsa:
      path: /etc/ssh/ssh_host_key_rsa
      type: ecdsa

Above, the key name ('rsa' or 'ecdsa') would largely be ignored, but we
could have the type default to the key name if not provided.

But that is much more verbose for someone wanting to modify config or
change it.

> So, in summary
>
> a) is current behavior to only delete the keys selected for generation - other
> keys, if any, are not deleted.

No.

> b) change default behavior for delete to ALL in (ALL) default location(s)

Sounds like the original intent for 'true'.

> c) add a way to specify both specific keys and/or (additional) paths to both
> "calls"

I agree with generate for sure, i'm not convinced that a boolean for
delete doesnt suffice, with code that "does the right thing" (deleting
them from whatever directory they'd likely be in).