OpenLDAP
Up to top level
Build   Contrib   Development   Documentation   Historical   Incoming   Software Bugs   Software Enhancements   Web  

Logged in as guest

Viewing Software Bugs/6103
Full headers

From: h.b.furuseth@usit.uio.no
Subject: canceled operations do not respond
Compose comment
Download message
State:
0 replies:
6 followups: 1 2 3 4 5 6

Major security issue: yes  no

Notes:

Notification:


Date: Sat, 09 May 2009 22:07:43 +0000
From: h.b.furuseth@usit.uio.no
To: openldap-its@OpenLDAP.org
Subject: canceled operations do not respond
Full_Name: Hallvard B Furuseth
Version: HEAD
OS: Linux
URL: 
Submission from: (NULL) (129.240.6.233)
Submitted by: hallvard


RFC 3909 says both the Cancel operation and the canceled operation
shall respond.  However I'm getting tooLate reply from Cancel
and no response from the successfully cancelled operation:

slapd.conf:
  include   servers/slapd/schema/core.schema
  database  null
  suffix   "cn=null"
client:
  DEL cn=null
  sleep 0.1 sec
  CANCEL <previous op>
  sleep 0.2 sec
  UNBIND
receives the folllowing statslog:
  conn=1 fd=9 ACCEPT from IP=127.0.0.1:48774 (IP=127.0.0.1:3890)
  conn=1 op=0 DEL dn="cn=null"
  conn=1 op=1 EXT oid=1.3.6.1.1.8
  conn=1 op=1 CANCEL msg=1
  conn=1 op=0 ABANDONED
  conn=1 op=1 RESULT oid= err=120 text=
  conn=1 op=2 UNBIND
  conn=1 fd=9 closed
and the client gets just ExtendedResponse tooLate before connection closes

with the following patch to slapd:

Index: delete.c
--- delete.c	21 Jan 2009 23:40:26 -0000	1.144
+++ delete.c	9 May 2009 22:06:07 -0000
@@ -75,4 +75,9 @@
 		op->o_log_prefix, op->o_req_dn.bv_val, 0, 0, 0 );
 
