uefi: add EFI_CERT_SHA384_GUID and EfiSignatureDataEfiCertSha384#805
Conversation
EFI_CERT_SHA384_GUID was commented out in EfiSignatureDataFactory with a
malformed C-struct-style UUID string that would be a syntax error if
uncommented. Fix the GUID definition and add full SHA-384 support:
- Introduce EfiSignatureDataEfiCertShaBase (inheriting CommonUefiStructure)
that holds all shared logic for fixed-size hash digest signature data.
Subclasses parameterise the base through two class attributes:
_HASH - hashlib hash constructor (e.g. hashlib.sha256)
_LABEL - algorithm label string used in print() output
- Refactor EfiSignatureDataEfiCertSha256 as a thin subclass of
EfiSignatureDataEfiCertShaBase, retaining its deprecated compatibility
shims (SignatureOwner, SignatureData, PopulateFromFileStream, Print,
Write, GetBytes, GetTotalSize) as they were part of the original public
API. No functional change for existing callers.
- Add EfiSignatureDataEfiCertSha384 as a second thin subclass.
- Uncomment and fix EFI_CERT_SHA384_GUID in EfiSignatureDataFactory,
converting the broken C-struct notation to a proper Python uuid.UUID()
string: "ff3e5307-9fd0-48c9-85f1-8ad56c701e01".
- Wire EFI_CERT_SHA384_GUID into EfiSignatureDataFactory.factory() and
EfiSignatureDataFactory.create() so callers can build and decode
EFI_SIGNATURE_LIST entries of type EFI_CERT_SHA384_GUID without falling
through to the "Not Supported" exception.
The resulting EFI_SIGNATURE_LIST layout for SHA-384 is: 28-byte ESL header
(16-byte type GUID + 3x UINT32) + 64-byte EFI_SIGNATURE_DATA
(16-byte owner GUID + 48-byte digest), matching the UEFI spec §32.4.1.
|
Look's good to me. |
| FIXED_SIZE = True | ||
|
|
||
|
|
||
| class EfiSignatureDataEfiCertSha384(EfiSignatureDataEfiCertShaBase): |
There was a problem hiding this comment.
Can you please add tests for EfiSignatureDataEfiCertSha384 in tests.unit/test_authenticated_variables_structure_support.py?
| class EfiSignatureDataEfiCertShaBase(CommonUefiStructure): | ||
| """Base class for EFI_SIGNATURE_DATA structures backed by a fixed-size hash digest. | ||
|
|
||
| Subclasses must define: | ||
| _HASH (hashlib hash constructor): e.g. hashlib.sha256 or hashlib.sha384 | ||
| STATIC_STRUCT_SIZE (int): 16 (owner GUID) + digest_size | ||
| FIXED_SIZE (bool): True | ||
| _LABEL (str): human-readable algorithm name used in print(), e.g. "EFI_CERT_SHA256" | ||
| """ | ||
|
|
||
| STATIC_STRUCT_SIZE = 16 + hashlib.sha256().digest_size # has guid and array | ||
| FIXED_SIZE = True | ||
| _HASH = None # must be overridden by subclass | ||
| _LABEL = None # must be overridden by subclass | ||
|
|
There was a problem hiding this comment.
Have you considered letting STATIC_STRUCT_SIZE and FIXED_SIZE be derived automatically with __init_subclass__()? It's less work in the subclasses but also makes them only need to define the more relevant data which is the hash and label.
| class EfiSignatureDataEfiCertShaBase(CommonUefiStructure): | |
| """Base class for EFI_SIGNATURE_DATA structures backed by a fixed-size hash digest. | |
| Subclasses must define: | |
| _HASH (hashlib hash constructor): e.g. hashlib.sha256 or hashlib.sha384 | |
| STATIC_STRUCT_SIZE (int): 16 (owner GUID) + digest_size | |
| FIXED_SIZE (bool): True | |
| _LABEL (str): human-readable algorithm name used in print(), e.g. "EFI_CERT_SHA256" | |
| """ | |
| STATIC_STRUCT_SIZE = 16 + hashlib.sha256().digest_size # has guid and array | |
| FIXED_SIZE = True | |
| _HASH = None # must be overridden by subclass | |
| _LABEL = None # must be overridden by subclass | |
| class EfiSignatureDataEfiCertShaBase(CommonUefiStructure): | |
| """Base class for EFI_SIGNATURE_DATA structures backed by a fixed-size hash digest. | |
| Subclasses only need to define: | |
| _HASH (hashlib hash constructor): e.g. hashlib.sha256 or hashlib.sha384 | |
| _LABEL (str): human-readable algorithm name used in print(), e.g. "EFI_CERT_SHA256" | |
| """ | |
| _HASH = None # must be overridden by subclass | |
| _LABEL = None # must be overridden by subclass | |
| FIXED_SIZE = True | |
| def __init_subclass__(cls, **kwargs) -> None: | |
| """Derives per-algorithm constants from a given _HASH.""" | |
| super().__init_subclass__(**kwargs) | |
| if cls._HASH is None: | |
| raise TypeError(f"{cls.__name__} must define a '_HASH' hashlib constructor") | |
| if cls._LABEL is None: | |
| raise TypeError(f"{cls.__name__} must define a '_LABEL' string") | |
| cls.STATIC_STRUCT_SIZE = 16 + cls._HASH().digest_size |
Then, I think a subclass just needs to be:
class EfiSignatureDataEfiCertSha256(EfiSignatureDataEfiCertShaBase):
"""An object representing a EFI_SIGNATURE_DATA Structure for Sha256 Certs."""
_HASH = hashlib.sha256
_LABEL = "EFI_CERT_SHA256"| def factory( | ||
| fs: BinaryIO, type: uuid.UUID, size: int | ||
| ) -> Union[EfiSignatureDataEfiCertSha256, EfiSignatureDataEfiCertX509]: |
There was a problem hiding this comment.
This should probably not be hardcoded to EfiSignatureDataEfiCertSha256.
Could you please look into updating this to EfiSignatureDataEfiCertShaBase?
Can you also check other locations in the file where this needs to be updated? Like this method below:
@staticmethod
def Factory(
fs: BinaryIO, type: uuid.UUID, size: int
) -> Union[EfiSignatureDataEfiCertSha256, EfiSignatureDataEfiCertX509]:
"""This method is a factory for creating the correct Efi Signature Data object from the filestream.
Uses a Filestream of an existing auth payload.
Args:
fs: filestream to read
type: Guid of the type
size: decodesize for x509, struct size for Sha256
"""| Args: | ||
| fs: filestream to read | ||
| type: Guid of the type | ||
| size: decodesize for x509, struct size for Sha256 |
There was a problem hiding this comment.
| size: decodesize for x509, struct size for fixed-size hash types (e.g. Sha256, Sha384) |
EFI_CERT_SHA384_GUID was commented out in EfiSignatureDataFactory with a malformed C-struct-style UUID string that would be a syntax error if uncommented. Fix the GUID definition and add full SHA-384 support:
The resulting EFI_SIGNATURE_LIST layout for SHA-384 is: 28-byte ESL header (16-byte type GUID + 3x UINT32) + 64-byte EFI_SIGNATURE_DATA (16-byte owner GUID + 48-byte digest), matching the UEFI spec §32.4.1.