On 10/24/19 1:50 PM, Robert Klein wrote:
> Hi,
>
>
>
> On Thu, 24 Oct 2019 05:26:49 +0200,
> Predrag Punosevac wrote:
>>
>> Kapetanakis Giannis wrote:
>>
>>> On 23/10/2019 19:14, Predrag Punosevac wrote:
>>>> Hi Misc,
>>>>
>>>> I just upgraded a LDAP server from 6.5 to 6.6 running authorization and
>>>> authentication services for a 100 some member university research group.
>>>> It appears TLS handshake is broken. This worked perfectly on 6.5 and
>>>> earlier.
>>>>
>
> [ rest deleted ]
>
>> I am out of fuel to look more this tonight but I am 99% sure something
>> have changed on 6.6 which broke the things. Maybe my configuration was
>> wrong all along and in 6.6 few screws got tighten up which bit me for my
>> rear end. I would appreciate any further commend or suggestions how to
>> debug this. I would also appreciate any reports of fully working ldapd
>> on 6.6 release
>>
>> Best,
>> Predrag
>>
>
> This is related to commit “Make sure that ber in ber_scanf_elements is
> not NULL before parsing format” (martijn@) and caused by the scan string
> used by ber_scanf_elements on line 310 in ldape.c
Thanks for looking into this. I didn't found the time yet.
>
> Regarding the commit, see also emails with subject “ber.c: Don't
> continue on nonexistent ber” to tech@ on August, 13.
>
> When you set scan string for ber_scanf_elements in line 310 of ldape.c
> from "{se" to "{s" it works again. Patch below.
>
> When you look at the ldap_extended function on ldape.c, you see ext_val
> is assigned to req_op in line 314. The only use could happen in the
> extended_ops[i]fn(req) call in line 318. This currently can only be a
> call to ldap_starttls (beginning at line 285, same file) which doesn't
> use req_op either. So it the `e' shouldn't matter.
>
> At a guess, this also conforms to RFC4511, section 4.14.1.
Glancing over the RFC seems that you are correct.
>
> If ldap_extended is extended to handle other operations than starttls,
> care must be taken for an optional additional octet string in the
> request (see definition of extended request in RFC4511, section 4.12).
Diff below should handle this. Also, you forgot to remove the ext_val.
>
>
> Best regards
> Robert
>
martijn@
Index: ldape.c
===================================================================
RCS file: /cvs/src/usr.sbin/ldapd/ldape.c,v
retrieving revision 1.31
diff -u -p -r1.31 ldape.c
--- ldape.c 28 Jun 2019 13:32:48 -0000 1.31
+++ ldape.c 24 Oct 2019 12:05:19 -0000
@@ -298,7 +298,6 @@ ldap_extended(struct request *req)
{
int i, rc = LDAP_PROTOCOL_ERROR;
char *oid = NULL;
- struct ber_element *ext_val = NULL;
struct {
const char *oid;
int (*fn)(struct request *);
@@ -307,11 +306,11 @@ ldap_extended(struct request *req)
{ NULL }
};
- if (ber_scanf_elements(req->op, "{se", &oid, &ext_val) != 0)
+ if (ber_scanf_elements(req->op, "{s", &oid) != 0)
goto done;
log_debug("got extended operation %s", oid);
- req->op = ext_val;
+ req->op = req->op->be_sub->be_next;
for (i = 0; extended_ops[i].oid != NULL; i++) {
if (strcmp(oid, extended_ops[i].oid) == 0) {
