Record Layer Buffers & Zeroization

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Record Layer Buffers & Zeroization

Martin Elshuber
Hi,

I have a question regarding the recordlayer and zeroization of plain
text data.

To my understanding openssl (I am on v1.1.1g) maintains inbound data for
TLS1.2 within
SSL.rlayer.rbuf.The data is split into records and the meta information
is kept in
SSL.rlayer.rrec. The data pointers isself alias SSS.rlayer.rbuf.

Decryption of inbound data is done in-place in ssl3_get_record(). So the
rbuf stores
(possibly sensitive) plaintext data until it is reused by followup data.

Is this correct?

I might be blind, but I just cannot find the location where this
plaintext data is
zeroized, neither by OPENSSL_cleanse() nor memset().

Am I blind, or is this just not done? Shouldn't there be a way to do
this just like
it is already done with keys?

The reasoning behind this, is that in some cases it might not only be
necessary to get
rid of keys when no longer used, but also to get rid of all residuals
transported over
the secure channel.

Note on outbound: In the outbound path, to my understanding no plaintext
copies are
created, since plain data only lives in the user buffer passed to
openssl. The user
has to take of this part.

Thx in advance & keep up your good work & kind regards
Martin



Reply | Threaded
Open this post in threaded view
|

Re: Record Layer Buffers & Zeroization

Matt Caswell-2


On 22/06/2020 18:28, Martin Elshuber wrote:
> I might be blind, but I just cannot find the location where this
> plaintext data is
> zeroized, neither by OPENSSL_cleanse() nor memset().
>
> Am I blind, or is this just not done? Shouldn't there be a way to do
> this just like
> it is already done with keys?

We don't currently do this. There would likely be some significant
performance impacts for doing this with all plaintext. That said it
might be a nice optional feature to add.

Matt
Reply | Threaded
Open this post in threaded view
|

Re: Record Layer Buffers & Zeroization

Martin Elshuber
Thx for the answer,

than at least a can stop looking for this :).

And yes I can understand the performance hit and I agree that this
should be optional and disabled by default.

I am thinking of adding a OPENSSL_cleanse just ofter the memcpy in
ssl3_read_bytes. And probably replacing the OPENSSL_free by an
OPENSSL_clear_free in SSL3_BUFFER_release. The later gets rid of data
not yet deliverd to the application.

I am thinking to make both dependent on a flags such as
SSL_OP_CLEANSE_PLAINTEXT. But I am not sure how to select a suitable
bit, since all unused bits are currently stated to be reserved for
openssl 1.2.

Another option is to add a compiler option; Though I do not like this
too much.

Did I forget any locations where to more cleanses? Any other suggestions?

If you are still interested, I am happy to prepare a PR and move the
discussion there. I guess this is "CLA: trivial" patch anyways.

Martin

Am 23.06.2020 um 12:19 schrieb Matt Caswell:

>
> On 22/06/2020 18:28, Martin Elshuber wrote:
>> I might be blind, but I just cannot find the location where this
>> plaintext data is
>> zeroized, neither by OPENSSL_cleanse() nor memset().
>>
>> Am I blind, or is this just not done? Shouldn't there be a way to do
>> this just like
>> it is already done with keys?
> We don't currently do this. There would likely be some significant
> performance impacts for doing this with all plaintext. That said it
> might be a nice optional feature to add.
>
> Matt


Reply | Threaded
Open this post in threaded view
|

Re: Record Layer Buffers & Zeroization

Matt Caswell-2


On 23/06/2020 11:57, Martin Elshuber wrote:

> Thx for the answer,
>
> than at least a can stop looking for this :).
>
> And yes I can understand the performance hit and I agree that this
> should be optional and disabled by default.
>
> I am thinking of adding a OPENSSL_cleanse just ofter the memcpy in
> ssl3_read_bytes. And probably replacing the OPENSSL_free by an
> OPENSSL_clear_free in SSL3_BUFFER_release. The later gets rid of data
> not yet deliverd to the application.
>
> I am thinking to make both dependent on a flags such as
> SSL_OP_CLEANSE_PLAINTEXT. But I am not sure how to select a suitable
> bit, since all unused bits are currently stated to be reserved for
> openssl 1.2.

We don't allow features in stable branches, so this could only be
included in the forthcoming OpenSSL 3.0 (i.e the master branch). You can
use those reserved values there because we are not aiming for ABI
compatibility (only API).


>
> Another option is to add a compiler option; Though I do not like this
> too much.
>
> Did I forget any locations where to more cleanses? Any other suggestions?
>
> If you are still interested, I am happy to prepare a PR and move the
> discussion there. I guess this is "CLA: trivial" patch anyways.

Please feel free to raise a PR - although this is unlikely to be CLA
trivial (we have a very high bar for something to be considered trivial).

Matt



>
> Martin
>
> Am 23.06.2020 um 12:19 schrieb Matt Caswell:
>>
>> On 22/06/2020 18:28, Martin Elshuber wrote:
>>> I might be blind, but I just cannot find the location where this
>>> plaintext data is
>>> zeroized, neither by OPENSSL_cleanse() nor memset().
>>>
>>> Am I blind, or is this just not done? Shouldn't there be a way to do
>>> this just like
>>> it is already done with keys?
>> We don't currently do this. There would likely be some significant
>> performance impacts for doing this with all plaintext. That said it
>> might be a nice optional feature to add.
>>
>> Matt
>
>