Need feedback for proposed changes in Apache source

["Rai, Pravesh R (STSD)" <pr...@hp.com>]

Hi,

We are using Apache 2.2.21 with our product in HP. As we all know that during some failure operations, Windows OS stores the memory dump as .mdmp & .hdmp files. In our case we have observed credentials (in plain text) in those dump files, which is a security concern for us.

During our investigation, we found that Apache source uses memcpy() at many places, which always leave behind the source string (in this case, credentials in plain text) in the memory. Also observed that the destination buffer, if bigger than the source buffer, always have remnants of its original content after copy/move operations. Such memory locations hold the data for unknown longer duration & any exception during the course exposes all these data in the dump file.

Have tried to modify few Apache source files, like:

httpd\srclib\apr-util\buckets\apr_brigade.c (diff file w.r.t. to Apache 2.2.21: diff_apr_brigade.c.txt)
httpd\modules\ssl\ssl_engine_io.c (diff file w.r.t. to Apache 2.2.21: diff_ssl_engine_io.c.txt)

Though the changes are minor & mainly intended to clean the buffer, but so far our Security testing team has not found any plain text credentials in any of our application dump files. Please go through these changes & let us know your views.

Thanks & Regards,
Pravesh

Re: Need feedback for proposed changes in Apache source

[Nick Kew <ni...@webthing.com>]

On Tue, 31 Jan 2012 11:27:32 +0000
"Rai, Pravesh R (STSD)" <pr...@hp.com> wrote:

> Hi,
> 
> We are using Apache 2.2.21 with our product in HP. As we all know that during some failure operations, Windows OS stores the memory dump as .mdmp & .hdmp files. In our case we have observed credentials (in plain text) in those dump files, which is a security concern for us.

Thanks for pointing this out.

What credentials specifically?  If that's HTTP Basic Auth
(or equivalent level of non-encryption) the strings from the
HTTP Request headers will exist in memory with the potential
to appear in a dump!

> Have tried to modify few Apache source files, like:
> 
> httpd\srclib\apr-util\buckets\apr_brigade.c (diff file w.r.t. to Apache 2.2.21: diff_apr_brigade.c.txt)

Your diff looks fine, but note that anything under apr or apr-util
belongs to the APR project, not to httpd.

> httpd\modules\ssl\ssl_engine_io.c (diff file w.r.t. to Apache 2.2.21: diff_ssl_engine_io.c.txt)

That one sets to NULL after a memmove.  Since memmove supports overlapping
source and destination buffers, you could be introducing complex edge-case
side-effects.  Have you checked?

> Though the changes are minor & mainly intended to clean the buffer, but so far our Security testing team has not found any plain text credentials in any of our application dump files. Please go through these changes & let us know your views.

I can imagine there might be other such instances in the code.

-- 
Nick Kew

Re: Need feedback for proposed changes in Apache source

[Graham Leggett <mi...@sharp.fm>]

On 31 Jan 2012, at 1:27 PM, Rai, Pravesh R (STSD) wrote:

> We are using Apache 2.2.21 with our product in HP. As we all know that during some failure operations, Windows OS stores the memory dump as .mdmp & .hdmp files. In our case we have observed credentials (in plain text) in those dump files, which is a security concern for us.

Keep in mind that a dump file is just that - a snapshot of memory at a point in time. It is an unavoidable situation that in order for the server to work at all, credentials need to be held in a memory buffer for periods of time while those credentials are being used. There is no workout for this.

What this in turn means is that those memory dumps are at all times a security risk, and should be treated with appropriate care and attention. Zeroing out memory buffers will never remove this requirement.

That said, trying to minimise the number of copies of known-to-be-credential data that end up lying around is not a bad thing, but zeroing out absolutely everything has performance implications. If you want to do that, the secret would be to make the behaviour optional, so that the administrator can make up their own mind as to their priorities. I must stress again though - even if you went the whole hog and zeroed out all buffers after you used them, your memory dumps could still contain credentials, just less of them, but would still have to be treated with the same care as before.

