Re: LDAP tls: handshake failure

2019-11-02 Thread Robert Klein
Hi Pedrag,

On Sat, 02 Nov 2019 05:45:08 +0100,
Predrag Punosevac wrote:
> 
> Martijn van Duren wrote:
> 
> > On 10/24/19 2:25 PM, Claudio Jeker wrote:
> > > 
> > > OK claudio@
> > > 
> > I'll commit this soon-ish based on claudio's OK, but if at all
> > possible I would like to ask the people affected by this to test this
> > and see if this solves their problem.
> 
> I did this on the pair of LDAP servers atlas and titan to make sure I
> can reproduce results.
> 
> atlas# uname -a
> OpenBSD atlas.int.autonlab.org 6.6 GENERIC.MP#0 amd64
> atlas# syspatch -l
> 001_bpf
> 002_ber
> 003_bgpd
> atlas# rcctl restart ldapd
> ldapd(ok)
> ldapd(ok)
> atlas# ldapvi -ZZ
> ldap_start_tls_s: Protocol error (2)
> 
> 
> # Getting source code 
> 
> atlas# cvs -qd anon...@anoncvs.ca.openbsd.org:/cvs checkout -rOPENBSD_6_6 -P 
> src
> atlas# cvs -q up -Pd -rOPENBSD_6_6
> 
> atlas# make clean
> atlas# make obj
> atlas# make
> atlas# make install
> 
> #atlas rcctl restart ldapd
> ldapd(ok)
> ldapd(ok)
> atlas# ldapvi -ZZ
> ldap_start_tls_s: Protocol error (2)
> 
> Upon close inspection I see that cvs is pulling the revision 1.31.2.1 
> 
> https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.sbin/ldapd/ldape.c?r1=1.33
> which is the same as the binary patch I already installed.
> 
> 
> Manually fetching revision 1.33 which I am guessing is going to current.
> and rebuilding the daemon 
> 
> ldapvi -ZZ
> 
> is now sucessful.
> 
> So for me personally version 1.33 of ldape.c works. This is the
> difference between 1.31.2.1 which can be obtained as a binary patch and
> the version 1.33
> 
> atlas# diff ldape.c.v.1.31.2.1 ldape.c.v.1.33
> 1c1
> < /*$OpenBSD: ldape.c,v 1.31.2.1 2019/10/27 20:05:13 tb Exp $ */
> ---
> > /*$OpenBSD: ldape.c,v 1.33 2019/10/26 17:52:55 martijn Exp $ */
> 301d300
> <   struct ber_element  *ext_val = NULL;
> 310c309
> <   if (ober_scanf_elements(req->op, "{se", , _val) != 0)
> ---
> >   if (ober_scanf_elements(req->op, "{s", ) != 0)
> 314c313
> <   req->op = ext_val;
> ---
> >   req->op = req->op->be_sub->be_next;

the patch is only in -current / snapshots, but not in -stable

Best regards
Robert



Re: LDAP tls: handshake failure

2019-11-01 Thread Predrag Punosevac
Martijn van Duren wrote:

> On 10/24/19 2:25 PM, Claudio Jeker wrote:
> > 
> > OK claudio@
> > 
> I'll commit this soon-ish based on claudio's OK, but if at all
> possible I would like to ask the people affected by this to test this
> and see if this solves their problem.

I did this on the pair of LDAP servers atlas and titan to make sure I
can reproduce results.

atlas# uname -a
OpenBSD atlas.int.autonlab.org 6.6 GENERIC.MP#0 amd64
atlas# syspatch -l
001_bpf
002_ber
003_bgpd
atlas# rcctl restart ldapd
ldapd(ok)
ldapd(ok)
atlas# ldapvi -ZZ
ldap_start_tls_s: Protocol error (2)


# Getting source code 

atlas# cvs -qd anon...@anoncvs.ca.openbsd.org:/cvs checkout -rOPENBSD_6_6 -P src
atlas# cvs -q up -Pd -rOPENBSD_6_6

atlas# make clean
atlas# make obj
atlas# make
atlas# make install

#atlas rcctl restart ldapd
ldapd(ok)
ldapd(ok)
atlas# ldapvi -ZZ
ldap_start_tls_s: Protocol error (2)

Upon close inspection I see that cvs is pulling the revision 1.31.2.1 

https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.sbin/ldapd/ldape.c?r1=1.33
which is the same as the binary patch I already installed.


Manually fetching revision 1.33 which I am guessing is going to current.
and rebuilding the daemon 

ldapvi -ZZ

is now sucessful.

