On Mon, Aug 15, 2016 at 04:01:13PM +0200, Jakub Hrozek wrote:
> Hi,
>
> attached are three small but important patches related to sssd-secrets.
> The context is that I started to write tests and manpage for
> sssd-secrets and noticed some issues. I hope the patches themselves
> offer a nice commit message.
>
> To test the socket activation, you can just install the RPMs built with
> these patches and call the socket responder, for example:
> curl -H "Content-Type: application/json" \
> --unix-socket /var/run/secrets.socket \
> -XGET http://localhost/secrets/
>
> (You can see more examples in the WIP manpage at:
> https://github.com/jhrozek/sssd/commit/508c9eec040cada06bc6a61fe200f2db20a0735b)
>
> If these patches are accepted, I'll also amend the manpage to list the
> service and the socket.
>
> btw it occured to me we might want to support service self-termination,
> but I would prefer to do that in another patchset and tracked with a
> ticket.
Let's try it with the patches actually..
>From 32dbacd4059a52a9982a1ba32bfa82d8e6a358cf Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Mon, 8 Aug 2016 14:07:04 +0200
Subject: [PATCH 1/3] UTIL: Use sss_atomic_read_s in generate_csprng_buffer
There was a bug in generate_csprng_buffer() where if we read the exact
amount of bytes from /dev/urandom, we would always return EIO. Instead,
let's reuse the existing code from sss_atomic_read_s() which fixes this
bug and reduces code duplication.
---
Makefile.am | 2 ++
src/util/crypto/sss_crypto.c | 29 +++++------------------------
2 files changed, 7 insertions(+), 24 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index
5d1d671096f986d9387e6199112c017e9bf30e1b..7c15bd7cdb336bc38f2055382121d73a3583b1e7
100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -812,6 +812,7 @@ if HAVE_NSS
src/util/crypto/nss/nss_nite.c \
src/util/crypto/nss/nss_util.c \
src/util/crypto/sss_crypto.c \
+ src/util/atomic_io.c \
$(NULL)
SSS_CRYPT_CFLAGS = $(NSS_CFLAGS)
SSS_CRYPT_LIBS = $(NSS_LIBS)
@@ -833,6 +834,7 @@ else
src/util/crypto/libcrypto/crypto_obfuscate.c \
src/util/crypto/libcrypto/crypto_nite.c \
src/util/crypto/sss_crypto.c \
+ src/util/atomic_io.c \
$(NULL)
SSS_CRYPT_CFLAGS = $(CRYPTO_CFLAGS)
SSS_CRYPT_LIBS = $(CRYPTO_LIBS)
diff --git a/src/util/crypto/sss_crypto.c b/src/util/crypto/sss_crypto.c
index
4c775f3d926ae32f3cb72b1329c0a025a0550ed5..ac90bac07c7006a2950331b86bcc412207a3e401
100644
--- a/src/util/crypto/sss_crypto.c
+++ b/src/util/crypto/sss_crypto.c
@@ -25,41 +25,22 @@
int generate_csprng_buffer(uint8_t *buf, size_t size)
{
ssize_t rsize;
- ssize_t pos;
int ret;
int fd;
fd = open("/dev/urandom", O_RDONLY);
if (fd == -1) return errno;
- rsize = 0;
- pos = 0;
- while (rsize < size) {
- rsize = read(fd, buf + pos, size - pos);
- switch (rsize) {
- case -1:
- if (errno == EINTR) continue;
- ret = EIO;
- goto done;
- case 0:
- ret = EIO;
- goto done;
- default:
- if (rsize + pos < size - pos) {
- pos += rsize;
- continue;
- }
- ret = EIO;
- goto done;
- }
- }
- if (rsize != size) {
+ rsize = sss_atomic_read_s(fd, buf, size);
+ if (rsize == -1) {
+ ret = errno;
+ goto done;
+ } else if (rsize != size) {
ret = EFAULT;
goto done;
}
ret = EOK;
-
done:
close(fd);
return ret;
--
2.4.11
>From bbd6be585da9cf7b14ac8c02e8ecca072753bc2e Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Mon, 8 Aug 2016 13:50:54 +0200
Subject: [PATCH 2/3] SECRETS: Use sss_atomic_read/write for better readability
sss_atomic_read_s and sss_atomic_write_s are macro-wrappers around
sss_atomic_io_s but it's easier to follow the code with the read/write
vairants used directly.
---
src/responder/secrets/local.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c
index
470aec0e195a54dd2af2b929ff1b7a304331a214..17469249b357cbdc5e50ddff6b563fdf2f377577
100644
--- a/src/responder/secrets/local.c
+++ b/src/responder/secrets/local.c
@@ -621,7 +621,7 @@ int generate_master_key(const char *filename, size_t size)
fd = open(filename, O_CREAT|O_EXCL|O_WRONLY, 0600);
if (fd == -1) return errno;
- rsize = sss_atomic_io_s(fd, buf, size, false);
+ rsize = sss_atomic_write_s(fd, buf, size);
close(fd);
if (rsize != size) {
ret = unlink(filename);
@@ -681,8 +681,8 @@ int local_secrets_provider_handle(struct sec_ctx *sctx,
}
if (ret) return EFAULT;
- size = sss_atomic_io_s(mfd, lctx->master_key.data,
- lctx->master_key.length, true);
+ size = sss_atomic_read_s(mfd, lctx->master_key.data,
+ lctx->master_key.length);
close(mfd);
if (size < 0 || size != lctx->master_key.length) return EIO;
--
2.4.11
>From 86a24747f45bdb3caeb8c36d63c74a08aed90421 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Mon, 15 Aug 2016 14:10:23 +0200
Subject: [PATCH 3/3] BUILD: Ship systemd service file for sssd-secrets
Adds two new files: sssd-secrets.socket and sssd-secrets.service. These
can be used to socket-acticate the secrets responder even without
explicitly starting it in the sssd config file.
The specfile activates the socket after installation which means that
the admin would just be able to use the secrets socket and the
sssd_secrets responder would be started automatically by systemd.
The sssd-secrets responder is started as root, mostly because I didn't
think of an easy way to pass the uid/gid to the responders without
asking about the sssd user identity in the first place. But nonetheless,
the sssd-secrets responder wasn't tested as non-root and at least the
initialization should be performed as root for the time being.
---
Makefile.am | 21 +++++++++++++++++++--
contrib/sssd.spec.in | 5 +++++
src/sysv/systemd/sssd-secrets.service.in | 2 ++
src/sysv/systemd/sssd-secrets.socket.in | 5 +++++
4 files changed, 31 insertions(+), 2 deletions(-)
create mode 100644 src/sysv/systemd/sssd-secrets.service.in
create mode 100644 src/sysv/systemd/sssd-secrets.socket.in
diff --git a/Makefile.am b/Makefile.am
index
7c15bd7cdb336bc38f2055382121d73a3583b1e7..5d38fc96462356471f1f1506403b2d76961d485d
100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3848,7 +3848,10 @@ systemdunit_DATA =
systemdconf_DATA =
if HAVE_SYSTEMD_UNIT
systemdunit_DATA += \
- src/sysv/systemd/sssd.service
+ src/sysv/systemd/sssd.service \
+ src/sysv/systemd/sssd-secrets.socket \
+ src/sysv/systemd/sssd-secrets.service \
+ $(NULL)
if WITH_JOURNALD
systemdconf_DATA += \
src/sysv/systemd/journal.conf
@@ -3886,6 +3889,7 @@ edit_cmd = $(SED) \
-e 's|@sbindir[@]|$(sbindir)|g' \
-e 's|@environment_file[@]|$(environment_file)|g' \
-e 's|@localstatedir[@]|$(localstatedir)|g' \
+ -e 's|@libexecdir[@]|$(libexecdir)|g' \
-e 's|@prefix[@]|$(prefix)|g'
replace_script = \
@@ -3897,7 +3901,10 @@ replace_script = \
EXTRA_DIST += \
src/sysv/systemd/sssd.service.in \
- src/sysv/systemd/journal.conf.in
+ src/sysv/systemd/journal.conf.in \
+ src/sysv/systemd/sssd-secrets.socket.in \
+ src/sysv/systemd/sssd-secrets.service.in \
+ $(NULL)
src/sysv/systemd/sssd.service: src/sysv/systemd/sssd.service.in Makefile
@$(MKDIR_P) src/sysv/systemd/
@@ -3907,6 +3914,14 @@ src/sysv/systemd/journal.conf:
src/sysv/systemd/journal.conf.in Makefile
@$(MKDIR_P) src/sysv/systemd/
$(replace_script)
+src/sysv/systemd/sssd-secrets.socket: src/sysv/systemd/sssd-secrets.socket.in
Makefile
+ @$(MKDIR_P) src/sysv/systemd/
+ $(replace_script)
+
+src/sysv/systemd/sssd-secrets.service:
src/sysv/systemd/sssd-secrets.service.in Makefile
+ @$(MKDIR_P) src/sysv/systemd/
+ $(replace_script)
+
SSSD_USER_DIRS = \
$(DESTDIR)$(dbpath) \
$(DESTDIR)$(keytabdir) \
@@ -4122,6 +4137,8 @@ endif
done;
rm -Rf ldb_mod_test_dir
rm -f $(builddir)/src/sysv/systemd/sssd.service
+ rm -f $(builddir)/src/sysv/systemd/sssd-secrets.socket
+ rm -f $(builddir)/src/sysv/systemd/sssd-secrets.service
rm -f $(builddir)/src/sysv/systemd/journal.conf
CLEANFILES += *.X */*.X */*/*.X
diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
index
a9a43560e61acf1cb4b2388d50303587252c2c19..b58eebba54a82a041f72507b430ceca708a34632
100644
--- a/contrib/sssd.spec.in
+++ b/contrib/sssd.spec.in
@@ -746,6 +746,8 @@ done
%{_sbindir}/sssd
%if (0%{?use_systemd} == 1)
%{_unitdir}/sssd.service
+%{_unitdir}/sssd-secrets.socket
+%{_unitdir}/sssd-secrets.service
%else
%{_initrddir}/%{name}
%endif
@@ -1077,12 +1079,15 @@ getent passwd sssd >/dev/null || useradd -r -g sssd -d
/ -s /sbin/nologin -c "Us
# systemd
%post common
%systemd_post sssd.service
+%systemd_post sssd-secrets.socket
%preun common
%systemd_preun sssd.service
+%systemd_preun sssd-secrets.socket
%postun common
%systemd_postun_with_restart sssd.service
+%systemd_postun_with_restart sssd-secrets.socket
%else
# sysv
diff --git a/src/sysv/systemd/sssd-secrets.service.in
b/src/sysv/systemd/sssd-secrets.service.in
new file mode 100644
index
0000000000000000000000000000000000000000..4236cc2eb85c83c574da8a96ca9d760922aacbd9
--- /dev/null
+++ b/src/sysv/systemd/sssd-secrets.service.in
@@ -0,0 +1,2 @@
+[Service]
+ExecStart=@libexecdir@/sssd/sssd_secrets --uid 0 --gid 0 --debug-to-files
diff --git a/src/sysv/systemd/sssd-secrets.socket.in
b/src/sysv/systemd/sssd-secrets.socket.in
new file mode 100644
index
0000000000000000000000000000000000000000..239ac496caa826ee0f032041f85d94a05f5aef94
--- /dev/null
+++ b/src/sysv/systemd/sssd-secrets.socket.in
@@ -0,0 +1,5 @@
+[Socket]
+ListenStream=@localstatedir@/run/secrets.socket
+
+[Install]
+WantedBy=sockets.target
--
2.4.11
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
