[openssl-project] to fully overlap or not to

[Bernd Edlinger]

On 03/01/18 10:59, Andy Polyakov wrote:
>>>> I'd like to request more opinions on
>>>> https://github.com/openssl/openssl/pull/5427. Key dispute question is
>>>> whether or not following fragment should work
>>>>
>>>>      unsigned char *inp = buf, *out = buf;
>>>>
>>>>      for (i = 0; i < sizeof(buf); i++) {
>>>>          EVP_EncryptUpdate(ctx, out, &outl, inp++, 1);
>>>> 	out += outl;
>>>>      }
>>>
>>> This should work.
>>>
>>
>> It does only work, if you know that ctx->buf_len == 0
>> before the loop is entered.
> 
> It's naturally implied that ctx is used *consistently*. I mean it's not
> like it's suggested that you can just use the above snippet in random
> place. Application would have to adhere to above pattern all along, for
> the life time of in-place processing "session". [I write "session" in
> quotes, because there might be better term.]
> 
>> It does not work with CFB1, CCM, XTS and WRAP modes.
> 
> Yes, some modes are so to say "one-shot", i.e. you have to pass all the
> data there is at once, no increments are allowed. But what does it have
> to do with question at hand, question about in-place processing that is?
> These two things are independent. So that question is rather should the
> snippet work in cases when incremental processing is an option. Related
> thing to recognize in the context is that *disputable* condition, the
> one that triggered this discussion, is exercised only when
> ctx->block_size is larger than 1, because then ctx->buf_len remains 0.
> Note that ctx->block_size for CFB1, CCM and XTS is 1. As for WRAP, yeah,
> it's special...
> 
>> There is no access function to get the internal state of the cipher
>> context.
> 
> But there is notion of in-place processing "session".
> 

Well, I'd say, apparently the consensus is at least not to restrict
what is currently recognized as "fully" overlapping.

All depends obviously on how "partially" overlapping is defined:
a) by humans
b) by OpenSSL

a)
I think humans would normally say two objects are partially overlapping
when they overlap, but do not fully overlap, in other words partially
overlapping in and out do have some common bytes, but do not start at
the same address, thus in != out.

b)
What is currently recognized as partially overlapping in the evp_enc.c
is a) but adds some I would say ad-hoc exceptions to accommodate one
special use case with certain block ciphers.  Yet it does for instance
artificially restrict use cases where other ciphers work just fine.

I think for instance of WRAP mode, where the primitive basically
supports arbitrary overlapping (uses memmove) but we can no longer
use that feature because of the partially overlapping check in the
EVP cipher handling.

And the other use case, which I think is worth to mention is

unsigned char *inp = buf + sizeof(buf) - 1, *out = buf;
for (i = 0; i<sizeof(buf)-X; i++) {
     *inp = getc();
     EVP_EncryptUpdate(ctx, out, &outl, inp, 1);
     out += outl;
}

This could work for some cipher modes at least.

And I think our overall assumption here is the user knows perfectly
well how the internal state of the ctx is at any time, otherwise
EVP_EncryptUpdate would probably need to have an input parameter to
tell how large the output buffer is in reality :)

So I think maybe we can just change the definition for b) to be

Partially overlapping is any kind of overlapping that is known to be
not working in the evp cipher update function.
It is dependent on the cipher mode, and dependent on the internal state
of the cipher context.

It is often the case that partially overlapping means overlapping
with in != out but most importantly the code in evp_enc.c is known
to fail if it was allowed to continue after EVP_R_PARTIALLY_OVERLAPPING.


Bernd.