So for me personally version 1.33 of ldape.c works. This is the
difference between 1.31.2.1 which can be obtained as a binary patch and
the version 1.33

atlas# diff ldape.c.v.1.31.2.1 ldape.c.v.1.33
1c1
< /*$OpenBSD: ldape.c,v 1.31.2.1 2019/10/27 20:05:13 tb Exp $ */
---
> /*$OpenBSD: ldape.c,v 1.33 2019/10/26 17:52:55 martijn Exp $ */
301d300
<   struct ber_element  *ext_val = NULL;
310c309
<   if (ober_scanf_elements(req->op, "{se", , _val) != 0)
---
>   if (ober_scanf_elements(req->op, "{s", ) != 0)
314c313
<   req->op = ext_val;
---
>   req->op = req->op->be_sub->be_next;


Cheers,
Predrag



Re: LDAP tls: handshake failure

2019-10-24 Thread Robert Klein
On Thu, 24 Oct 2019 15:35:44 +0200,
Martijn van Duren wrote:
> 
> On 10/24/19 3:29 PM, Robert Klein wrote:
> > 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().
> 
> In its current form probably not, but on the other hand it keeps the
> current behaviour/intent more consistent and might help expand if we
> ever want to add additional extended operations.
> 
> If you feel strongly I'll remove it altogether, I'm not strongly
> inclined either way.

No, I just wanted to know.  Better keep it for the moment.

Robert

