Re: (ITS#8474) Securely Erase BerElement Buffer After Use

[hyc@symas.com]

Alan Cronin (alcronin) wrote:
> Hi Howard,
>
> I can change the code so that it overwrites the memory from
ldap_free_request_int() instead. This will cover the execution using a bind
(both synchronous and asynchronous). Is there any other places where the
password would be in the buffer that needs to be cleared? The reason for
inserting it into ber_free_buf() was that it would clear all places.

I see, you're looking at the client side. What about the slapd side?

There are 2 main places where passwords occur - Bind, and PasswordModify exop. 
They might also occur in Add and Modify requests. Add is not so unusual, 
Modify should not happen since clients should use PasswordModify instead.

> Some compilers will optimize calls to memset() so that only the first
character will be changed. Leaving the rest of the string intact.

That's clearly a compiler bug then. If you have examples of such situations 
you should report that to the respective compiler maintainers.

> This is why
the iteration is in place.
>
> Thanks for looking into this patch.
>
> Alan
>
> On 05/08/2016, 19:50, "Howard Chu" <hyc@symas.com> wrote:
>
>      alcronin@cisco.com wrote:
>      > Full_Name: Alan Cronin
>      > Version: 2.4.44
>      > OS: Windows 8.1
>      > URL: https://dl.dropboxusercontent.com/u/82343475/SecurelyEraseBuffer.diff
>      > Submission from: (NULL) (2001:420:4041:2003:f942:59a5:545f:b334)
>      >
>      >
>      > The following patch is a modification to the OpenLDAP BerElement buffer. The
>      > buffer can be used to contain the LDAP request including the password for
>      > authentication. While this is free'd when it is no longer needed, the contents
>      > of the buffer is not overwritten from memory. This can lead to someone reading
>      > the memory of the process and determining what the password is. The change
>      > included in this patch will iterate over the memory and clear it. This will
>      > remove any trace of the password by the time execution is handed back to the
>      > caller.
>
>      Why would you insert a performance pessimization into every use of the LBER
>      library, rather than just erasing the password from a Bind request?
>
>      Why would you use an explicitly coded loop setting one character at a time,
>      instead of using libc's memset() which has probably been well optimized?
>
>      > The attached patch file is derived from OpenLDAP Software. All of the
>      > modifications to OpenLDAP Software represented in the following patch(es) were
>      > developed by Alan Cronin alcronin@cisco.com. I have not assigned rights and/or
>      > interest in this work to any party.
>      > The attached file is derived from OpenLDAP Software. All of the modifications to
>      > OpenLDAP Software represented in the following patch(es) were developed by Cisco
>      > Systems, Inc.. Cisco Systems, Inc. has not assigned rights and/or interest in
>      > this work to any party. I, Alan Cronin am authorized by Cisco Systems, Inc., my
>      > employer, to release this work under the following terms.
>      > Cisco Systems, Inc. hereby place the following modifications to OpenLDAP
>      > Software (and only these modifications) into the public domain. Hence, these
>      > modifications may be freely used and/or redistributed for any purpose with or
>      > without attribution and/or other notice.
>      >
>      >
>
>
>      --
>         -- Howard Chu
>         CTO, Symas Corp.           http://www.symas.com
>         Director, Highland Sun     http://highlandsun.com/hyc/
>         Chief Architect, OpenLDAP  http://www.openldap.org/project/
>
>


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