Re: [PATCH 00/13] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites

[Laszlo Ersek <lersek@redhat.com>]

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.

- 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.)

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

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

Thanks!
Laszlo