> > 
> > 
> > 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.c28 Jun 2019 13:32:48 -  1.31
> >> +++ ldape.c24 Oct 2019 12:05:19 -
> >> @@ -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", , _val) != 0)
> >> +  if (ber_scanf_elements(req->op, "{s", ) != 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) {
> > 



Re: LDAP tls: handshake failure

2019-10-24 Thread Martijn van Duren
On 10/24/19 3:29 PM, Robert Klein wrote:
> 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().

In its current form probably not, but on the other hand it keeps the
current behaviour/intent more consistent and might help expand if we
ever want to add additional extended operations.

If you feel strongly I'll remove it altogether, I'm not strongly
inclined either way.
> 
> 
> 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 -  1.31
>> +++ ldape.c  24 Oct 2019 12:05:19 -
>> @@ -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", , _val) != 0)
>> +if (ber_scanf_elements(req->op, "{s", ) != 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) {
> 



Re: LDAP tls: handshake failure

2019-10-24 Thread Robert Klein
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 -  1.31
> +++ ldape.c   24 Oct 2019 12:05:19 -
> @@ -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", , _val) != 0)
> + if (ber_scanf_elements(req->op, "{s", ) != 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) {



Re: LDAP tls: handshake failure

2019-10-24 Thread Martijn van Duren
On 10/24/19 2:25 PM, Claudio Jeker wrote:
> 
> OK claudio@
> 
I'll commit this soon-ish based on claudio's OK, but if at all possible 
I would like to ask the people affected by this to test this and see if 
this solves their problem.

For the people running -current, here's an updated diff based on tb@'s
commit which changes ber_* to ober_* (note that you also need to get
the latest libutil).

martijn@

Index: ldape.c
===
RCS file: /cvs/src/usr.sbin/ldapd/ldape.c,v
retrieving revision 1.32
diff -u -p -r1.32 ldape.c
--- ldape.c 24 Oct 2019 12:39:26 -  1.32
+++ ldape.c 24 Oct 2019 13:22:37 -
@@ -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 (ober_scanf_elements(req->op, "{se", , _val) != 0)
+   if (ober_scanf_elements(req->op, "{s", ) != 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) {



Re: LDAP tls: handshake failure

2019-10-24 Thread Claudio Jeker
On Thu, Oct 24, 2019 at 02:06:47PM +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.
> > 
> > 
> > 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 -  1.31
> +++ ldape.c   24 Oct 2019 12:05:19 -
> @@ -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", , _val) != 0)
> + if (ber_scanf_elements(req->op, "{s", ) != 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) {

OK claudio@

-- 
:wq Claudio



Re: LDAP tls: handshake failure

2019-10-24 Thread Martijn van Duren
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 -  1.31
+++ ldape.c 24 Oct 2019 12:05:19 -
@@ -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", , _val) != 0)
+   if (ber_scanf_elements(req->op, "{s", ) != 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) {



Re: LDAP tls: handshake failure

2019-10-24 Thread Robert Klein
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

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.

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).


Best regards
Robert



--- ldape.c.origFri Jun 28 15:32:48 2019
+++ ldape.c Thu Oct 24 13:28:38 2019
@@ -307,7 +307,7 @@
{ NULL }
};

-   if (ber_scanf_elements(req->op, "{se", , _val) != 0)
+   if (ber_scanf_elements(req->op, "{s", , _val) != 0)
goto done;

log_debug("got extended operation %s", oid);




Re: LDAP tls: handshake failure

2019-10-23 Thread Predrag Punosevac
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.
> >
> > titan# uname -a
> > OpenBSD titan.int.autonlab.org 6.6 GENERIC.MP#372 amd64
> >
> > I am using LDAP daemon from the base
> >
> > titan# more /etc/ldapd.conf
> > #   $OpenBSD: ldapd.conf,v 1.1 2014/07/11 21:20:10 deraadt Exp $
> >
> > schema "/etc/ldap/core.schema"
> > schema "/etc/ldap/inetorgperson.schema"
> > schema "/etc/ldap/nis.schema"
> >
> > listen on lo0 tls certificate titan
> > listen on em0 tls certificate titan
> > listen on "/var/run/ldapi"
> >
> > namespace "dc=autonlab,dc=org" {
> > rootdn  "cn=admin,dc=autonlab,dc=org"
> > rootpw  "{SSHA}secret"
> > index   sn
> > index   givenName
> > index   cn
> > index   mail
> > }
> >
> >
> > Server certificate is regenerated and signed by my own certification of
> > authority which is on the different machine. I used easy-rsa just like
> > for one of my OpenBSD server.
> >
> >
> > This is the configuration of openldap-client on the LDAP server itself
> > which is used to modify database
> >
> > titan# pkg_info |grep openldap
> > openldap-client-2.4.48 open-source LDAP software (client)
> > openldap-server-2.4.48 open-source LDAP software (server)
> >
> > titan# more ldap.conf
> > BASEdc=autonlab,dc=org
> > URI ldap://titan.int.autonlab.org:389
> >
> > SIZELIMIT   12
> > TIMELIMIT   15
> > DEREF   never
> >
> > SSL START_TLS
> > TLS_REQCERT demand
> >
> > TLS_CACERT  /etc/ldap/certs/ca.crt
> > TLS_CERT/etc/ldap/certs/titan.crt
> > TLS_CACERTDIR   /etc/ldap/certs
> > TLS_CIPHER_SUITE
> > ECDHE-RSA-AES256-SHA384:AES256-SHA256:!RC4:HIGH:!MD5:!aNULL:!EDH:!EXP:!SSLV2:!eNULL
> > TLS_PROTOCOL_MIN 3.3
> >
> > I didn't change DNS settings and I even have 
> >
> > titan# more /etc/hosts
> > 127.0.0.1   localhost
> > ::1 localhost
> > 192.168.6.1 titan.int.autonlab.org titan
> >
> >
> > I would appreciate any clues.
> >
> > Cheers,
> > Predrag Punosevac
> >
> >
> >
> 
> ldapsearch -d9 might give some hint.
> 
> openssl s_client -connect titan.int.autonlab.org:389 -starttls ldap
> 
> might also give something.

Thank you so much for this hints. This is what I have done. I have
rebuilt a LDAP server using 6.5 

deimos# uname -a
OpenBSD deimos.int.autonlab.org 6.5 GENERIC.MP#5 amd64

with identical configuration to nonfunctional server

titan# uname -a
OpenBSD titan.int.autonlab.org 6.6 GENERIC.MP#372 amd64

on the fully functional server I see

deimos# ldapsearch -d9 -ZZ -D "cn=admin,dc=autonlab,dc=org"  -W 
ldap_create
ldap_extended_operation_s
ldap_extended_operation
ldap_send_initial_request
ldap_new_connection 1 1 0
ldap_int_open_connection
ldap_connect_to_host: TCP deimos.int.autonlab.org:389
ldap_new_socket: 3
ldap_prepare_socket: 3
ldap_connect_to_host: Trying 192.168.6.253:389
ldap_pvt_connect: fd: 3 tm: -1 async: 0
attempting to connect: 
connect success
ldap_open_defconn: successful
ldap_send_server_request
ber_scanf fmt ({it) ber:
ber_scanf fmt ({) ber:
ber_flush2: 31 bytes to sd 3
ldap_result ld 0xea5c2f97080 msgid 1
wait4msg ld 0xea5c2f97080 msgid 1 (infinite timeout)
wait4msg continue ld 0xea5c2f97080 msgid 1 all 1
** ld 0xea5c2f97080 Connections:
* host: deimos.int.autonlab.org  port: 389  (default)
  refcnt: 2  status: Connected
  last used: Wed Oct 23 23:16:25 2019


** ld 0xea5c2f97080 Outstanding Requests:
 * msgid 1,  origid 1, status InProgress
   outstanding referrals 0, parent count 0
  ld 0xea5c2f97080 request count 1 (abandoned 0)
** ld 0xea5c2f97080 Response Queue:
   Empty
  ld 0xea5c2f97080 response count 0
ldap_chkResponseList ld 0xea5c2f97080 msgid 1 all 1
ldap_chkResponseList returns ld 0xea5c2f97080 NULL
ldap_int_select
read1msg: ld 0xea5c2f97080 msgid 1 all 1
ber_get_next
ber_get_next: tag 0x30 len 36 contents:
read1msg: ld 0xea5c2f97080 msgid 1 message type extended-result
ber_scanf fmt ({eAA) ber:
read1msg: ld 0xea5c2f97080 0 new referrals
read1msg:  mark request completed, ld 0xea5c2f97080 msgid 1
request done: ld 0xea5c2f97080 msgid 1
res_errno: 0, res_error: <>, res_matched: <>
ldap_free_request (origid 1, msgid 1)
ldap_parse_extended_result
ber_scanf fmt ({eAA) ber:
ldap_parse_result
ber_scanf fmt ({iAA) ber:
ber_scanf fmt (}) ber:
ldap_msgfree
TLS trace: SSL_connect:before/connect initialization
TLS trace: SSL_connect:SSLv3 write client hello A
TLS trace: SSL_connect:SSLv3 read server hello A
TLS certificate verification: depth: 1, err: 0, subject:
/C=US/ST=Pennsylvania/L=Pittsburgh/O=Carnegie Mellon University/OU=The
Auton Lab/CN=The Auton Lab CA/emailAddress=predr...@cs.cmu.edu, issuer:
/C=US/ST=Pennsylvania/L=Pittsburgh/O=Carnegie Mellon 

Re: LDAP tls: handshake failure

2019-10-23 Thread Kapetanakis Giannis
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.
>
> titan# uname -a
> OpenBSD titan.int.autonlab.org 6.6 GENERIC.MP#372 amd64
>
> I am using LDAP daemon from the base
>
> titan# more /etc/ldapd.conf
> #   $OpenBSD: ldapd.conf,v 1.1 2014/07/11 21:20:10 deraadt Exp $
>
> schema "/etc/ldap/core.schema"
> schema "/etc/ldap/inetorgperson.schema"
> schema "/etc/ldap/nis.schema"
>
> listen on lo0 tls certificate titan
> listen on em0 tls certificate titan
> listen on "/var/run/ldapi"
>
> namespace "dc=autonlab,dc=org" {
> rootdn  "cn=admin,dc=autonlab,dc=org"
> rootpw  "{SSHA}secret"
> index   sn
> index   givenName
> index   cn
> index   mail
> }
>
>
> Server certificate is regenerated and signed by my own certification of
> authority which is on the different machine. I used easy-rsa just like
> for one of my OpenBSD server.
>
>
> This is the configuration of openldap-client on the LDAP server itself
> which is used to modify database
>
> titan# pkg_info |grep openldap
> openldap-client-2.4.48 open-source LDAP software (client)
> openldap-server-2.4.48 open-source LDAP software (server)
>
> titan# more ldap.conf
> BASEdc=autonlab,dc=org
> URI ldap://titan.int.autonlab.org:389
>
> SIZELIMIT   12
> TIMELIMIT   15
> DEREF   never
>
> SSL START_TLS
> TLS_REQCERT demand
>
> TLS_CACERT  /etc/ldap/certs/ca.crt
> TLS_CERT/etc/ldap/certs/titan.crt
> TLS_CACERTDIR   /etc/ldap/certs
> TLS_CIPHER_SUITE
> ECDHE-RSA-AES256-SHA384:AES256-SHA256:!RC4:HIGH:!MD5:!aNULL:!EDH:!EXP:!SSLV2:!eNULL
> TLS_PROTOCOL_MIN 3.3
>
> I didn't change DNS settings and I even have 
>
> titan# more /etc/hosts
> 127.0.0.1   localhost
> ::1 localhost
> 192.168.6.1 titan.int.autonlab.org titan
>
>
> I would appreciate any clues.
>
> Cheers,
> Predrag Punosevac
>
>
>

ldapsearch -d9 might give some hint.

openssl s_client -connect titan.int.autonlab.org:389 -starttls ldap

might also give something.

G