Re: [systemd-devel] [PATCH] [RFC] Add binary password agent protocol
On Mon, 14.07.14 15:43, David Härdeman (da...@hardeman.nu) wrote: > No problem. Getting it right is part of the systemd philosophy...that > sometime takes longer > > Anyhow, I've looked at the in-kernel keyring stuff before. Basically > userspace can request a key via a syscall (or in-kernel code can do > pretty much the same thing via a similar function call). > > If the key is missing, a placeholder gets created and /sbin/request-key > is invoked to fill that placeholder with a proper key. The current > request-key uses config files under /etc to determine how to handle the > key request. > > The key can then be properly constructed by the add_key sysctl (to write > the payload to the key). > > As far as I can tell, this could replace the sockets that are currently > used in systemd, but not much more. All the information in the ask. > files would still need to be conveyed separately (e.g. by still creating > the ask. file and providing the path to the file as the callout_info > in request_key). > > I'm guessing you'd like the dbus API to be a higher-level API that > userspace can use to get systemd to create the ask. file and call > request_key()? Are there any advantages of that redirection over doing > it straight away in the app? I'd really like to drop the ask. files, too, and do that on the bus. Some bus API, where clients that can't find the password they need in the kernel keyring can ask for it in a nice way, which is then dispatched to a number of agents, which will place the key in the keyring and then notify the client about it. This piece of infrastructure should probably also be able to persist specific keys to disk, a bit like gnome-keyring, but much simpler because it would only be a persistancy frontend for the kernel keyring. It should then also do both system-level and user-level keyring handling... > Also, I guess this means that systemd would have to add a homegrown > version of /sbin/request-key (external dependencies for early boot does > not seem to be part of the systemd way of doing things)? Well, yeah, I figure. I think the API is awful though. In this day and age we really shouldn't have a kernel callout-to-userspace anymore, but use something else, like some netlink event or so... But anyway we shuld be able to hide this away sufficiently. The idea is that we would then just make this stub binary drop the request on the bus and get nice interactivity. Hope this makes some sense. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] [RFC] Add binary password agent protocol
On Thu, Jul 03, 2014 at 01:41:43PM +0200, Lennart Poettering wrote: >On Fri, 27.06.14 01:54, David Härdeman (da...@hardeman.nu) wrote: > >> Add binary string handling functions and extend the password agent >> protocol to support binary strings (using "=" as a string prefix >> instead of "+"). > >I am feeling a bit uneasy about this one. I have the suspicion that the >entire password logic should be changed around: we should never transfer >the passwords in userspace, but use the kernel keyring for this. And the >queries should probably be triggered via dbus (as soon as kdbus is done, >and we can use dbus in early-boot). > >THis all makes me want to stay away from this for now. The kernel >keyring is binary-safe anyway, so half the problem goes away there. The >kernel also already has a key request API iirc (though a weird one, from >a cursory look), so we really should align ourselves a lot more with >that. > >Sorry if this sounds disappointing, but I think the proper fix to get >binary passwords done is the kernel keyring, not just polishing what we >have right now. No problem. Getting it right is part of the systemd philosophy...that sometime takes longer Anyhow, I've looked at the in-kernel keyring stuff before. Basically userspace can request a key via a syscall (or in-kernel code can do pretty much the same thing via a similar function call). If the key is missing, a placeholder gets created and /sbin/request-key is invoked to fill that placeholder with a proper key. The current request-key uses config files under /etc to determine how to handle the key request. The key can then be properly constructed by the add_key sysctl (to write the payload to the key). As far as I can tell, this could replace the sockets that are currently used in systemd, but not much more. All the information in the ask. files would still need to be conveyed separately (e.g. by still creating the ask. file and providing the path to the file as the callout_info in request_key). I'm guessing you'd like the dbus API to be a higher-level API that userspace can use to get systemd to create the ask. file and call request_key()? Are there any advantages of that redirection over doing it straight away in the app? Also, I guess this means that systemd would have to add a homegrown version of /sbin/request-key (external dependencies for early boot does not seem to be part of the systemd way of doing things)? -- David Härdeman ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] [RFC] Add binary password agent protocol
On Fri, 27.06.14 01:54, David Härdeman (da...@hardeman.nu) wrote: > Add binary string handling functions and extend the password agent > protocol to support binary strings (using "=" as a string prefix > instead of "+"). I am feeling a bit uneasy about this one. I have the suspicion that the entire password logic should be changed around: we should never transfer the passwords in userspace, but use the kernel keyring for this. And the queries should probably be triggered via dbus (as soon as kdbus is done, and we can use dbus in early-boot). THis all makes me want to stay away from this for now. The kernel keyring is binary-safe anyway, so half the problem goes away there. The kernel also already has a key request API iirc (though a weird one, from a cursory look), so we really should align ourselves a lot more with that. Sorry if this sounds disappointing, but I think the proper fix to get binary passwords done is the kernel keyring, not just polishing what we have right now. Sorry (in particular, because I didn't reply to your mail any more quicker, but I was unsure about this whole thing myself...), Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] [RFC] Add binary password agent protocol
On Thu, Jun 26, 2014 at 4:54 PM, David Härdeman wrote: > Add binary string handling functions and extend the password agent > protocol to support binary strings (using "=" as a string prefix > instead of "+"). Please also add /test-bstrv to .gitignore. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] [RFC] Add binary password agent protocol
Add binary string handling functions and extend the password agent
protocol to support binary strings (using "=" as a string prefix
instead of "+").
---
Makefile.am|9 +
src/ask-password/ask-password.c| 33 +--
src/cryptsetup/cryptsetup.c| 60 +++---
src/shared/ask-password-api.c | 58 +++---
src/shared/ask-password-api.h |7 -
src/shared/bstrv.c | 194
src/shared/bstrv.h | 50 +
src/test/test-bstrv.c | 122 +
.../tty-ask-password-agent.c | 11 +
9 files changed, 451 insertions(+), 93 deletions(-)
create mode 100644 src/shared/bstrv.c
create mode 100644 src/shared/bstrv.h
create mode 100644 src/test/test-bstrv.c
diff --git a/Makefile.am b/Makefile.am
index e02dede..08705a4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -736,6 +736,8 @@ libsystemd_shared_la_SOURCES = \
src/shared/sleep-config.h \
src/shared/strv.c \
src/shared/strv.h \
+ src/shared/bstrv.c \
+ src/shared/bstrv.h \
src/shared/env-util.c \
src/shared/env-util.h \
src/shared/strbuf.c \
@@ -1219,6 +1221,7 @@ tests += \
test-env-replace \
test-strbuf \
test-strv \
+ test-bstrv \
test-path-util \
test-strxcpyx \
test-unit-name \
@@ -1584,6 +1587,12 @@ test_strv_LDADD = \
libsystemd-internal.la \
libsystemd-shared.la
+test_bstrv_SOURCES = \
+src/test/test-bstrv.c
+
+test_bstrv_LDADD = \
+libsystemd-shared.la
+
test_path_util_SOURCES = \
src/test/test-path-util.c
diff --git a/src/ask-password/ask-password.c b/src/ask-password/ask-password.c
index 4d5690c..6a00ea8 100644
--- a/src/ask-password/ask-password.c
+++ b/src/ask-password/ask-password.c
@@ -39,6 +39,7 @@
#include "macro.h"
#include "util.h"
#include "strv.h"
+#include "bstrv.h"
#include "ask-password-api.h"
#include "def.h"
@@ -147,6 +148,7 @@ static int parse_argv(int argc, char *argv[]) {
int main(int argc, char *argv[]) {
int r;
usec_t timeout;
+_cleanup_bstrv_free_ bstr **passwords = NULL;
log_parse_environment();
log_open();
@@ -159,28 +161,19 @@ int main(int argc, char *argv[]) {
else
timeout = 0;
-if (arg_use_tty && isatty(STDIN_FILENO)) {
-char *password = NULL;
-
-if ((r = ask_password_tty(arg_message, timeout, NULL,
&password)) >= 0) {
-puts(password);
-free(password);
-}
-
-} else {
-char **l;
-
-if ((r = ask_password_agent(arg_message, arg_icon, arg_id,
timeout, arg_accept_cached, &l)) >= 0) {
-char **p;
-
-STRV_FOREACH(p, l) {
-puts(*p);
+if (arg_use_tty && isatty(STDIN_FILENO))
+r = ask_password_tty(arg_message, timeout, NULL, &passwords);
+else
+r = ask_password_agent(arg_message, arg_icon, arg_id, timeout,
arg_accept_cached, &passwords);
-if (!arg_multiple)
-break;
-}
+if (r >= 0) {
+bstr **p;
-strv_free(l);
+BSTRV_FOREACH(p, passwords) {
+fwrite(bstr_data(*p), bstr_length(*p), 1, stdout);
+putchar('\n');
+if (!arg_multiple)
+break;
}
}
diff --git a/src/cryptsetup/cryptsetup.c b/src/cryptsetup/cryptsetup.c
index a67d85e..a0209e4 100644
--- a/src/cryptsetup/cryptsetup.c
+++ b/src/cryptsetup/cryptsetup.c
@@ -31,6 +31,7 @@
#include "util.h"
#include "path-util.h"
#include "strv.h"
+#include "bstrv.h"
#include "ask-password-api.h"
#include "def.h"
#include "libudev.h"
@@ -260,9 +261,8 @@ static char *disk_mount_point(const char *label) {
return NULL;
}
-static int get_password(const char *name, usec_t until, bool accept_cached,
char ***passwords) {
+static int get_password(const char *name, usec_t until, bool accept_cached,
bstr ***passwords) {
int r;
-char **p;
_cleanup_free_ char *text = NULL;
_cleanup_free_ char *escaped_name = NULL;
char *id;
@@ -286,9 +286,9 @@ static int get_password(const char *name, usec_t until,
bool accept_cached, char
}
if (arg_verify) {
-_cleanup_strv_free_ char **passwords2 = NULL;
+_cleanup_bstrv_free_ bstr **passwords2 = NULL;
-assert(strv_length(*passwords) == 1);
+assert(bstrv_length(*passwords) == 1);