> During our investigation, we found that Apache source uses memcpy() at many places, which always leave behind the source string (in this case, credentials in plain text) in the memory. Also observed that the destination buffer, if bigger than the source buffer, always have remnants of its original content after copy/move operations. Such memory locations hold the data for unknown longer duration & any exception during the course exposes all these data in the dump file.
>  
>  
> Have tried to modify few Apache source files, like:
>  
> httpd\srclib\apr-util\buckets\apr_brigade.c (diff file w.r.t. to Apache 2.2.21: diff_apr_brigade.c.txt)

This change above is a problem, because it attempts to zero out the data in the original bucket in the apr_brigade_flatten() function, and that breaks the function - overriding "const" in this situation should have warned you of this. There is no reason why a bucket can't be accessed twice (and in mod_cache, buckets are always accessed twice, once to write them to the cache, once to write them to the network).

If you want to make a general across-the-board clear-out-everything switch, you would need to modify the apr pools code to optionally do this to run when cleanups are run. If you have a specific buffer that you want to ensure doesn't live longer than the current pool, use the apr_crypto_clear() function in apr-util v1.4+.

However, keep in mind that pool lifetimes can vary from minutes (in the case of requests), to practically forever (in the case of keepalive connections), so you'll still be leaking some credentials in dumps.

Regards,
Graham
--

Re: Need feedback for proposed changes in Apache source

[Graham Leggett <mi...@sharp.fm>]

On 31 Jan 2012, at 1:27 PM, Rai, Pravesh R (STSD) wrote:

> We are using Apache 2.2.21 with our product in HP. As we all know that during some failure operations, Windows OS stores the memory dump as .mdmp & .hdmp files. In our case we have observed credentials (in plain text) in those dump files, which is a security concern for us.

Keep in mind that a dump file is just that - a snapshot of memory at a point in time. It is an unavoidable situation that in order for the server to work at all, credentials need to be held in a memory buffer for periods of time while those credentials are being used. There is no workout for this.

What this in turn means is that those memory dumps are at all times a security risk, and should be treated with appropriate care and attention. Zeroing out memory buffers will never remove this requirement.

That said, trying to minimise the number of copies of known-to-be-credential data that end up lying around is not a bad thing, but zeroing out absolutely everything has performance implications. If you want to do that, the secret would be to make the behaviour optional, so that the administrator can make up their own mind as to their priorities. I must stress again though - even if you went the whole hog and zeroed out all buffers after you used them, your memory dumps could still contain credentials, just less of them, but would still have to be treated with the same care as before.

> During our investigation, we found that Apache source uses memcpy() at many places, which always leave behind the source string (in this case, credentials in plain text) in the memory. Also observed that the destination buffer, if bigger than the source buffer, always have remnants of its original content after copy/move operations. Such memory locations hold the data for unknown longer duration & any exception during the course exposes all these data in the dump file.
>  
>  
> Have tried to modify few Apache source files, like:
>  
> httpd\srclib\apr-util\buckets\apr_brigade.c (diff file w.r.t. to Apache 2.2.21: diff_apr_brigade.c.txt)

This change above is a problem, because it attempts to zero out the data in the original bucket in the apr_brigade_flatten() function, and that breaks the function - overriding "const" in this situation should have warned you of this. There is no reason why a bucket can't be accessed twice (and in mod_cache, buckets are always accessed twice, once to write them to the cache, once to write them to the network).

If you want to make a general across-the-board clear-out-everything switch, you would need to modify the apr pools code to optionally do this to run when cleanups are run. If you have a specific buffer that you want to ensure doesn't live longer than the current pool, use the apr_crypto_clear() function in apr-util v1.4+.

However, keep in mind that pool lifetimes can vary from minutes (in the case of requests), to practically forever (in the case of keepalive connections), so you'll still be leaking some credentials in dumps.

Regards,
Graham
--