Re: opensmtpd / ldap unreliable
On Sat, May 26, 2018 at 08:16:28AM +0200, Gilles Chehade wrote: > please do so we have more people able to test Done, thanks. What are your thoughts design-wise on dealing with ldap not being available at startup? Should layer 7 issues (ldap auth failed, etc) be handled differently than transport level issues (connection refused/timed out)?
Re: opensmtpd / ldap unreliable
On Thu, May 24, 2018 at 11:45:40AM -0700, Paul B. Henson wrote: > > From: Gilles Chehade > > Sent: Wednesday, May 23, 2018 1:20 PM > > > > That's bad but could easily be fixed if you want to help us > > So I dropped in the latest table-ldap from git, and it still failed > authentications after an LDAP server outage. It looks like the check is only > in the table_ldap_check function? I'm not sure what that's for, but it > doesn't seem to be called at all when doing authentication. I added a > similar check into the table_ldap_lookup function, and also had to reorder > the functions in the file a bit due to errors like this: > > table_ldap.c:92:15: warning: implicit declaration of function 'ldap_open' is > invalid in C99 > [-Wimplicit-function-declaration] > > Afterwards, opensmtpd successfully reconnected to LDAP and performed > authentication after an LDAP outage :). > > users[14726]: debug: table_ldap: ldap_query: > filter=(&(objectClass=uidObject)(uid=henson)), ret=0 > users[14726]: debug: table-ldap: reconnecting > users[14726]: info: table-ldap: closed previous connection > users[14726]: debug: ldap server accepted credentials > users[14726]: debug: table_ldap: ldap_query: > filter=(&(objectClass=uidObject)(uid=henson)), ret=1 > > > Here's what my changes currently are. I can submit a pull request on github > if you'd like. Thanks. > please do so we have more people able to test I'll review shortly > diff --git a/extras/tables/table-ldap/table_ldap.c > b/extras/tables/table-ldap/table_ldap.c > index 88c9ffd..9d20526 100644 > --- a/extras/tables/table-ldap/table_ldap.c > +++ b/extras/tables/table-ldap/table_ldap.c > @@ -74,45 +74,6 @@ table_ldap_update(void) > return 1; > } > > -static int > -table_ldap_check(int service, struct dict *params, const char *key) > -{ > - int ret; > - > - switch(service) { > - case K_ALIAS: > - case K_DOMAIN: > - case K_CREDENTIALS: > - case K_USERINFO: > - case K_MAILADDR: > - if ((ret = ldap_run_query(service, key, NULL, 0)) >= 0) { > - return ret; > - } > - log_debug("debug: table-ldap: reconnecting"); > - if (!(ret = ldap_open())) { > - log_warnx("warn: table-ldap: failed to connect"); > - } > - return ret; > - default: > - return -1; > - } > -} > - > -static int > -table_ldap_lookup(int service, struct dict *params, const char *key, char > *dst, size_t sz) > -{ > - switch(service) { > - case K_ALIAS: > - case K_DOMAIN: > - case K_CREDENTIALS: > - case K_USERINFO: > - case K_MAILADDR: > - return ldap_run_query(service, key, dst, sz); > - default: > - return -1; > - } > -} > - > static int > table_ldap_fetch(int service, struct dict *params, char *dst, size_t sz) > { > @@ -361,6 +322,32 @@ err: > return 0; > } > > +static int > +table_ldap_lookup(int service, struct dict *params, const char *key, char > *dst, size_t sz) > +{ > + int ret; > + > + switch(service) { > + case K_ALIAS: > + case K_DOMAIN: > + case K_CREDENTIALS: > + case K_USERINFO: > + case K_MAILADDR: > + if ((ret = ldap_run_query(service, key, dst, sz)) > 0) { > + return ret; > + } > + log_debug("debug: table-ldap: reconnecting"); > + if (!(ret = ldap_open())) { > + log_warnx("warn: table-ldap: failed to connect"); > + return ret; > + } > + return ldap_run_query(service, key, dst, sz); > + default: > + return -1; > + } > +} > + > + > static int > ldap_query(const char *filter, char **attributes, char ***outp, size_t n) > { > @@ -498,6 +485,31 @@ end: > return ret; > } > > +static int > +table_ldap_check(int service, struct dict *params, const char *key) > +{ > + int ret; > + > + switch(service) { > + case K_ALIAS: > + case K_DOMAIN: > + case K_CREDENTIALS: > + case K_USERINFO: > + case K_MAILADDR: > + if ((ret = ldap_run_query(service, key, NULL, 0)) >= 0) { > + return ret; > + } > + log_debug("debug: table-ldap: reconnecting"); > + if (!(ret = ldap_open())) { > + log_warnx("warn: table-ldap: failed to connect"); > + } > + return ret; > + default: > + return -1; > + } > +} > + > + > int > main(int argc, char **argv) > { > > -- Gilles Chehade https://www.poolp.org @poolpOrg
Re: opensmtpd / ldap unreliable
Hello, Thanks for this patch! I'm setting up a similar configuration. I'll have a test also. Regards. Christophe Le 05/24/18 à 20:45, Paul B. Henson a écrit : From: Gilles Chehade Sent: Wednesday, May 23, 2018 1:20 PM That's bad but could easily be fixed if you want to help us So I dropped in the latest table-ldap from git, and it still failed authentications after an LDAP server outage. It looks like the check is only in the table_ldap_check function? I'm not sure what that's for, but it doesn't seem to be called at all when doing authentication. I added a similar check into the table_ldap_lookup function, and also had to reorder the functions in the file a bit due to errors like this: table_ldap.c:92:15: warning: implicit declaration of function 'ldap_open' is invalid in C99 [-Wimplicit-function-declaration] Afterwards, opensmtpd successfully reconnected to LDAP and performed authentication after an LDAP outage :). users[14726]: debug: table_ldap: ldap_query: filter=(&(objectClass=uidObject)(uid=henson)), ret=0 users[14726]: debug: table-ldap: reconnecting users[14726]: info: table-ldap: closed previous connection users[14726]: debug: ldap server accepted credentials users[14726]: debug: table_ldap: ldap_query: filter=(&(objectClass=uidObject)(uid=henson)), ret=1 Here's what my changes currently are. I can submit a pull request on github if you'd like. Thanks. diff --git a/extras/tables/table-ldap/table_ldap.c b/extras/tables/table-ldap/table_ldap.c index 88c9ffd..9d20526 100644 --- a/extras/tables/table-ldap/table_ldap.c +++ b/extras/tables/table-ldap/table_ldap.c @@ -74,45 +74,6 @@ table_ldap_update(void) return 1; } -static int -table_ldap_check(int service, struct dict *params, const char *key) -{ - int ret; - - switch(service) { - case K_ALIAS: - case K_DOMAIN: - case K_CREDENTIALS: - case K_USERINFO: - case K_MAILADDR: - if ((ret = ldap_run_query(service, key, NULL, 0)) >= 0) { - return ret; - } - log_debug("debug: table-ldap: reconnecting"); - if (!(ret = ldap_open())) { - log_warnx("warn: table-ldap: failed to connect"); - } - return ret; - default: - return -1; - } -} - -static int -table_ldap_lookup(int service, struct dict *params, const char *key, char *dst, size_t sz) -{ - switch(service) { - case K_ALIAS: - case K_DOMAIN: - case K_CREDENTIALS: - case K_USERINFO: - case K_MAILADDR: - return ldap_run_query(service, key, dst, sz); - default: - return -1; - } -} - static int table_ldap_fetch(int service, struct dict *params, char *dst, size_t sz) { @@ -361,6 +322,32 @@ err: return 0; } +static int +table_ldap_lookup(int service, struct dict *params, const char *key, char *dst, size_t sz) +{ + int ret; + + switch(service) { + case K_ALIAS: + case K_DOMAIN: + case K_CREDENTIALS: + case K_USERINFO: + case K_MAILADDR: + if ((ret = ldap_run_query(service, key, dst, sz)) > 0) { + return ret; + } + log_debug("debug: table-ldap: reconnecting"); + if (!(ret = ldap_open())) { + log_warnx("warn: table-ldap: failed to connect"); + return ret; + } + return ldap_run_query(service, key, dst, sz); + default: + return -1; + } +} + + static int ldap_query(const char *filter, char **attributes, char ***outp, size_t n) { @@ -498,6 +485,31 @@ end: return ret; } +static int +table_ldap_check(int service, struct dict *params, const char *key) +{ + int ret; + + switch(service) { + case K_ALIAS: + case K_DOMAIN: + case K_CREDENTIALS: + case K_USERINFO: + case K_MAILADDR: + if ((ret = ldap_run_query(service, key, NULL, 0)) >= 0) { + return ret; + } + log_debug("debug: table-ldap: reconnecting"); + if (!(ret = ldap_open())) { + log_warnx("warn: table-ldap: failed to connect"); + } + return ret; + default: + return -1; + } +} + + int main(int argc, char **argv) {
Re: opensmtpd / ldap unreliable
> From: Gilles Chehade > Sent: Wednesday, May 23, 2018 1:20 PM > > That's bad but could easily be fixed if you want to help us So I dropped in the latest table-ldap from git, and it still failed authentications after an LDAP server outage. It looks like the check is only in the table_ldap_check function? I'm not sure what that's for, but it doesn't seem to be called at all when doing authentication. I added a similar check into the table_ldap_lookup function, and also had to reorder the functions in the file a bit due to errors like this: table_ldap.c:92:15: warning: implicit declaration of function 'ldap_open' is invalid in C99 [-Wimplicit-function-declaration] Afterwards, opensmtpd successfully reconnected to LDAP and performed authentication after an LDAP outage :). users[14726]: debug: table_ldap: ldap_query: filter=(&(objectClass=uidObject)(uid=henson)), ret=0 users[14726]: debug: table-ldap: reconnecting users[14726]: info: table-ldap: closed previous connection users[14726]: debug: ldap server accepted credentials users[14726]: debug: table_ldap: ldap_query: filter=(&(objectClass=uidObject)(uid=henson)), ret=1 Here's what my changes currently are. I can submit a pull request on github if you'd like. Thanks. diff --git a/extras/tables/table-ldap/table_ldap.c b/extras/tables/table-ldap/table_ldap.c index 88c9ffd..9d20526 100644 --- a/extras/tables/table-ldap/table_ldap.c +++ b/extras/tables/table-ldap/table_ldap.c @@ -74,45 +74,6 @@ table_ldap_update(void) return 1; } -static int -table_ldap_check(int service, struct dict *params, const char *key) -{ - int ret; - - switch(service) { - case K_ALIAS: - case K_DOMAIN: - case K_CREDENTIALS: - case K_USERINFO: - case K_MAILADDR: - if ((ret = ldap_run_query(service, key, NULL, 0)) >= 0) { - return ret; - } - log_debug("debug: table-ldap: reconnecting"); - if (!(ret = ldap_open())) { - log_warnx("warn: table-ldap: failed to connect"); - } - return ret; - default: - return -1; - } -} - -static int -table_ldap_lookup(int service, struct dict *params, const char *key, char *dst, size_t sz) -{ - switch(service) { - case K_ALIAS: - case K_DOMAIN: - case K_CREDENTIALS: - case K_USERINFO: - case K_MAILADDR: - return ldap_run_query(service, key, dst, sz); - default: - return -1; - } -} - static int table_ldap_fetch(int service, struct dict *params, char *dst, size_t sz) { @@ -361,6 +322,32 @@ err: return 0; } +static int +table_ldap_lookup(int service, struct dict *params, const char *key, char *dst, size_t sz) +{ + int ret; + + switch(service) { + case K_ALIAS: + case K_DOMAIN: + case K_CREDENTIALS: + case K_USERINFO: + case K_MAILADDR: + if ((ret = ldap_run_query(service, key, dst, sz)) > 0) { + return ret; + } + log_debug("debug: table-ldap: reconnecting"); + if (!(ret = ldap_open())) { + log_warnx("warn: table-ldap: failed to connect"); + return ret; + } + return ldap_run_query(service, key, dst, sz); + default: + return -1; + } +} + + static int ldap_query(const char *filter, char **attributes, char ***outp, size_t n) { @@ -498,6 +485,31 @@ end: return ret; } +static int +table_ldap_check(int service, struct dict *params, const char *key) +{ + int ret; + + switch(service) { + case K_ALIAS: + case K_DOMAIN: + case K_CREDENTIALS: + case K_USERINFO: + case K_MAILADDR: + if ((ret = ldap_run_query(service, key, NULL, 0)) >= 0) { + return ret; + } + log_debug("debug: table-ldap: reconnecting"); + if (!(ret = ldap_open())) { + log_warnx("warn: table-ldap: failed to connect"); + } + return ret; + default: + return -1; + } +} + + int main(int argc, char **argv) {
Re: opensmtpd / ldap unreliable
On 17:20 Wed 23 May, Allan Streib wrote: > "Paul B. Henson" writes: > > >> What you ask is a very general question: If A depends on B, and B is > >> missing, how do expect A to behave? > > > > In this specific case, I expect A to complain it was unable to contact > > B, to continue initializing, return temporary failures for any > > operation which requires B, and reattempt a connection to B on a > > regular basis until it is successful. From a reliability and full > > tolerance perspective, falling over and dying doesn't seem a very good > > choice for the circumstances. > > Falling over and dying is the simplest thing. It makes no assumptions > about the cause of the problem and when it might be resolved. It does > not attempt to carry on in some hobbled fashion, possibly creating > further problems. > > If you depend on services being up, you will need monitors/supervisors > to detect when they are not up, and attempt restarts and/or notify you > as appropriate. Baking this into the services themselves is a > duplication of functionality that can be handled externally. > > Allan So... your web browser crashes when there is no network, right?
Re: opensmtpd / ldap unreliable
> From: Gilles Chehade > Sent: Wednesday, May 23, 2018 1:20 PM > > That's bad but could easily be fixed if you want to help us Definitely; I'll pull the latest github head down and see if that fixes the LDAP connection recovery after startup issue, and then I can try any suggestions to make it more reliable at startup or possibly fiddle with that code myself. > That would be a bad idea... it's experimental :-p I did see that mentioned circa 2013, but I guess I kind of hoped it had moved beyond that by now :). Thanks much.
Re: opensmtpd / ldap unreliable
"Paul B. Henson" writes: >> What you ask is a very general question: If A depends on B, and B is >> missing, how do expect A to behave? > > In this specific case, I expect A to complain it was unable to contact > B, to continue initializing, return temporary failures for any > operation which requires B, and reattempt a connection to B on a > regular basis until it is successful. From a reliability and full > tolerance perspective, falling over and dying doesn't seem a very good > choice for the circumstances. Falling over and dying is the simplest thing. It makes no assumptions about the cause of the problem and when it might be resolved. It does not attempt to carry on in some hobbled fashion, possibly creating further problems. If you depend on services being up, you will need monitors/supervisors to detect when they are not up, and attempt restarts and/or notify you as appropriate. Baking this into the services themselves is a duplication of functionality that can be handled externally. Allan
Re: opensmtpd / ldap unreliable
On Wed, May 23, 2018 at 10:19:47PM +0200, Gilles Chehade wrote: > On Tue, May 22, 2018 at 06:13:23PM -0700, Paul B. Henson wrote: > > So I recently converted my opensmtpd server to use ldap as the backend > > for user authentication. It seems it's a bit untolerant to ldap issues? > > > > [...] > > Just to clarify, the "extras" are add-ons which we believe not to belong in the smtpd code base, so the amount of efforts we pour on them is very dependant on the interest of developers and the interest the community's showing for the add-on, it's a community and volunteer driven effort. None of the add-ons are part of the opensmtpd roadmap, some became quite popular like table-sqlite or table-passwd and are well maintained, other have very few users who aren't pushing much for improvement, so the code doesn't evolve much and/or we are not aware of shortcomings. If you want proper ldap support, become active and it will happen :-)x -- Gilles Chehade https://www.poolp.org @poolpOrg
Re: opensmtpd / ldap unreliable
On Tue, May 22, 2018 at 06:13:23PM -0700, Paul B. Henson wrote: > So I recently converted my opensmtpd server to use ldap as the backend > for user authentication. It seems it's a bit untolerant to ldap issues? > yes most likely If the ldap server isn't available when opensmtpd is started, it says it > started: > > # /etc/rc.d/smtpd start > smtpd(ok) > > But it isn't there: > > # ps -aux | grep smtpd > root 89090 0.0 0.0 304 1208 p6 S+p5:52PM0:00.00 grep smtpd > > And it's not really obvious why: > > May 22 17:52:51 bart smtpd[46044]: info: OpenSMTPD 6.0.4 starting > May 22 17:52:51 bart smtpd[23325]: warn: table-proc: pipe closed > May 22 17:52:51 bart smtpd[23325]: lookup: table-proc: exiting > May 22 17:52:51 bart smtpd[73239]: smtpd: process lka socket closed > not good > Starting in debug mode: > > # smtpd -d > info: OpenSMTPD 6.0.4 starting > users[43283]: debug: reading key "url" -> "ldap://localhost:3389"; > users[43283]: debug: reading key "basedn" -> > users[43283]: debug: reading key "username" -> > users[43283]: debug: reading key "password" -> > users[43283]: debug: reading key "credentials_filter" -> > "(&(objectClass=uidObject)(uid=%s))" > users[43283]: debug: parsing attribute "credentials_attributes" (2) -> > "uid,description" > users[43283]: debug: done reading config > users[43283]: warn: aldap_parse > users[43283]: fatal: failed to connect > warn: table-proc: pipe closed > lookup: table-proc: exiting > smtpd: process lka socket closed > > You can see it looks like it fails to connect to the ldap server at > startup and just dies. > > Further, if the ldap server is up at startup, but ever restarts or has > the connection broken, authentication just fails: > > May 21 13:22:10 bart smtpd[42132]: warn: user credentials lookup fail for > users:henson > > The opensmtpd process needs to be restarted before authentication works > again. > not good > In debug mode, it shows: > > users[7295]: debug: table_ldap: ldap_query: > filter=(&(objectClass=uidObject)(uid=henson)), ret=0 > 5e46e2fabbf8d72e smtp event=authentication user=henson > address=134.71.249.41 host=134.71.249.41 result=permfail > > Is it expected that the ldap support is currently not production ready? > I see in a presentation from back in 2013 that ldap was classified > experimental at the time, but it's not clear if that's still the case. > Yes, sadly I wrote the initial ldap support but I don't use ldap myself and I could not get any user to spend time with me testing related diffs more than a couple times, so... > I see in the repo at > > https://github.com/OpenSMTPD/OpenSMTPD-extras/blob/master/extras/tables/table-ldap/table_ldap.c > > there's a change to add ldap reconnection support: > > https://github.com/OpenSMTPD/OpenSMTPD-extras/commit/04e4c521b34d1987af915ff97dcb0d87daf122b0#diff-369c0fcbfbc85bf2cdad7dba1131b872 > > but it's dated 7/27/2017, and the last github release seems to be > 201601072302 (although the openbsd port appears to be 201703132115, I > guess it's not downloading it from github?). > It's been a while since the last -extras release indeed, I suppose the openbsd port pulls from github, I dunno really > It looks like the code in head still fails to start if the ldap server > isn't available when opensmtpd is started though. > That's bad but could easily be fixed if you want to help us > Is anybody using opensmtpd with ldap in production? If so, how are you > working around this issue? > That would be a bad idea... it's experimental :-p -- Gilles Chehade https://www.poolp.org @poolpOrg
Re: opensmtpd / ldap unreliable
> From: justina colmena > Sent: Tuesday, May 22, 2018 9:08 PM > > Are they being started in the wrong order at boot time? The LDAP server in use is not running on the local openBSD system. It might not be available due to an underlying network issue or some other problem that temporarily prevents successful connections/queries. > What you ask is a very general question: If A depends on B, and B is > missing, how do expect A to behave? In this specific case, I expect A to complain it was unable to contact B, to continue initializing, return temporary failures for any operation which requires B, and reattempt a connection to B on a regular basis until it is successful. From a reliability and full tolerance perspective, falling over and dying doesn't seem a very good choice for the circumstances.
Re: opensmtpd / ldap unreliable
On Tue, 22 May 2018 18:13:23 -0700 "Paul B. Henson" wrote: > If the ldap server isn't available when opensmtpd is started, it says > it started: > > # /etc/rc.d/smtpd start Then apparently you should have done # /etc/rc.d/ldapd start or preferably # rcctl start ldapd first. Are you enabling them both at boot time? # rcctl enable ldapd # rcctl enable smtpd Are they being started in the wrong order at boot time? Otherwise, there may well be arguably a certain reliability or error reporting issue in the code for it, but there is a certain bottom line here in that if you configure ASDF to _require_ JKL for authentication, then you really do need to have JKL "available" if you expect ASDF to work the way you configured it. What you ask is a very general question: If A depends on B, and B is missing, how do expect A to behave? Anyways, smtpd is a daemon. It was forked off, and it was still running when it was checked immediately after you issued the command "/etc/rc.d/smtpd start". Its parent process had no doubt already terminated and control had reverted to the shell before the fatal error occurred. You would have to issue the command # rcctl check smtpd to check that smtpd is still running at some later time.
opensmtpd / ldap unreliable
So I recently converted my opensmtpd server to use ldap as the backend for user authentication. It seems it's a bit untolerant to ldap issues? If the ldap server isn't available when opensmtpd is started, it says it started: # /etc/rc.d/smtpd start smtpd(ok) But it isn't there: # ps -aux | grep smtpd root 89090 0.0 0.0 304 1208 p6 S+p5:52PM0:00.00 grep smtpd And it's not really obvious why: May 22 17:52:51 bart smtpd[46044]: info: OpenSMTPD 6.0.4 starting May 22 17:52:51 bart smtpd[23325]: warn: table-proc: pipe closed May 22 17:52:51 bart smtpd[23325]: lookup: table-proc: exiting May 22 17:52:51 bart smtpd[73239]: smtpd: process lka socket closed Starting in debug mode: # smtpd -d info: OpenSMTPD 6.0.4 starting users[43283]: debug: reading key "url" -> "ldap://localhost:3389"; users[43283]: debug: reading key "basedn" -> users[43283]: debug: reading key "username" -> users[43283]: debug: reading key "password" -> users[43283]: debug: reading key "credentials_filter" -> "(&(objectClass=uidObject)(uid=%s))" users[43283]: debug: parsing attribute "credentials_attributes" (2) -> "uid,description" users[43283]: debug: done reading config users[43283]: warn: aldap_parse users[43283]: fatal: failed to connect warn: table-proc: pipe closed lookup: table-proc: exiting smtpd: process lka socket closed You can see it looks like it fails to connect to the ldap server at startup and just dies. Further, if the ldap server is up at startup, but ever restarts or has the connection broken, authentication just fails: May 21 13:22:10 bart smtpd[42132]: warn: user credentials lookup fail for users:henson The opensmtpd process needs to be restarted before authentication works again. In debug mode, it shows: users[7295]: debug: table_ldap: ldap_query: filter=(&(objectClass=uidObject)(uid=henson)), ret=0 5e46e2fabbf8d72e smtp event=authentication user=henson address=134.71.249.41 host=134.71.249.41 result=permfail Is it expected that the ldap support is currently not production ready? I see in a presentation from back in 2013 that ldap was classified experimental at the time, but it's not clear if that's still the case. I see in the repo at https://github.com/OpenSMTPD/OpenSMTPD-extras/blob/master/extras/tables/table-ldap/table_ldap.c there's a change to add ldap reconnection support: https://github.com/OpenSMTPD/OpenSMTPD-extras/commit/04e4c521b34d1987af915ff97dcb0d87daf122b0#diff-369c0fcbfbc85bf2cdad7dba1131b872 but it's dated 7/27/2017, and the last github release seems to be 201601072302 (although the openbsd port appears to be 201703132115, I guess it's not downloading it from github?). It looks like the code in head still fails to start if the ldap server isn't available when opensmtpd is started though. Is anybody using opensmtpd with ldap in production? If so, how are you working around this issue? Thanks...