On 03/24/16 17:03, James Bottomley wrote:
> On Thu, 2016-03-24 at 16:51 +0100, Laszlo Ersek wrote:

>> Either way, I think all flash sizes are finite in practice, so we
>> shouldn't be trying to increase PcdMaxAuthVariableSize specifically 
>> for dbx's sake. For enrolling whitelist-like stuff (hashes and
>> certificates in db / kek), I agree we might have to increase the 
>> size; I'm just not sure we need this much. If the current 8K are 
>> enough for 6 reasonably sized X509 certificates (average cert size: 
>> 1171 to 1365 bytes), then 32KB is presumably enough for 24-27 
>> certificates. Honestly, I find that overkill, and I'm worried that 
>> decreasing this PCD in the future (if anything justified it) would be 
>> much harder than increasing it only gradually now. Can we perhaps do
>> 12KB or 16KB instead?
> 
> I'd be fine with that.  My usecase was actually the putting the current
> certificate store of my UEFI laptop into OVMF to run a test of a patch
> I'm doing to the Pkcs7Verifier.  My current laptop has 7 certificates
> in db, hence the issue (that's 8317)
> 
> dbx currently has 1 signature and 12 hashes, for an overall size of 
> 2343, so I think we're good there.
> 
> The reason for the huge number in dbx

(ITYM "db", not "dbx", here)

> is that I need the system certs
> to boot (thats 2).  I need my own secure key, three opensuse
> certificates for their kernels (they use a variety of keys depending on
> which kernel base you're using; kernels you get from different archives
> have different certificates) and I had an extra temporary key for a
> test ... so 7 in total, meaning I'd be happy with 10KB.

That sounds great, thank you. Your use case is pretty sophisticated, so
I think 10KB should cover common needs going forward.

>> (Honestly, I'm curious why 6 certificates are not enough -- if
>> individual hashes are enrolled in db, I guess that could exhaust the
>> current db size limit, but for real root certificates (where binaries
>> would be tagged with certificate chains), 6-12 should be plenty. Am I
>> wrong?)
>>
>> Also, is this patch well-formed? The triple-minus and the diffstat 
>> seem to be missing between the commit message and the patch.
> 
> it's what git show --pretty=email gives.  I can add --stat --patch if
> you want, which gives what I think you're asking for.

Interesting, I have not bee aware of --pretty=email.

Normally we format patches with "git format-patch", and mail the patch
with "git send-email". My only concern was if "git am" would be able to
apply your patch, formatted as-is. Let me try... Yes, the way you
formatted and posted it works. Okay.

> 
>> Furthermore, the OvmfPkg/OvmfPkgIa32X64.dsc file should be kept in
>> sync.
> 
> Heh, well, I never actually build that, so I didn't think to look in
> there, but if we agree on the sizing, I can certainly add it.

Sounds good!

So let me look at my list... (1) is answered, you will address (2), we
agreed upon the size, 10KB (4).

For (3) and (5), please drop the reference to
PcdMaxHardwareErrorVariableSize from the commit message, and do include
a short summary of your use case above -- I find such descriptions most
valuable when running "git blame" later. Personal or env-specific
details are a plus, not a minus.

Thanks!
Laszlo

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to