On Thu, 24 Oct 2019 14:06:47 +0200,
Martijn van Duren wrote:
>
> 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.
Sorry. Been too happy to get it working.
Is it necessary to assign req->op ? I didn't see it used and it gets
freed in the call to request_free().
Robert
> >
> >
> > 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) {
