Thanks, Laszlo.
In fact, these implementation optimizations are good to me.  ☺

On 04/10/18 12:02, Laszlo Ersek wrote:
> On 04/10/18 09:40, Long, Qin wrote:

>> #0005, #0006, #0007, #0012, #0013:
>>         These implementation looks good to me.
>>         But some of updates were based on the assumption of #0008-0009. I 
>> have no strong opinion
>>         if some original light implementation are good enough currently.

I'd like to comment on this in more detail (namely that "some original
light implementation are good enough currently"):

- I now agree that "TlsCipherMappingTable" should match the ciphers
built into OpensslLib exactly. However, that makes it only more
important that we *not* return EFI_UNSUPPORTED immediately when we find
a cipher suite in the platform's preference list that we don't support.
Instead, we should filter the platform's list down to what we do support.

[qlong] Yes, I agree it’s better to filter out any unavailable items.

- The stack allocation with 500 bytes for CipherString is questionable
practice, in my opinion, given that we add a variable list of cipher
suite names. It's just not deterministic. It can produce confusing
results that don't match the caller's (the platform's) intent, and it
will only become worse when you extend "TlsCipherMappingTable" to the
full cipher list that we build into OpensslLib *right now*. (And that's
not considering any future cipher enablements.)

[qlong] Yes, the original fixed buffer is limited to the future extension.
        It’s good to me to have more flexible implementation.

- "@STRENGTH" must be dropped. I have no doubt about that. :)

[qlong] I agree. “@STRENGTH” will cause to re-order the preferred cipher lists.
          I prefer to keep the configuration-defined order.

So, I'd like to keep patch #13 as-is, perhaps squahed together with
patch #12 if you all prefer that.

[qlong] Sure. It’s OK for me.

edk2-devel mailing list<>
edk2-devel mailing list

Reply via email to