Re: [Monotone-devel] key management

[Thomas Keller]

Am 10.08.2010 01:59, schrieb Stephen Leake:
> Thomas Keller <address@hidden> writes:
> 
>> Am 07.08.2010 20:40, schrieb Stephen Leake:
>>> Stephen Leake <address@hidden> writes:
>>>
>>> I used the command names 'automate pubkey', 'automate dropkey', to match
>>> the corresponding non-automate commands.
>>>
>>> 'automate dropkey' drops the private key if present, as non-automate
>>> does. I didn't see any reason to change the behavior.
>>
>> Ouch - that might not be a good idea. This would e.g. enable the
>> deletion of the key which is used to authenticate the server, rendering
>> a running monotone instance completely useless. We cannot even restrict
>> the execution of this command by argument easily, ie. I don't want to
>> tell server admins to expand their get_remote_automate_permitted() hook
>> to specifically exclude the key id for this new command, this is way too
>> harmful if forgotten.
>>
>> So please, either split the functionality in two commands
>> (drop_public_key / drop_private_key) or disable key deletion over
>> automate. In the former case we could at least give sensible hints for a
>> server admin to disallow the drop_private_key command completely.
> 
> I changed 'automate dropkey' to 'automate drop_public_key'.

Ok, cool, I had the time to quickly look over the changes, some minor
things:

1) Please rename pubkey to get_public_key - we should try to keep the
command names consistent within automate. And while we're at it, rename
genkey to generate_key. We'll break BC in automate for 0.99 anyways when
nvm.options lands, so we can clean up the command naming mess on the
same run. We still have a slight inconsistency wrt getter commands (most
of them have a "get_" prefix, most of the tree commands have not
though), but thats out of scope for this branch.

2) I've seen this before in your work and now again, so unfortunately I
have to say something :) - please don't put a space before the opening
bracket in function declarations and definitions, this violates the
coding standards.

3) Please also implement put_public_key. This way we can safely
deprecate "automate read_packets" and the whole packet machinery for
file deltas, revisions, and so on, while still giving the people an
upgrade path.

4) The docs on drop_public_key should be tweaked a bit, i.e. right now
the first sentence in the Purpose section reads as if we're still
dropping private keys for which it would indeed very harmful to execute
the command, but we don't do that any longer. Even if we drop the public
key from the server, we can simply read in the private key to get it
back or sync it from another node. Furthermore it would be cool to
document that just as well, i.e. that a public key comes back on the
next sync and that the main consequence is that signatures can no longer
be verified for this particular key (point them to the new k: selector
here as well, so they have a mean to figure out if the key they want to
drop is actually in use).

5) Would it be a good idea to rename common/test_utils_inventory.lua to
something more generic now that this is used a couple of times outside
the basic inventory tests? Alternatively the common stuff could be moved
out of this file.

Other than that it looks ok.

Thanks for your work!

Thomas.

-- 
GPG-Key 0x160D1092 | address@hidden | http://thomaskeller.biz
Please note that according to the EU law on data retention, information
on every electronic information exchange might be retained for a period
of six months or longer: http://www.vorratsdatenspeicherung.de/?lang=en

signature.asc
Description: OpenPGP digital signature