openldap-bugs

openldap-bugs@openldap.org

21065 discussions

Re: (ITS#8707) slapd: Add systemd service notification support
by fumiyas＠osstech.co.jp 10 Aug '17

10 Aug '17

On Thu, 10 Aug 2017 02:16:56 +0900, Ryan Tandy wrote: > > I've revised the patch for master to check if the listener initialization > > suceeds or not. With this patch, `slapd -d0` can return non-zero exit code > > if the listener initialization fails, thus systemd can detect slapd.service > > startup failure. > > Nice catch! That change looks correct to me, but I hope Howard will check it too. Actually it's probably relevant for any init system; possibly worth splitting out as its own commit? This patch does NOT affect SysV-init system because an init script for slapd normally does NOT use the -d option. > Just checking, do you plan to add the service file and its installation (conditional on enabled systemd support) in this branch as well, or should I work on that myself if I want it? Yes, I'll do that later. Thank you. -- -- Name: SATOH Fumiyasu @ OSS Technology Corp. (fumiyas @ osstech co jp) -- Business Home: http://www.OSSTech.co.jp/ -- GitHub Home: https://GitHub.com/fumiyas/ -- PGP Fingerprint: BBE1 A1C9 525A 292E 6729 CDEC ADC2 9DCA 5E1C CBCA

1 0

(ITS#8709) pw-totp: OpenSSL 1.1 compatibility
by fumiyas＠osstech.co.jp 10 Aug '17

10 Aug '17

Full_Name: SATOH Fumiyasu Version: master OS: URL: Submission from: (NULL) (118.240.43.13) Current contrib/slapd-modules/passwd/totp is not compatible with OpenSSL 1.1+. I've created a patch to fix that. Please see the following patch I post: * https://patch-diff.githubusercontent.com/raw/osstech-jp/openldap/pull/5.pat… * https://github.com/osstech-jp/openldap/pull/5

1 0

Re: (ITS#8707) slapd: Add systemd service notification support
by ryan＠nardis.ca 09 Aug '17

09 Aug '17

On Wed, Aug 09, 2017 at 05:13:44PM +0900, SATOH Fumiyasu wrote: >I've revised the patch for master to check if the listener initialization >suceeds or not. With this patch, `slapd -d0` can return non-zero exit code >if the listener initialization fails, thus systemd can detect slapd.service >startup failure. Nice catch! That change looks correct to me, but I hope Howard will check it too. Actually it's probably relevant for any init system; possibly worth splitting out as its own commit? Just checking, do you plan to add the service file and its installation (conditional on enabled systemd support) in this branch as well, or should I work on that myself if I want it? Thanks!

1 0

Re: (ITS#8707) slapd: Add systemd service notification support
by fumiyas＠osstech.co.jp 09 Aug '17

09 Aug '17

On Tue, 08 Aug 2017 04:23:27 +0900, Ryan Tandy wrote: > > + rc = sd_notifyf( 1, > > + "READY=1\n" > > + "STATUS=slapd: ready to serve connections...\n" > > + "MAINPID=%lu", > > + (unsigned long) getpid() ); > I see you've placed this call later than the parent's exit point. Any comments about the timing of this relative to the parent's exit, and to the listener startup and so on? Are the listeners more likely to be ready to serve connections at this point? I seem to recall that in the past there was opposition to moving the parent's exit later, but I can't remember why. (and I still wish we could do that, and dispense with the ldapsearch-loop hacks...) I've revised the patch for master to check if the listener initialization suceeds or not. With this patch, `slapd -d0` can return non-zero exit code if the listener initialization fails, thus systemd can detect slapd.service startup failure. * https://github.com/osstech-jp/openldap/pull/4 * https://patch-diff.githubusercontent.com/raw/osstech-jp/openldap/pull/4.pat… -- -- Name: SATOH Fumiyasu @ OSS Technology Corp. (fumiyas @ osstech co jp) -- Business Home: http://www.OSSTech.co.jp/ -- GitHub Home: https://GitHub.com/fumiyas/ -- PGP Fingerprint: BBE1 A1C9 525A 292E 6729 CDEC ADC2 9DCA 5E1C CBCA

1 0

(ITS#8708) SASL EXTERNAL binds and sasl-secprops minssf
by dhawes＠gmail.com 08 Aug '17

08 Aug '17

Full_Name: David Hawes Version: 2.4.45 OS: Linux URL: ftp://ftp.openldap.org/incoming/ Submission from: (NULL) (2001:468:c80:2103:0:523:da5e:da5e) With ITS #8568 [1], I notice that the first SASL EXTERNAL (using TLS client auth) bind on a connection succeeds, but subsequent SASL EXTERNAL binds on the same connection fail with: slapd[31088]: conn=1009 op=3 RESULT tag=97 err=48 text=SASL(-15): mechanism too weak for this user: mech EXTERNAL is too weak when: sasl-secprops minssf=128 In previous OpenLDAP versions, both the initial and subsequent SASL EXTERNAL binds succeed due to the bug in #8568. This was a misconfiguration on my part (I should have kept the default of 0), but I wonder if the initial SASL bind should also fail. It seems to succeed because tls_ssf is used in connection.c: slap_sasl_external( c, c->c_tls_ssf, &authid ); [1] http://www.openldap.org/its/index.cgi/Software%20Bugs?id=8568;selectid=8568

1 0

Re: (ITS#8707) slapd: Add systemd service notification support
by ryan＠nardis.ca 08 Aug '17

08 Aug '17

On Tue, Aug 08, 2017 at 03:10:35PM +0900, SATOH Fumiyasu wrote: >My slapd.service file: > >[Unit] >Description=OpenLDAP Server >After=syslog.target network-online.target >Documentation=man:slapd >Documentation=man:slapd.conf >Documentation=man:slapd-config >Documentation=man:slapd-bdb >Documentation=man:slapd-hdb >Documentation=man:slapd-mdb > >[Service] >Type=notify >ExecStart=%LIBEXECDIR%/slapd -d 0 -h "ldapi:/// ldap:///" > >[Install] >WantedBy=multi-user.target > >Any comment? No comment, looks good to me. :)

1 0

Re: (ITS#8707) slapd: Add systemd service notification support
by fumiyas＠osstech.co.jp 07 Aug '17

07 Aug '17

Hi, Sorry, I'm not good at English. :-) On Tue, 08 Aug 2017 05:12:02 +0900, Ryan Tandy wrote: > > I guess MAINPID= is actually needed, unless we run slapd with -d, regardless of > > whether we set Type=forking or Type=notify. Not exactly "needed", but better to > > have it than not. > > I experimented a bit with a service file. It seems to work well with either Type=forking and NotifyAccess=all, or Type=notify and ExecStart=slapd -d none. The latter (disabling forking) is definitely what systemd upstream recommends. I think so too. I don't like forking, thus I use "Type=notify" and "ExecStart=slapd -d0 ..." in my slapd.service file. > In either case, MAINPID= didn't actually seem to help anything. Yes. > NotifyAccess=main has a chicken-and-egg problem, because systemd needs to know the main PID in order for us to send it the message containing the PID! :) I think the only reasonable way to leave forking enabled would be to also require a PIDFile= setting, which solves that problem. But I'd rather sidestep the entire thing, omit MAINPID= as well, and > Looking at the systemctl output I still think STATUS= is redundant and could be omitted. > > So I guess my recommendation for the notify call boils down to: > > rc = sd_notify( 1, "READY=1" ); Agreed. I'll fix servers/slapd/daemon.c later. Thank you. > and a slapd.service along the lines of: > > [Unit] > Description=OpenLDAP server > > [Service] > Type=notify > ExecStart=%LIBEXECDIR%/slapd -h 'ldap:/// ldapi:///' -d0 > > [Install] > WantedBy=multi-user.target > > (basically identical to the example in systemd.service(5).) My slapd.service file: [Unit] Description=OpenLDAP Server After=syslog.target network-online.target Documentation=man:slapd Documentation=man:slapd.conf Documentation=man:slapd-config Documentation=man:slapd-bdb Documentation=man:slapd-hdb Documentation=man:slapd-mdb [Service] Type=notify ExecStart=%LIBEXECDIR%/slapd -d 0 -h "ldapi:/// ldap:///" [Install] WantedBy=multi-user.target Any comment? -- -- Name: SATOH Fumiyasu @ OSS Technology Corp. (fumiyas @ osstech co jp) -- Business Home: http://www.OSSTech.co.jp/ -- GitHub Home: https://GitHub.com/fumiyas/ -- PGP Fingerprint: BBE1 A1C9 525A 292E 6729 CDEC ADC2 9DCA 5E1C CBCA

1 0

Re: (ITS#8707) slapd: Add systemd service notification support
by ryan＠nardis.ca 07 Aug '17

07 Aug '17

On Mon, Aug 07, 2017 at 01:12:02PM -0700, Ryan Tandy wrote: >Side note: the version message from slapd appears in the journal >twice, once with the timestamp and once without... not sure exactly >why! Sorry, meant to delete this paragraph before sending. The difference between "-dnone" and "-d0" is precisely an extra log to stderr :)

1 0

Re: (ITS#8707) slapd: Add systemd service notification support
by ryan＠nardis.ca 07 Aug '17

07 Aug '17

On Mon, Aug 07, 2017 at 12:23:27PM -0700, Ryan Tandy wrote: >I guess MAINPID= is actually needed, unless we run slapd with -d, regardless of >whether we set Type=forking or Type=notify. Not exactly "needed", but better to >have it than not. I experimented a bit with a service file. It seems to work well with either Type=forking and NotifyAccess=all, or Type=notify and ExecStart=slapd -d none. The latter (disabling forking) is definitely what systemd upstream recommends. In either case, MAINPID= didn't actually seem to help anything. NotifyAccess=main has a chicken-and-egg problem, because systemd needs to know the main PID in order for us to send it the message containing the PID! :) I think the only reasonable way to leave forking enabled would be to also require a PIDFile= setting, which solves that problem. But I'd rather sidestep the entire thing, omit MAINPID= as well, and Looking at the systemctl output I still think STATUS= is redundant and could be omitted. So I guess my recommendation for the notify call boils down to: rc = sd_notify( 1, "READY=1" ); and a slapd.service along the lines of: [Unit] Description=OpenLDAP server [Service] Type=notify ExecStart=%LIBEXECDIR%/slapd -h 'ldap:/// ldapi:///' -d0 [Install] WantedBy=multi-user.target (basically identical to the example in systemd.service(5).) Side note: the version message from slapd appears in the journal twice, once with the timestamp and once without... not sure exactly why!

1 0

Re: (ITS#8707) slapd: Add systemd service notification support
by ryan＠nardis.ca 07 Aug '17

07 Aug '17

On Mon, Aug 07, 2017 at 01:30:44PM +0000, fumiyas(a)osstech.co.jp wrote: >I've created patches for slapd to add systemd service notification >(sd_notify(3)) support. Thanks for working on this! I'm also interested in having this feature. What do you think about including a slapd.service file? I know OpenLDAP has traditionally not included an init script, but systemd units are intended to be distro-agnostic as far as possible, and shipped by upstream projects in most cases, unlike init scripts. Ideally I'd like to include a template in the source and have the build system fill in the autoconf'ed path (i.e. to slapd) and have 'make install' install it to the right place. Autoconf bits look fine to me. >+ rc = sd_notifyf( 1, >+ "READY=1\n" >+ "STATUS=slapd: ready to serve connections...\n" >+ "MAINPID=%lu", >+ (unsigned long) getpid() ); unset_environment=1 seems reasonable, it's a little unfortunate that we can't call (for example) sd_notify("STOPPING=1") afterward, but it feels worthwhile compared to having to sanitize the environment when forking a child process. I'm not sure the STATUS= message adds value compared to just the basic readiness notification; can you comment on why you included it? I guess MAINPID= is actually needed, unless we run slapd with -d, regardless of whether we set Type=forking or Type=notify. Not exactly "needed", but better to have it than not. I see you've placed this call later than the parent's exit point. Any comments about the timing of this relative to the parent's exit, and to the listener startup and so on? Are the listeners more likely to be ready to serve connections at this point? I seem to recall that in the past there was opposition to moving the parent's exit later, but I can't remember why. (and I still wish we could do that, and dispense with the ldapsearch-loop hacks...) cheers, Ryan

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

openldap-bugs