+	{
+		struct timeval timeout = { 0, 200000 };
+		select(0, NULL, NULL, NULL, &timeout);
+	}
+
 	if( op->o_req_ndn.bv_len == 0 ) {
 		Debug( LDAP_DEBUG_ANY, "%s do_delete: root dse!\n",
Index: result.c
--- result.c	14 Mar 2009 05:47:43 -0000	1.329
+++ result.c	9 May 2009 22:06:07 -0000
@@ -418,4 +418,6 @@
 	if ( rs->sr_err == SLAPD_ABANDON || op->o_abandon ) {
 		rc = SLAPD_ABANDON;
+		Statslog( LDAP_DEBUG_STATS,
+			"%s ABANDONED\n", op->o_log_prefix, 0, 0, 0, 0 );
 		goto clean2;
 	}


Followup 1

Download message
Date: Sun, 10 May 2009 12:28:32 -0700
From: Howard Chu <hyc@symas.com>
To: h.b.furuseth@usit.uio.no
CC: openldap-its@openldap.org
Subject: Re: (ITS#6103) canceled operations do not respond
h.b.furuseth@usit.uio.no wrote:
> Full_Name: Hallvard B Furuseth
> Version: HEAD
> OS: Linux
> URL:
> Submission from: (NULL) (129.240.6.233)
> Submitted by: hallvard
>
>
> RFC 3909 says both the Cancel operation and the canceled operation
> shall respond.  However I'm getting tooLate reply from Cancel
> and no response from the successfully cancelled operation:

The tooLate reply is correct; back-null doesn't ever check for cancel/abandon 
therefore the operation is *not* successfully canceled. Likewise, back-bdb/hdb 
only checks in Delete if there is a transaction retry.

The lack of reply from the original op - that's a simple oversight. 
send_ldap_ber() checks for op->o_abandon and returns silently if it's set. I 
guess we need to also check for o_cancel to let it proceed.
>
> slapd.conf:
>    include   servers/slapd/schema/core.schema
>    database  null
>    suffix   "cn=null"
> client:
>    DEL cn=null
>    sleep 0.1 sec
>    CANCEL<previous op>
>    sleep 0.2 sec
>    UNBIND
> receives the folllowing statslog:
>    conn=1 fd=9 ACCEPT from IP=127.0.0.1:48774 (IP=127.0.0.1:3890)
>    conn=1 op=0 DEL dn="cn=null"
>    conn=1 op=1 EXT oid=1.3.6.1.1.8
>    conn=1 op=1 CANCEL msg=1
>    conn=1 op=0 ABANDONED
>    conn=1 op=1 RESULT oid= err=120 text=
>    conn=1 op=2 UNBIND
>    conn=1 fd=9 closed
> and the client gets just ExtendedResponse tooLate before connection closes
>
> with the following patch to slapd:
>
> Index: delete.c
> --- delete.c	21 Jan 2009 23:40:26 -0000	1.144
> +++ delete.c	9 May 2009 22:06:07 -0000
> @@ -75,4 +75,9 @@
>   		op->o_log_prefix, op->o_req_dn.bv_val, 0, 0, 0 );
>
> +	{
> +		struct timeval timeout = { 0, 200000 };
> +		select(0, NULL, NULL, NULL,&timeout);
> +	}
> +
>   	if( op->o_req_ndn.bv_len == 0 ) {
>   		Debug( LDAP_DEBUG_ANY, "%s do_delete: root dse!\n",
> Index: result.c
> --- result.c	14 Mar 2009 05:47:43 -0000	1.329
> +++ result.c	9 May 2009 22:06:07 -0000
> @@ -418,4 +418,6 @@
>   	if ( rs->sr_err == SLAPD_ABANDON || op->o_abandon ) {
>   		rc = SLAPD_ABANDON;
> +		Statslog( LDAP_DEBUG_STATS,
> +			"%s ABANDONED\n", op->o_log_prefix, 0, 0, 0, 0 );
>   		goto clean2;
>   	}
>
>
>


-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/



Followup 2

Download message
From: Hallvard B Furuseth <h.b.furuseth@usit.uio.no>
Date: Mon, 11 May 2009 01:05:54 +0200
To: Howard Chu <hyc@symas.com>
Cc: openldap-its@openldap.org
Subject: Re: (ITS#6103) canceled operations do not respond
Howard Chu writes:
> The tooLate reply is correct; back-null doesn't ever check for
> cancel/abandon therefore the operation is *not* successfully
> canceled. Likewise, back-bdb/hdb only checks in Delete if there is a
> transaction retry.

No difference if I use back-ldif, which does check for abandon.

> The lack of reply from the original op - that's a simple oversight. 
> send_ldap_ber() checks for op->o_abandon and returns silently if it's
set. I 
> guess we need to also check for o_cancel to let it proceed.

The operation does not reach send_ldap_ber().  To verify, I temporarily
inserted assert(!op->o_abandon) just before send_ldap_ber()'s o_abandon
test.  send_ldap_response() notices both o_abandon and SLAPD_ABANDON,
and skips to the cleanup code in either case.

Also, backend operations that do return SLAPD_ABANDON - at least Search
in backglue and BDB - may do so without calling send_ldap_response() at
all.  Pierangelo made partial fixes in ITS#4645.  See also ITS#6059,
which I may have misunderstood somewhat.  (Sorry, Rein.)

Maybe o_abandon should only mean that:
- (a) the backend has been asked to abandon the operation,
- (b) slapd must check and react to o_cancel,
- (c) if not o_cancel, send_ldap_ber() need not send.

-- 
Hallvard



Followup 3

Download message
Date: Sun, 10 May 2009 19:31:03 -0700
From: Howard Chu <hyc@symas.com>
To: Hallvard B Furuseth <h.b.furuseth@usit.uio.no>
CC: openldap-its@openldap.org
Subject: Re: (ITS#6103) canceled operations do not respond
Hallvard B Furuseth wrote:
> Howard Chu writes:
>> The tooLate reply is correct; back-null doesn't ever check for
>> cancel/abandon therefore the operation is *not* successfully
>> canceled. Likewise, back-bdb/hdb only checks in Delete if there is a
>> transaction retry.
>
> No difference if I use back-ldif, which does check for abandon.
>
>> The lack of reply from the original op - that's a simple oversight.
>> send_ldap_ber() checks for op->o_abandon and returns silently if
it's set. I
>> guess we need to also check for o_cancel to let it proceed.
>
> The operation does not reach send_ldap_ber().  To verify, I temporarily
> inserted assert(!op->o_abandon) just before send_ldap_ber()'s o_abandon
> test.  send_ldap_response() notices both o_abandon and SLAPD_ABANDON,
> and skips to the cleanup code in either case.
>
> Also, backend operations that do return SLAPD_ABANDON - at least Search
> in backglue and BDB - may do so without calling send_ldap_response() at
> all.  Pierangelo made partial fixes in ITS#4645.  See also ITS#6059,
> which I may have misunderstood somewhat.  (Sorry, Rein.)
>
> Maybe o_abandon should only mean that:
> - (a) the backend has been asked to abandon the operation,
> - (b) slapd must check and react to o_cancel,
> - (c) if not o_cancel, send_ldap_ber() need not send.
>
The test case with delete / back-ldif now works in HEAD. backglue and back-bdb 
should be OK now as well.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/



Followup 4

Download message
From: Hallvard B Furuseth <h.b.furuseth@usit.uio.no>
Date: Wed, 27 May 2009 16:30:06 +0200
To: Howard Chu <hyc@symas.com>
Cc: openldap-its@openldap.org
Subject: Re: (ITS#6103) canceled operations do not respond
The send_ldap_response() fix is still incomplete:

When rc == LDAP_CANCELLED, the function should not send sr_matched,
sr_text, sr_ref, and sr_ctrls.  Looking at how Abandon works, it seems
slap_cleanup_play() should still see them if they are not sent?

Note, not just when the function itself sets rc = LDAP_CANCELLED,
since syncprov can set rs->sr_err = LDAP_CANCELLED too.
Which also suggests the assert( rs->sr_err == LDAP_REFERRAL ) when
sr_ref != NULL is rather dubious.  Maybe sr_ref should just
be ignored when sending something else than LDAP_REFERRAL.

Also, this:
	rc = rs->sr_err;
	if ( rc == SLAPD_ABANDON && op->o_cancel )
		rc = LDAP_CANCELLED;
will send a negative result code (SLAPD_ABANDON) if !o_cancel.
I guess it should be
	if ( rc == SLAPD_ABANDON ) {
		if ( !op->o_cancel ) goto <cleanup or clean2>;
		rc = LDAP_CANCELLED;
	}

While we're at it, I suggest to generalize this to avoid sending
result codes < 0.  Send LDAP_OTHER instead.  That's ITS#5328 again.

-- 
Hallvard



Followup 5

Download message
Date: Wed, 27 May 2009 09:38:09 -0700
From: Howard Chu <hyc@symas.com>
To: Hallvard B Furuseth <h.b.furuseth@usit.uio.no>
CC: openldap-its@openldap.org
Subject: Re: (ITS#6103) canceled operations do not respond
Hallvard B Furuseth wrote:
> The send_ldap_response() fix is still incomplete:
>
> When rc == LDAP_CANCELLED, the function should not send sr_matched,
> sr_text, sr_ref, and sr_ctrls.  Looking at how Abandon works, it seems
> slap_cleanup_play() should still see them if they are not sent?

Dunno what you mean.

> Note, not just when the function itself sets rc = LDAP_CANCELLED,
> since syncprov can set rs->sr_err = LDAP_CANCELLED too.
> Which also suggests the assert( rs->sr_err == LDAP_REFERRAL ) when
> sr_ref != NULL is rather dubious.  Maybe sr_ref should just
> be ignored when sending something else than LDAP_REFERRAL.

Leave the assert() in place until you have proof one way or the other.

> Also, this:
> 	rc = rs->sr_err;
> 	if ( rc == SLAPD_ABANDON&&  op->o_cancel )
> 		rc = LDAP_CANCELLED;
> will send a negative result code (SLAPD_ABANDON) if !o_cancel.

No, it won't. That is all bypassed at the top of the function.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/



Followup 6

Download message
From: Hallvard B Furuseth <h.b.furuseth@usit.uio.no>
Date: Wed, 27 May 2009 23:32:57 +0200
To: hyc@symas.com
Cc: openldap-its@openldap.org
Subject: Re: (ITS#6103) canceled operations do not respond
hyc@symas.com writes:
>Hallvard B Furuseth wrote:
>> The send_ldap_response() fix is still incomplete:

[Reversing the quoteed message]

>> Also, this:
>> 	rc = rs->sr_err;
>> 	if ( rc == SLAPD_ABANDON&&  op->o_cancel )
>> 		rc = LDAP_CANCELLED;
>> will send a negative result code (SLAPD_ABANDON) if !o_cancel.
> 
> No, it won't. That is all bypassed at the top of the function.

If so, the quoted op->o_cancel test could be skipped.  But an overlay
could set rs->sr_err = SLAPD_ABANDON during slap_response_play().

Though that ought to mean o_abandon is set, so send_ldap_ber() will drop
the message.  So you're right after all that it won't be sent.

>> Note, not just when the function itself sets rc = LDAP_CANCELLED,
>> since syncprov can set rs->sr_err = LDAP_CANCELLED too.
>> Which also suggests the assert( rs->sr_err == LDAP_REFERRAL ) when
>> sr_ref != NULL is rather dubious.  Maybe sr_ref should just
>> be ignored when sending something else than LDAP_REFERRAL.
> 
> Leave the assert() in place until you have proof one way or the other.

Haven't tested with an example of an overlay setting SLAPD_ABANDON;
could check syncprov or retcode I suppose.  Faked it with
	nanosleep(&((struct timespec){0,2E8}), 0); /*gcc or C99 code*/
	if ( op->o_abandon ) rs->sr_err = SLAPD_ABANDON;
after the op->o_callback->sc_response in slap_response_play().
Then, with this slapd.conf:
    include	servers/slapd/schema/core.schema
    referral	ldap://elsewhere/
    database	frontend
    overlay	valsort
send
    Delete "cn=urgle"
    <wait 0.1 second>
    Abandon <previous request>
lt-slapd: result.c:479: send_ldap_response:
    Assertion `rs->sr_err == 0x0a' failed.


One more detail, looking at result.c rev 1.330 send_ldap_response():
I presume the LDAP_CONNECTIONLESS branch should also ber_printf rc
rather than rs->sr_err to the response.  Not sure if Cancel makes sense
in cldap (and after V2 Bind which has no extended operation at that),
but OpenLDAP does let through Cancel after a V2 Bind.

>> When rc == LDAP_CANCELLED, the function should not send sr_matched,
>> sr_text, sr_ref, and sr_ctrls.  Looking at how Abandon works, it seems
>> slap_cleanup_play() should still see them if they are not sent?
> 
> Dunno what you mean.

I thought that was obvious, but since it isn't maybe I should ask
ldapext instead.  Cancel does seem underspecified in this regard.

Anyway, I meant: Either an operation gets cancelled/abandoned, or it
doesn't and comletes.  If these fields are set, they most likely relate
to a completed operation.  Nowhere SLAPD_ABANDON gets set, are the
others set - possibly except with back-meta/back-ldap, I haven't looked
too closely.  OTOH, code setting SLAPD_ABANDON rarely _clears_ those
fields, so I imagine they can convey information which would have been
removed (access controls maybe?)  or updated before they were sent, if
the operation had completed.

Clearing a critical control can be a bit doubtful, I'll grant.  But for
not obeying critical controls it's a choice between which error code to
return - unavailableCriticalExtension, cancelled, or something else.  Or
some controls could mark the operation as "uncancellable" (another flag
needed...)

But come to think of it, an Extended Response has some of the same
problem.  By the above reasoning, responseName and responseValue should
be dropped in a Cancelled response, as when sending protocolError to an
unrecognized reqest.  Might be a bit extreme.

-- 
Hallvard


Up to top level
Build   Contrib   Development   Documentation   Historical   Incoming   Software Bugs   Software Enhancements   Web  

Logged in as guest


The OpenLDAP Issue Tracking System uses a hacked version of JitterBug

______________
© Copyright 2013, OpenLDAP Foundation, info@OpenLDAP.org