(ITS#6139) slapd: password-hash rejects multiple values
by fumiyas@osstech.co.jp
Full_Name: SATOH Fumiyasu
Version: 2.4.16
OS: Linux
URL: ftp://ftp.openldap.org/incoming/
Submission from: (NULL) (115.176.213.88)
My /etc/openldap/slapd.conf has the following line:
password-hash {CRYPT} {SSHA}
On OpenLDAP 2.4.16, slapd (slaptest) rejects this config:
# /usr/sbin/slaptest
/etc/openldap/slapd.conf: line 36: <password-hash> extra cruft after <hash>.
slaptest: bad configuration file!
On OpenLDAP 2.3.43, slapd (slaptest) accept this config.
The slapd.conf(5) (included in OpenLDAP 2.3.43/2.4.16) manpage said:
password-hash <hash> [<hash>...]
This option configures one or more hashes to
be used in generation of user passwords
stored in the userPassword attribute during
processing of LDAP Password Modify Extended
Operations (RFC 3062).
14 years, 6 months
Re: (ITS#6104) race condition with cancel operation
by h.b.furuseth@usit.uio.no
hyc(a)symas.com writes:
>> Bitflags should be unnecessary as such, just need a set of values.
>> Though it wouldn't hurt if they were chosen so one could use bit
>> operations to reduce checks for "o_abandon == A or B". "Suppress
>> response" value mentioned in ITS#6138 also needed, maybe that must be a
>> bitflag when combined with Cancel stuff.
>
> Yes, it still seems some bitflags will be more appropriate.
As long as we decide which flag combinations are (not) valid. Problem
with "unrestricted" bitflags compared to a list of values is that they
allow and thus require support for flag combinations we have no real
need to support.
>> Question:
>> syncprov_op_abandon() holds&si->si_ops_mutex when setting
>> so->s_op->o_abandon. Does it need that? The function needs to
>> grab op->o_conn->c_mutex when called as Cancel, but must not do
>> that while holding&si->si_ops_mutex since Abandon grabs the
>> locks in the opposite order.
>
> syncprov_op_abandon() can remove ops from the list, so yes, it must
> hold si_ops_mutex.
Right, but does it need si_ops_mutex while setting o_abandon? Cancel
could remove the op, unlock si_ops_mutex, lock c_mutex, update
o_abandon. Alternative: Lock c_mutex first (that must be safe since
Abandon is doing it anyway), then lock si_ops_mutex. So it's not
a critical difference, just that a lock must be held a bit longer.
--
Hallvard
14 years, 6 months
Re: (ITS#6104) race condition with cancel operation
by hyc@symas.com
h.b.furuseth(a)usit.uio.no wrote:
> Well, this turned out a lot of problems. Currently blocked by
> ITS#6138 Bad Cancel/Abandon/"internal abandon"/Syncprov interactions.
>
> back-relay/op.c done (ITS#6133). syncprov/retcode issues may fit better
> under ITS#6138.
>
> Bitflags should be unnecessary as such, just need a set of values.
> Though it wouldn't hurt if they were chosen so one could use bit
> operations to reduce checks for "o_abandon == A or B". "Suppress
> response" value mentioned in ITS#6138 also needed, maybe that must be a
> bitflag when combined with Cancel stuff.
Yes, it still seems some bitflags will be more appropriate.
> Some tidbits uglifying a fix:
> * op->o_conn->c_mutex is locked when be->be_abandon() is called,
> but not when be->be_cancel() is called.
> * send_ldap_ber() needs the lock while it holds&conn->c_write1_mutex,
> but code elsewhere indicates the reverse lock order.
>
> Question:
> syncprov_op_abandon() holds&si->si_ops_mutex when setting
> so->s_op->o_abandon. Does it need that? The function needs to
> grab op->o_conn->c_mutex when called as Cancel, but must not do
> that while holding&si->si_ops_mutex since Abandon grabs the
> locks in the opposite order.
syncprov_op_abandon() can remove ops from the list, so yes, it must hold
si_ops_mutex.
--
-- Howard Chu
CTO, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc/
Chief Architect, OpenLDAP http://www.openldap.org/project/
14 years, 6 months
Re: (ITS#6104) race condition with cancel operation
by h.b.furuseth@usit.uio.no
Well, this turned out a lot of problems. Currently blocked by
ITS#6138 Bad Cancel/Abandon/"internal abandon"/Syncprov interactions.
back-relay/op.c done (ITS#6133). syncprov/retcode issues may fit better
under ITS#6138.
Bitflags should be unnecessary as such, just need a set of values.
Though it wouldn't hurt if they were chosen so one could use bit
operations to reduce checks for "o_abandon == A or B". "Suppress
response" value mentioned in ITS#6138 also needed, maybe that must be a
bitflag when combined with Cancel stuff.
Some tidbits uglifying a fix:
* op->o_conn->c_mutex is locked when be->be_abandon() is called,
but not when be->be_cancel() is called.
* send_ldap_ber() needs the lock while it holds &conn->c_write1_mutex,
but code elsewhere indicates the reverse lock order.
Question:
syncprov_op_abandon() holds &si->si_ops_mutex when setting
so->s_op->o_abandon. Does it need that? The function needs to
grab op->o_conn->c_mutex when called as Cancel, but must not do
that while holding &si->si_ops_mutex since Abandon grabs the
locks in the opposite order.
--
Hallvard
14 years, 6 months
(ITS#6138) Bad Cancel/Abandon/"internal abandon"/Syncprov interactions
by h.b.furuseth@usit.uio.no
Full_Name: Hallvard B Furuseth
Version: HEAD
OS:
URL:
Submission from: (NULL) (129.240.6.233)
Submitted by: hallvard
Might make this a tracking issue for Cancel/Abandon problems.
And/or copy some to separate ITSes, but they all seem interconnected:
Cancel(abandoned operation)-requests are not rejected. Thus slapd
sets o_cancel and turns an active Abandon into a Cancel. Presumably
that can confuse Cancel/Abandon handlers, like that in Syncprov.
Similarly, Abandon(abandoned/cancelled operation) is not ignored.
connection_abandon() re-abandons abandoned/cancelled operations too.
However Syncprov:RefreshAndPersist abuses op->o_abandon: It sets it to
mean "Suppress the response. A copy of this operation will send it."
So if Cancel(op with o_abandon!=0) is fixed to respond protocolError
"already abandoned", presumably that breaks Cancel(RefreshAndPersist).
I'm not touching it with a flagpole - I don't know syncprov. Help?
Overlay retcode does something similar - sends a response and then sets
o_abandon.
Cancel/Abandon can in any case fail by targeting the wrong operation
though: A connection can have multiple messages with the same IDs when
the response is sent and the client reuses the message ID, before the
old operation in slapd can clean up and finish.
syncprov_op_abandon() identifies messages by (connid, msgid). Can
multiple messages with the same ID break that, or more importantly,
break what gets sent/written with syncrepl?
Anyway, the current code needs an o_abandon value which means "suppress
response". Or maybe "abandoned, except as far as future Abandon and
Cancel operations are concerned". Syncprov and Retcode need to handle
the various possible orderings of Cancel/Abandon vs Suppress, including
when they forward o_abandon from one operation to another. I haven't
looked too closely at that code either.
ITS#6104 (race condition with cancel operation) also calls for multiple
o_abandon values (joining some or all of o_cancel into o_abandon), and
must be considered when picking a value. However I suggest to consider
just the various possible values first and get that right, and deal
with the race condition later.
Source and binary compatibility with third-party modules complicates
this, if we're trying to support both and old compiled slapd + new
compiled module and vice versa. Might try a minimal solution now.
14 years, 6 months
Re: (ITS#6137) Cancel(pending operation) acts like Abandon
by h.b.furuseth@usit.uio.no
Confirmed & fixed. Responds "too busy for Cancel, try Abandon instead".
If someone wants to, they could improve that. As noted in the commit:
/* TODO: We could instead remove the cancelled operation
* from c_pending_ops like Abandon does, and send its
* response here. Not if it is pending because of a
* congested connection though.
*/
--
Hallvard
14 years, 6 months
(ITS#6137) Cancel(pending operation) acts like Abandon
by h.b.furuseth@usit.uio.no
Full_Name: Hallvard B Furuseth
Version: HEAD
OS:
URL:
Submission from: (NULL) (129.240.6.233)
Submitted by: hallvard
Looking at the code (not sure how to reproduce):
Cancel(pending operation) abandons the operation, without responding to
either Cancel or the cancelled operation. That code in slapd/cancel.c
is copied from the equivalent abandon.c code.
Cancel as currently written must wait for the cancelled operation to
complete. But an active operation must not wait for a pending one,
all threads could block. For now we can make Cancel respond with
LDAP_CANNOT_CANCEL: "can't handle Cancel now, try Abandon".
Repeating myself, a better fix (RE25?) would be to make the cancelled
operation send the Cancel response. Don't how deep surgery that'd need
to slapd.
14 years, 6 months
Re: (ITS#5328) backends sending/setting results
by masarati@aero.polimi.it
h.b.furuseth(a)usit.uio.no wrote:
> Fixed relay_back_has_subordinates().
> I've lost track of what I thought to do with back-ldap.
I think you should just ignore it, otherwise all you could do is to to
perform a onelevel search for each entry (!) that requests the
attribute. We (dunno if it's documented) do not guarantee
hasSubordinates is correct, all in all.
p.
14 years, 6 months
(ITS#6136) replicated slapd.d interrupts syncrepl refresh
by mbackes@symas.com
Full_Name: Matthew Backes
Version: 2.4, probably head?
OS: any
URL:
Submission from: (NULL) (12.49.250.67)
If you modify an object in cn=config in a multi-master environment
with replicated cn=config, other databases on replicating
other-masters in the refresh-state will be interrupted when they
receive the change.
Applying any change to their local cn=config resumes the syncrepl
refresh operation.
14 years, 6 months