[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#741644: RFS: keyringer/0.3.2-1 -- Distributed secret management using GnuPG and Git



Hi,

Silvio Rhatto wrote (14 Mar 2014 20:43:23 GMT) :
> I am looking for a sponsor for release 0.3.2-1 of "keyringer".

Great! Here's a quick code review of the changes since 0.2.9.


I'm not convinced that the key expiry check does not introduce more
severe regressions than it improves the UX. Let's see:

 * Does it support the case when I have multiple keys for a given
   recipient in my keyring, and e.g. one is expired, but the one that
   should be used is not?

 * It seems that all subkeys (regardless of their intended usage) must
   *not* be expired, else keyringer errors out. This won't work with
   people who change their encryption subkey on a regular basis,
   will it?

 * Why try to check this ourselves at all, while GnuPG will do it
   anyway (and likely in a more correct way)?

> +    candidates=(`keyringer_exec find "$BASEDIR" | grep -i "$1" | grep -e '.asc$'`)

This seems fragile to me. How about passing find something like
'"-name "*$i*.asc"' instead?

Same concerns in lib/keyringer/actions/find.

> +      echo "Could not find exact match \"$1\", please chose one of the following secrets:"

s/exact match/exact match for/
s/chose/choose/

> +Alis to clip action.
>+:   Alis to clip action.

s/Alis/Alias/

> +If you're using debian `jessie` or `unstable`, just run

s/debian/Debian/

> +#     If you run it more than that interval, no canary will
> +#     be updated.

"more than once in this time span", maybe?

> +#   - Can optinally be included in another git repository
> +#     (encrypted or plain+signed), commited and pushed

Ask your preferred spell checker :)

> +#  - Then, add the following at your ~/.profile or wherever you want your canary
> +#    be called from: "keyringer <keyring> canary".

s/at your/to your/
It's unclear to me what "calling a canary" means in this context.
Clarify?

In a few places, s/GPG/GnuPG/

> +          echo "Please check for this key or fix the recipient file."

s/check for/retrieve this key yourself/, maybe?

> +        gpg --batch --refresh-keys "$recipient"

I'm not sure this really does what you mean here. At least the GnuPG
manpage does not talk about it. Perhaps use --recv-keys for
better clarity?

> +  keyringer_exec git "$BASEDIR" gc --prune=all

Potentially losing user's data feels quite wrong, in an action called
"check", and documented as "Run maintenance checks in a keyring".

> +    echo "Fatal: please set the full GPG signature hash for key ID $recipient:"

I'm afraid "GPG signature hash" is unclear to most people.
Just say "OpenPGP fingerprint" instead?

Cheers,
--
  intrigeri
  | GnuPG key @ https://gaffer.ptitcanardnoir.org/intrigeri/intrigeri.asc
  | OTR fingerprint @ https://gaffer.ptitcanardnoir.org/intrigeri/otr.asc


Reply to: