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" <[email protected]> wrote: > > [email protected] 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 [email protected]. 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/
