Am 10.08.2010 01:59, schrieb Stephen Leake: > Thomas Keller <[email protected]> writes: > >> Am 07.08.2010 20:40, schrieb Stephen Leake: >>> Stephen Leake <[email protected]> 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 | [email protected] | 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
_______________________________________________ Monotone-devel mailing list [email protected] http://lists.nongnu.org/mailman/listinfo/monotone-devel
