[systemd-devel] [PATCH 1/3] Add more password agent information

[Lennart Poettering]

On Wed, 12.02.14 23:55, David Härdeman (david at hardeman.nu) wrote:

Sorry for the late review!

> Add an (optional) "Id" key in the password agent .ask files. The Id is
> supposed to be a simple string in "<subsystem>:<target>" form which
> is used to provide more information on what the requested passphrase
> is to be used for (which e.g. allows an agent to only react to cryptsetup
> requests).

I wonder if this is related to the "keyhandler" stuff Benjamin Sans has
asked for.

http://lists.freedesktop.org/archives/systemd-devel/2014-March/017869.html

Benjamin, can you comment?

> -        r = ask_password_auto(text, "drive-harddisk", until, accept_cached, passwords);
> +	if (asprintf(&id, "cryptsetup:%s", name) < 0)
> +		return log_oom();
> +
> +        r = ask_password_auto(text, "drive-harddisk", id, until,
> accept_cached, passwords);

Hmm, no tabs please...

Also, please use strappend() for cases like this, where we just want to
concatenate two strings.

That said, I wodner, if we should escape the second part of the string,
just to be sure. Using cescape() here would suffice?

>          if (r < 0) {
>                  log_error("Failed to query password: %s", strerror(-r));
>                  return r;
> @@ -281,7 +285,10 @@ static int get_password(const char *name, usec_t until, bool accept_cached, char
>                  if (asprintf(&text, "Please enter passphrase for disk %s! (verification)", name) < 0)
>                          return log_oom();
>  
> -                r = ask_password_auto(text, "drive-harddisk", until, false, &passwords2);
> +		if (asprintf(&id, "cryptsetup-verification:%s", name) < 0)
> +			return log_oom();
> +

Similar here.

Otherwise this looks good to go, but I'd like to see a comment by
Benjamin, to see if this would work for him, too!

Lennart

-- 
Lennart Poettering, Red Hat