* [PATCH V2 0/4] Add SPDM device security
@ 2019-10-31 12:30 Yao, Jiewen
2019-10-31 12:30 ` [PATCH V2 1/4] MdePkg/Include: Add DMTF SPDM definition Yao, Jiewen
` (7 more replies)
0 siblings, 8 replies; 20+ messages in thread
From: Yao, Jiewen @ 2019-10-31 12:30 UTC (permalink / raw)
To: devel
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303
This patch series add support for device security based
upon the DMTF SPDM specification.
https://www.dmtf.org/sites/default/files/standards/documents/DSP0274_0.95a.zip
We did design review at 18 Oct, 2019.
https://edk2.groups.io/g/devel/files/Designs/2019/1018
And the feedback from the meeting is addressed.
https://edk2.groups.io/g/devel/files/Designs/2019/1018/EDKII-Device%20Firmware%20Security%20v2.pdf
We add the Device security protocol in EDKII repo.
PCI bus driver consumes the interface.
If there is no producer, the PCI bus driver keeps current behavior.
So far, we only provide the producer what follows Intel
PCI security spec.
https://www.intel.com/content/www/us/en/io/pci-express/pcie-device-security-enhancements-spec.html
The implementation is put to EDKII platform repo.
The EDKII repo update is at https://github.com/jyao1/edk2/tree/DeviceSecurityMasterV2
The EDKII platform repo update is at https://github.com/jyao1/edk2-platforms/tree/DeviceSecurityMasterV2
The validation has been done on a Intel internal platform.
The device measurement can be shown in TCG event log.
signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
Jiewen Yao (4):
MdePkg/Include: Add DMTF SPDM definition.
MdeModulePkg/Include: Add DeviceSecurity.h
MdeModulePkg/dec: Add EdkiiDeviceSecurityProtocolGuid.
MdeModulePkg/Pci: Add DeviceSecurity support.
MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c | 12 +-
MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 +
MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 4 +-
.../Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 63 +++++-
MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c | 4 +-
.../Include/Protocol/DeviceSecurity.h | 162 ++++++++++++++
MdeModulePkg/MdeModulePkg.dec | 5 +
MdePkg/Include/IndustryStandard/Spdm.h | 203 ++++++++++++++++++
8 files changed, 447 insertions(+), 7 deletions(-)
create mode 100644 MdeModulePkg/Include/Protocol/DeviceSecurity.h
create mode 100644 MdePkg/Include/IndustryStandard/Spdm.h
--
2.19.2.windows.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH V2 1/4] MdePkg/Include: Add DMTF SPDM definition.
2019-10-31 12:30 [PATCH V2 0/4] Add SPDM device security Yao, Jiewen
@ 2019-10-31 12:30 ` Yao, Jiewen
2019-11-06 14:38 ` Liming Gao
2019-10-31 12:30 ` [PATCH V2 2/4] MdeModulePkg/Include: Add DeviceSecurity.h Yao, Jiewen
` (6 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Yao, Jiewen @ 2019-10-31 12:30 UTC (permalink / raw)
To: devel; +Cc: Michael D Kinney, Liming Gao, Yun Lou
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Yun Lou <yun.lou@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
MdePkg/Include/IndustryStandard/Spdm.h | 203 ++++++++++++++++++++
1 file changed, 203 insertions(+)
diff --git a/MdePkg/Include/IndustryStandard/Spdm.h b/MdePkg/Include/IndustryStandard/Spdm.h
new file mode 100644
index 0000000000..d62b24e9ef
--- /dev/null
+++ b/MdePkg/Include/IndustryStandard/Spdm.h
@@ -0,0 +1,203 @@
+/** @file
+ Definitions of Security Protocol & Data Model Specification (SPDM)
+ in Distributed Management Task Force (DMTF).
+
+Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+
+#ifndef __SPDM_H__
+#define __SPDM_H__
+
+#pragma pack(1)
+
+#define SPDM_DIGESTS 0x01
+#define SPDM_CERTIFICATE 0x02
+#define SPDM_CHALLENGE_AUTH 0x03
+#define SPDM_MEASUREMENTS 0x60
+#define SPDM_CAPABILITIES 0x61
+#define SPDM_SET_CERT_RESPONSE 0x62
+#define SPDM_ALGORITHMS 0x63
+#define SPDM_ERROR 0x7F
+#define SPDM_GET_DIGESTS 0x81
+#define SPDM_GET_CERTIFICATE 0x82
+#define SPDM_CHALLENGE 0x83
+#define SPDM_GET_MEASUREMENTS 0xE0
+#define SPDM_GET_CAPABILITIES 0xE1
+#define SPDM_SET_CERTIFICATE 0xE2
+#define SPDM_NEGOTIATE_ALGORITHMS 0xE3
+#define SPDM_RESPOND_IF_READY 0xFF
+
+typedef struct {
+ UINT8 SPDMVersion;
+ UINT8 RequestResponseCode;
+ UINT8 Param1;
+ UINT8 Param2;
+} SPDM_MESSAGE_HEADER;
+
+#define SPDM_VERSION 0x10
+
+typedef struct {
+ SPDM_MESSAGE_HEADER Header;
+} SPDM_GET_CAPABILITIES_REQUEST;
+
+typedef struct {
+ SPDM_MESSAGE_HEADER Header;
+ UINT8 DetailedVersion;
+ UINT8 CryptographicTimeout;
+ UINT16 Reserved;
+ UINT32 Flags;
+ UINT16 SPDMMajorVersions;
+ UINT16 Reserved2;
+} SPDM_CAPABILITIES_RESPONSE;
+
+#define SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_AUTH_CAP BIT1
+#define SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_MEAS_CAP BIT3
+#define SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_MEAS_FRESH_CAP BIT4
+
+typedef struct {
+ SPDM_MESSAGE_HEADER Header;
+ UINT16 Length;
+ UINT8 MeasurementSpecification;
+ UINT8 Reserved;
+ UINT32 BaseAsymAlgo;
+ UINT32 BaseHashAlgo;
+ UINT64 Reserved2;
+ UINT8 ExtAsymCount;
+ UINT8 ExtHashCount;
+ UINT16 Reserved3;
+//UINT32 ExtAsym[ExtAsymCount];
+//UINT32 ExtHash[ExtHashCount];
+} SPDM_NEGOTIATE_ALGORITHMS_REQUEST;
+
+#define SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_RSASSA_2048 BIT0
+#define SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_RSASSA_3072 BIT1
+#define SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_ECDSA_ECC_NIST_P256 BIT2
+#define SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_RSASSA_4096 BIT3
+#define SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_ECDSA_ECC_NIST_P384 BIT4
+#define SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_ECDSA_ECC_NIST_P521 BIT5
+
+#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA2_256 BIT0
+#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA3_256 BIT1
+#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA2_384 BIT2
+#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA3_384 BIT3
+#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA2_512 BIT4
+#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA3_512 BIT5
+
+typedef struct {
+ SPDM_MESSAGE_HEADER Header;
+ UINT16 Length;
+ UINT8 MeasurementSpecification;
+ UINT8 MeasurementHashAlgo;
+ UINT32 BaseAsymSel;
+ UINT32 BaseHashSel;
+ UINT64 Reserved;
+ UINT8 ExtAsymSelCount;
+ UINT8 ExtHashSelCount;
+ UINT16 Reserved2;
+//UINT32 ExtAsymSel[ExtAsymSelCount];
+//UINT32 ExtHashSel[ExtHashSelCount];
+} SPDM_ALGORITHMS_RESPONSE;
+
+typedef struct {
+ SPDM_MESSAGE_HEADER Header;
+} SPDM_GET_DIGESTS_REQUEST;
+
+typedef struct {
+ SPDM_MESSAGE_HEADER Header;
+//UINT8 Digest[DigestSize];
+} SPDM_DIGESTS_RESPONSE;
+
+typedef struct {
+ SPDM_MESSAGE_HEADER Header;
+ UINT16 Offset;
+ UINT16 Length;
+} SPDM_GET_CERTIFICATE_REQUEST;
+
+typedef struct {
+ SPDM_MESSAGE_HEADER Header;
+//UINT8 CertChain[CertChainSize];
+} SPDM_CERTIFICATE_RESPONSE;
+
+typedef struct {
+ SPDM_MESSAGE_HEADER Header;
+//UINT8 Nonce[DigestSize];
+} SPDM_CHALLENGE_REQUEST;
+
+typedef struct {
+ SPDM_MESSAGE_HEADER Header;
+ UINT8 MinSPDMVersion;
+ UINT8 MaxSPDMVersion;
+ UINT8 Capabilities;
+ UINT8 Reserved;
+//UINT8 CertChainHash[DigestSize];
+//UINT8 Salt[DigestSize];
+//UINT8 ContextHash[DigestSize];
+ //
+ // M1 = Concatenate(
+ // GET_CAPABILITIES_REQUEST1, CAPABILITIES_RESPONSE1,
+ // NEGOTIATE_ALGORITHMS_REQUEST1, ALGORITHMS_RESPONSE1, CHALLENGE_REQUEST1,
+ // CHALLENGE_AUTH_RESPONSE_WITHOUT_SIGNATURE1)
+ // Signature = Sign(SK, Hash1(M1))
+ //
+//UINT8 Signature[KeySize];
+} SPDM_CHALLENGE_AUTH_RESPONSE;
+
+typedef struct {
+ SPDM_MESSAGE_HEADER Header;
+ // Param1 == Request Type
+ // Param2 == Measurement Index (0xFF == all)
+//UINT8 Nonce[DigestSize];
+} SPDM_GET_MEASUREMENTS_REQUEST;
+
+typedef struct {
+ UINT8 Index;
+ UINT8 MeasurementType;
+ UINT8 MeasurementSpecification;
+ UINT8 Reserved;
+} SPDM_MEASUREMENT_BLOCK_HEADER;
+
+#define SPDM_MEASUREMENT_BLOCK_MEASUREMENT_TYPE_IMMUTABLE_ROM 1
+#define SPDM_MEASUREMENT_BLOCK_MEASUREMENT_TYPE_MUTABLE_FIRMWARE 2
+#define SPDM_MEASUREMENT_BLOCK_MEASUREMENT_TYPE_HARDWARE_CONFIGURATION 3
+#define SPDM_MEASUREMENT_BLOCK_MEASUREMENT_TYPE_FIRMWARE_CONFIGURATION 4
+
+typedef struct {
+ SPDM_MESSAGE_HEADER Header;
+ UINT8 NumberOfBlocks;
+//SPDM_MEASUREMENT_BLOCK_STRUCT MeasurementRecord[NumberOfBlocks];
+//UINT8 Salt[DigestSize];
+//UINT8 ContextHash[DigestSize];
+ //
+ // L1 = Concatenate(
+ // GET_MEASUREMENTS_REQUEST1, MEASUREMENTS_RESPONSE_WITHOUT_SIGNATURE1)
+ // Signature = Sign(SK, Hash1(L1))
+ //
+//UINT8 Signature[KeySize];
+} SPDM_MEASUREMENTS_RESPONSE;
+
+typedef struct {
+ SPDM_MESSAGE_HEADER Header;
+ // Param1 == Error Code
+ // Param2 == Error Data
+//UINT8 ExtendedErrorData[];
+} SPDM_ERROR_RESPONSE;
+
+#define SPDM_ERROR_CODE_INVALID_REQUEST 0x01
+#define SPDM_ERROR_CODE_BUSY 0x03
+#define SPDM_ERROR_CODE_UNEXPECTED_REQUEST 0x04
+#define SPDM_ERROR_CODE_UNINITIALIZED 0x05
+#define SPDM_ERROR_CODE_REQUESTED_INFO_TOO_LONG 0x40
+#define SPDM_ERROR_CODE_MAJOR_VERSION_MISMATCH 0x41
+#define SPDM_ERROR_CODE_RESPONSE_NOT_READY 0x42
+
+typedef struct {
+ SPDM_MESSAGE_HEADER Header;
+} SPDM_RESPONSE_IF_READY_REQUEST;
+
+#pragma pack()
+
+#endif
+
--
2.19.2.windows.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH V2 2/4] MdeModulePkg/Include: Add DeviceSecurity.h
2019-10-31 12:30 [PATCH V2 0/4] Add SPDM device security Yao, Jiewen
2019-10-31 12:30 ` [PATCH V2 1/4] MdePkg/Include: Add DMTF SPDM definition Yao, Jiewen
@ 2019-10-31 12:30 ` Yao, Jiewen
2019-11-06 7:55 ` [edk2-devel] " Ni, Ray
2019-10-31 12:30 ` [PATCH V2 3/4] MdeModulePkg/dec: Add EdkiiDeviceSecurityProtocolGuid Yao, Jiewen
` (5 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Yao, Jiewen @ 2019-10-31 12:30 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Hao A Wu, Yun Lou
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303
EDKII_DEVICE_SECURITY_PROTOCOL is used for device
measurement and/or authentication.
It is similar to EFI_SECURITY_ARCH_PROTOCOL.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Yun Lou <yun.lou@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
MdeModulePkg/Include/Protocol/DeviceSecurity.h | 162 ++++++++++++++++++++
1 file changed, 162 insertions(+)
diff --git a/MdeModulePkg/Include/Protocol/DeviceSecurity.h b/MdeModulePkg/Include/Protocol/DeviceSecurity.h
new file mode 100644
index 0000000000..c3bf624cac
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/DeviceSecurity.h
@@ -0,0 +1,162 @@
+/** @file
+ Device Security Protocol definition.
+
+ It is used to authenticate a device based upon the platform policy.
+ It is similar to the EFI_SECURITY_ARCH_PROTOCOL, which is used to verify a image.
+
+Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+
+#ifndef __DEVICE_SECURITY_H__
+#define __DEVICE_SECURITY_H__
+
+//
+// Device Security Protocol GUID value
+//
+#define EDKII_DEVICE_SECURITY_PROTOCOL_GUID \
+ { \
+ 0x5d6b38c8, 0x5510, 0x4458, { 0xb4, 0x8d, 0x95, 0x81, 0xcf, 0xa7, 0xb0, 0xd } \
+ }
+
+//
+// Forward reference for pure ANSI compatability
+//
+typedef struct _EDKII_DEVICE_SECURITY_PROTOCOL EDKII_DEVICE_SECURITY_PROTOCOL;
+
+//
+// Revision The revision to which the DEVICE_SECURITY interface adheres.
+// All future revisions must be backwards compatible.
+// If a future version is not back wards compatible it is not the same GUID.
+//
+#define EDKII_DEVICE_SECURITY_PROTOCOL_REVISION 0x00010000
+
+//
+// The device identifier.
+//
+typedef struct {
+ ///
+ /// Version of this data structure.
+ ///
+ UINT32 Version;
+ ///
+ /// Type of the device.
+ /// This field is also served as a device Access protocol GUID.
+ /// The device access protocol is installed on the DeviceHandle.
+ /// The device access protocol is device specific.
+ /// EDKII_DEVICE_IDENTIFIER_TYPE_PCI_GUID means the device access protocol is PciIo.
+ /// EDKII_DEVICE_IDENTIFIER_TYPE_USB_GUID means the device access protocol is UsbIo.
+ ///
+ EFI_GUID DeviceType;
+ ///
+ /// The handle created for this device.
+ /// NOTE: This might be a temporary handle.
+ /// If the device is not authenticated, this handle shall be uninstalled.
+ ///
+ /// As minimal requirement, there should be 2 protocols installed on the device handle.
+ /// 1) An EFI_DEVICE_PATH_PROTOCOL with EFI_DEVICE_PATH_PROTOCOL_GUID.
+ /// 2) A device access protocol with EDKII_DEVICE_IDENTIFIER_TYPE_xxx_GUID.
+ /// If the device is PCI device, the EFI_PCI_IO_PROTOCOL is installed with
+ /// EDKII_DEVICE_IDENTIFIER_TYPE_PCI_GUID.
+ /// If the device is USB device, the EFI_USB_IO_PROTOCOL is installed with
+ /// EDKII_DEVICE_IDENTIFIER_TYPE_USB_GUID.
+ ///
+ /// The device access protocol is required, because the verifier need have a way
+ /// to communciate with the device hardware to get the measurement or do the
+ /// challenge/response for the device authentication.
+ ///
+ /// NOTE: We don't use EFI_PCI_IO_PROTOCOL_GUID or EFI_USB_IO_PROTOCOL_GUID here,
+ /// because we don't want to expose a real protocol. A platform may have driver
+ /// register a protocol notify function. Installing a real protocol may cause
+ /// the callback function being executed before the device is authenticated.
+ ///
+ EFI_HANDLE DeviceHandle;
+} EDKII_DEVICE_IDENTIFIER;
+
+//
+// Revision The revision to which the DEVICE_IDENTIFIER interface adheres.
+// All future revisions must be backwards compatible.
+//
+#define EDKII_DEVICE_IDENTIFIER_REVISION 0x00010000
+
+//
+// Device Identifier GUID value
+//
+#define EDKII_DEVICE_IDENTIFIER_TYPE_PCI_GUID \
+ { \
+ 0x2509b2f1, 0xa022, 0x4cca, { 0xaf, 0x70, 0xf9, 0xd3, 0x21, 0xfb, 0x66, 0x49 } \
+ }
+
+#define EDKII_DEVICE_IDENTIFIER_TYPE_USB_GUID \
+ { \
+ 0x7394f350, 0x394d, 0x488c, { 0xbb, 0x75, 0xc, 0xab, 0x7b, 0x12, 0xa, 0xc5 } \
+ }
+
+/**
+ The device driver uses this service to measure and/or verify a device.
+
+ The flow in device driver is:
+ 1) Device driver discovers a new device.
+ 2) Device driver creates an EFI_DEVICE_PATH_PROTOCOL.
+ 3) Device driver creates a device access protocol. e.g.
+ EFI_PCI_IO_PROTOCOL for PCI device.
+ EFI_USB_IO_PROTOCOL for USB device.
+ EFI_EXT_SCSI_PASS_THRU_PROTOCOL for SCSI device.
+ EFI_ATA_PASS_THRU_PROTOCOL for ATA device.
+ EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL for NVMe device.
+ EFI_SD_MMC_PASS_THRU_PROTOCOL for SD/MMC device.
+ 4) Device driver installs the EFI_DEVICE_PATH_PROTOCOL with EFI_DEVICE_PATH_PROTOCOL_GUID,
+ and the device access protocol with EDKII_DEVICE_IDENTIFIER_TYPE_xxx_GUID.
+ Once it is done, a DeviceHandle is returned.
+ 5) Device driver creates EDKII_DEVICE_IDENTIFIER with EDKII_DEVICE_IDENTIFIER_TYPE_xxx_GUID
+ and the DeviceHandle.
+ 6) Device driver calls DeviceAuthenticate().
+ 7) If DeviceAuthenticate() returns EFI_SECURITY_VIOLATION, the device driver uninstalls
+ all protocols on this handle.
+ 8) If DeviceAuthenticate() returns EFI_SUCCESS, the device driver installs the device access
+ protocol with a real protocol GUID. e.g.
+ EFI_PCI_IO_PROTOCOL with EFI_PCI_IO_PROTOCOL_GUID.
+ EFI_USB_IO_PROTOCOL with EFI_USB_IO_PROTOCOL_GUID.
+
+ @param[in] This The protocol instance pointer.
+ @param[in] DeviceId The Identifier for the device.
+
+ @retval EFI_SUCCESS The device specified by the DeviceId passed the measurement
+ and/or authentication based upon the platform policy.
+ If TCG measurement is required, the measurement is extended to TPM PCR.
+ @retval EFI_SECURITY_VIOLATION The device fails to return the measurement data.
+ @retval EFI_SECURITY_VIOLATION The device fails to response the authentication request.
+ @retval EFI_SECURITY_VIOLATION The system fails to verify the device based upon the authentication response.
+ @retval EFI_SECURITY_VIOLATION The system fails to extend the measurement to TPM PCR.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_DEVICE_AUTHENTICATE)(
+ IN EDKII_DEVICE_SECURITY_PROTOCOL *This,
+ IN EDKII_DEVICE_IDENTIFIER *DeviceId
+ );
+
+///
+/// Device Security Protocol structure.
+/// It is similar to the EFI_SECURITY_ARCH_PROTOCOL, which is used to verify a image.
+/// This protocol is used to authenticate a device based upon the platform policy.
+///
+struct _EDKII_DEVICE_SECURITY_PROTOCOL {
+ UINT64 Revision;
+ EDKII_DEVICE_AUTHENTICATE DeviceAuthenticate;
+};
+
+///
+/// Device Security Protocol GUID variable.
+///
+extern EFI_GUID gEdkiiDeviceSecurityProtocolGuid;
+
+///
+/// Device Identifier tpye GUID variable.
+///
+extern EFI_GUID gEdkiiDeviceIdentifierTypePciGuid;
+extern EFI_GUID gEdkiiDeviceIdentifierTypeUsbGuid;
+
+#endif
--
2.19.2.windows.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH V2 3/4] MdeModulePkg/dec: Add EdkiiDeviceSecurityProtocolGuid.
2019-10-31 12:30 [PATCH V2 0/4] Add SPDM device security Yao, Jiewen
2019-10-31 12:30 ` [PATCH V2 1/4] MdePkg/Include: Add DMTF SPDM definition Yao, Jiewen
2019-10-31 12:30 ` [PATCH V2 2/4] MdeModulePkg/Include: Add DeviceSecurity.h Yao, Jiewen
@ 2019-10-31 12:30 ` Yao, Jiewen
2019-10-31 12:30 ` [PATCH V2 4/4] MdeModulePkg/Pci: Add DeviceSecurity support Yao, Jiewen
` (4 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: Yao, Jiewen @ 2019-10-31 12:30 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Hao A Wu, Yun Lou
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Yun Lou <yun.lou@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
MdeModulePkg/MdeModulePkg.dec | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index d6bac974da..b7356aa4ed 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -584,6 +584,11 @@
## Include/Protocol/IoMmu.h
gEdkiiIoMmuProtocolGuid = { 0x4e939de9, 0xd948, 0x4b0f, { 0x88, 0xed, 0xe6, 0xe1, 0xce, 0x51, 0x7c, 0x1e } }
+ ## Include/Protocol/DeviceSecurity.h
+ gEdkiiDeviceSecurityProtocolGuid = { 0x5d6b38c8, 0x5510, 0x4458, { 0xb4, 0x8d, 0x95, 0x81, 0xcf, 0xa7, 0xb0, 0xd } }
+ gEdkiiDeviceIdentifierTypePciGuid = { 0x2509b2f1, 0xa022, 0x4cca, { 0xaf, 0x70, 0xf9, 0xd3, 0x21, 0xfb, 0x66, 0x49 } }
+ gEdkiiDeviceIdentifierTypeUsbGuid = { 0x7394f350, 0x394d, 0x488c, { 0xbb, 0x75, 0xc, 0xab, 0x7b, 0x12, 0xa, 0xc5 } }
+
## Include/Protocol/SmmMemoryAttribute.h
gEdkiiSmmMemoryAttributeProtocolGuid = { 0x69b792ea, 0x39ce, 0x402d, { 0xa2, 0xa6, 0xf7, 0x21, 0xde, 0x35, 0x1d, 0xfe } }
--
2.19.2.windows.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH V2 4/4] MdeModulePkg/Pci: Add DeviceSecurity support.
2019-10-31 12:30 [PATCH V2 0/4] Add SPDM device security Yao, Jiewen
` (2 preceding siblings ...)
2019-10-31 12:30 ` [PATCH V2 3/4] MdeModulePkg/dec: Add EdkiiDeviceSecurityProtocolGuid Yao, Jiewen
@ 2019-10-31 12:30 ` Yao, Jiewen
2019-11-07 4:42 ` Ni, Ray
[not found] ` <15D2BB2D773DBDBA.23805@groups.io>
` (3 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Yao, Jiewen @ 2019-10-31 12:30 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Hao A Wu, Ray Ni, Yun Lou
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303
Whenever a PCI device is discovered, PCI bus calls the
EDKII_DEVICE_SECURITY_PROTOCOL to authenticate it.
If the function returns success, the PCI bus allocates
the resource and installs the PCI_IO for the device.
If the function returns fail, the PCI bus skips the device.
It is similar to EFI_SECURITY_ARCH_PROTOCOL, which
is used to verify an EFI image.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Yun Lou <yun.lou@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c | 12 +++-
MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 +
MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 4 +-
MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 63 +++++++++++++++++++-
MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c | 4 +-
5 files changed, 77 insertions(+), 7 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
index b020ce50ce..64284ac825 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
@@ -8,7 +8,7 @@
PCI Root Bridges. So it means platform needs install PCI Root Bridge IO protocol for each
PCI Root Bus and install PCI Host Bridge Resource Allocation Protocol.
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -37,7 +37,7 @@ UINT64 gAllZero = 0;
EFI_PCI_PLATFORM_PROTOCOL *gPciPlatformProtocol;
EFI_PCI_OVERRIDE_PROTOCOL *gPciOverrideProtocol;
EDKII_IOMMU_PROTOCOL *mIoMmuProtocol;
-
+EDKII_DEVICE_SECURITY_PROTOCOL *mDeviceSecurityProtocol;
GLOBAL_REMOVE_IF_UNREFERENCED EFI_PCI_HOTPLUG_REQUEST_PROTOCOL mPciHotPlugRequest = {
PciHotPlugRequestNotify
@@ -293,6 +293,14 @@ PciBusDriverBindingStart (
);
}
+ if (mDeviceSecurityProtocol == NULL) {
+ gBS->LocateProtocol (
+ &gEdkiiDeviceSecurityProtocolGuid,
+ NULL,
+ (VOID **) &mDeviceSecurityProtocol
+ );
+ }
+
if (PcdGetBool (PcdPciDisableBusEnumeration)) {
gFullEnumeration = FALSE;
} else {
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
index 504a1b1c12..d4113993c8 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
@@ -27,6 +27,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Protocol/PciOverride.h>
#include <Protocol/PciEnumerationComplete.h>
#include <Protocol/IoMmu.h>
+#include <Protocol/DeviceSecurity.h>
#include <Library/DebugLib.h>
#include <Library/UefiDriverEntryPoint.h>
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
index 05c22025b8..9284998f36 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
@@ -2,7 +2,7 @@
# The PCI bus driver will probe all PCI devices and allocate MMIO and IO space for these devices.
# Please use PCD feature flag PcdPciBusHotplugDeviceSupport to enable hot plug supporting.
#
-# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -90,6 +90,8 @@
gEfiIncompatiblePciDeviceSupportProtocolGuid ## SOMETIMES_CONSUMES
gEfiLoadFile2ProtocolGuid ## SOMETIMES_PRODUCES
gEdkiiIoMmuProtocolGuid ## SOMETIMES_CONSUMES
+ gEdkiiDeviceSecurityProtocolGuid ## SOMETIMES_CONSUMES
+ gEdkiiDeviceIdentifierTypePciGuid ## SOMETIMES_CONSUMES
gEfiLoadedImageDevicePathProtocolGuid ## CONSUMES
[FeaturePcd]
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
index c7eafff593..df3d1c8fcc 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
@@ -10,6 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include "PciBus.h"
extern CHAR16 *mBarTypeStr[];
+extern EDKII_DEVICE_SECURITY_PROTOCOL *mDeviceSecurityProtocol;
#define OLD_ALIGN 0xFFFFFFFFFFFFFFFFULL
#define EVEN_ALIGN 0xFFFFFFFFFFFFFFFEULL
@@ -2092,9 +2093,10 @@ CreatePciIoDevice (
IN UINT8 Func
)
{
- PCI_IO_DEVICE *PciIoDevice;
- EFI_PCI_IO_PROTOCOL *PciIo;
- EFI_STATUS Status;
+ PCI_IO_DEVICE *PciIoDevice;
+ EFI_PCI_IO_PROTOCOL *PciIo;
+ EFI_STATUS Status;
+ EDKII_DEVICE_IDENTIFIER DeviceIdentifier;
PciIoDevice = AllocateZeroPool (sizeof (PCI_IO_DEVICE));
if (PciIoDevice == NULL) {
@@ -2156,6 +2158,61 @@ CreatePciIoDevice (
PciIoDevice->IsPciExp = TRUE;
}
+ //
+ // Now we can do the authentication check for the device.
+ //
+ if (mDeviceSecurityProtocol != NULL) {
+ //
+ // Prepare the parameter
+ //
+ DeviceIdentifier.Version = EDKII_DEVICE_IDENTIFIER_REVISION;
+ CopyGuid (&DeviceIdentifier.DeviceType, &gEdkiiDeviceIdentifierTypePciGuid);
+ DeviceIdentifier.DeviceHandle = NULL;
+ Status = gBS->InstallMultipleProtocolInterfaces (
+ &DeviceIdentifier.DeviceHandle,
+ &gEfiDevicePathProtocolGuid,
+ PciIoDevice->DevicePath,
+ &gEdkiiDeviceIdentifierTypePciGuid,
+ &PciIoDevice->PciIo,
+ NULL
+ );
+ if (EFI_ERROR(Status)) {
+ if (PciIoDevice->DevicePath != NULL) {
+ FreePool (PciIoDevice->DevicePath);
+ }
+ FreePool (PciIoDevice);
+ return NULL;
+ }
+
+ //
+ // Do DeviceAuthentication
+ //
+ Status = mDeviceSecurityProtocol->DeviceAuthenticate (mDeviceSecurityProtocol, &DeviceIdentifier);
+ //
+ // Always uninstall, because they are only for Authentication.
+ // No need to check return Status.
+ //
+ gBS->UninstallMultipleProtocolInterfaces (
+ DeviceIdentifier.DeviceHandle,
+ &gEfiDevicePathProtocolGuid,
+ PciIoDevice->DevicePath,
+ &gEdkiiDeviceIdentifierTypePciGuid,
+ &PciIoDevice->PciIo,
+ NULL
+ );
+
+ //
+ // If authentication fails, skip this device.
+ //
+ if (EFI_ERROR(Status)) {
+ if (PciIoDevice->DevicePath != NULL) {
+ FreePool (PciIoDevice->DevicePath);
+ }
+ FreePool (PciIoDevice);
+ return NULL;
+ }
+ }
+
if (PcdGetBool (PcdAriSupport)) {
//
// Check if the device is an ARI device.
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
index 5b55fb5d3b..72690ab647 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
@@ -1054,7 +1054,9 @@ PciScanBus (
&PciDevice
);
- ASSERT (!EFI_ERROR (Status));
+ if (EFI_ERROR (Status)) {
+ continue;
+ }
PciAddress = EFI_PCI_ADDRESS (StartBusNumber, Device, Func, 0);
--
2.19.2.windows.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH V2 1/4] MdePkg/Include: Add DMTF SPDM definition.
[not found] ` <15D2BB2D773DBDBA.23805@groups.io>
@ 2019-11-06 6:47 ` Yao, Jiewen
0 siblings, 0 replies; 20+ messages in thread
From: Yao, Jiewen @ 2019-11-06 6:47 UTC (permalink / raw)
To: devel@edk2.groups.io, Yao, Jiewen
Cc: Kinney, Michael D, Gao, Liming, Lou, Yun
Hi Liming/Michael
Would you please review this patch?
We need this feature in next stable tag as planned.
Thank you
Yao Jiewen
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
> Sent: Thursday, October 31, 2019 8:30 PM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Lou, Yun <yun.lou@intel.com>
> Subject: [edk2-devel] [PATCH V2 1/4] MdePkg/Include: Add DMTF SPDM
> definition.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Yun Lou <yun.lou@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
> MdePkg/Include/IndustryStandard/Spdm.h | 203 ++++++++++++++++++++
> 1 file changed, 203 insertions(+)
>
> diff --git a/MdePkg/Include/IndustryStandard/Spdm.h
> b/MdePkg/Include/IndustryStandard/Spdm.h
> new file mode 100644
> index 0000000000..d62b24e9ef
> --- /dev/null
> +++ b/MdePkg/Include/IndustryStandard/Spdm.h
> @@ -0,0 +1,203 @@
> +/** @file
> + Definitions of Security Protocol & Data Model Specification (SPDM)
> + in Distributed Management Task Force (DMTF).
> +
> +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#ifndef __SPDM_H__
> +#define __SPDM_H__
> +
> +#pragma pack(1)
> +
> +#define SPDM_DIGESTS 0x01
> +#define SPDM_CERTIFICATE 0x02
> +#define SPDM_CHALLENGE_AUTH 0x03
> +#define SPDM_MEASUREMENTS 0x60
> +#define SPDM_CAPABILITIES 0x61
> +#define SPDM_SET_CERT_RESPONSE 0x62
> +#define SPDM_ALGORITHMS 0x63
> +#define SPDM_ERROR 0x7F
> +#define SPDM_GET_DIGESTS 0x81
> +#define SPDM_GET_CERTIFICATE 0x82
> +#define SPDM_CHALLENGE 0x83
> +#define SPDM_GET_MEASUREMENTS 0xE0
> +#define SPDM_GET_CAPABILITIES 0xE1
> +#define SPDM_SET_CERTIFICATE 0xE2
> +#define SPDM_NEGOTIATE_ALGORITHMS 0xE3
> +#define SPDM_RESPOND_IF_READY 0xFF
> +
> +typedef struct {
> + UINT8 SPDMVersion;
> + UINT8 RequestResponseCode;
> + UINT8 Param1;
> + UINT8 Param2;
> +} SPDM_MESSAGE_HEADER;
> +
> +#define SPDM_VERSION 0x10
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> +} SPDM_GET_CAPABILITIES_REQUEST;
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> + UINT8 DetailedVersion;
> + UINT8 CryptographicTimeout;
> + UINT16 Reserved;
> + UINT32 Flags;
> + UINT16 SPDMMajorVersions;
> + UINT16 Reserved2;
> +} SPDM_CAPABILITIES_RESPONSE;
> +
> +#define SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_AUTH_CAP BIT1
> +#define SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_MEAS_CAP BIT3
> +#define SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_MEAS_FRESH_CAP BIT4
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> + UINT16 Length;
> + UINT8 MeasurementSpecification;
> + UINT8 Reserved;
> + UINT32 BaseAsymAlgo;
> + UINT32 BaseHashAlgo;
> + UINT64 Reserved2;
> + UINT8 ExtAsymCount;
> + UINT8 ExtHashCount;
> + UINT16 Reserved3;
> +//UINT32 ExtAsym[ExtAsymCount];
> +//UINT32 ExtHash[ExtHashCount];
> +} SPDM_NEGOTIATE_ALGORITHMS_REQUEST;
> +
> +#define SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_RSASSA_2048
> BIT0
> +#define SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_RSASSA_3072
> BIT1
> +#define
> SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_ECDSA_ECC_NIST_P256
> BIT2
> +#define SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_RSASSA_4096
> BIT3
> +#define
> SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_ECDSA_ECC_NIST_P384
> BIT4
> +#define
> SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_ECDSA_ECC_NIST_P521
> BIT5
> +
> +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA2_256
> BIT0
> +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA3_256
> BIT1
> +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA2_384
> BIT2
> +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA3_384
> BIT3
> +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA2_512
> BIT4
> +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA3_512
> BIT5
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> + UINT16 Length;
> + UINT8 MeasurementSpecification;
> + UINT8 MeasurementHashAlgo;
> + UINT32 BaseAsymSel;
> + UINT32 BaseHashSel;
> + UINT64 Reserved;
> + UINT8 ExtAsymSelCount;
> + UINT8 ExtHashSelCount;
> + UINT16 Reserved2;
> +//UINT32 ExtAsymSel[ExtAsymSelCount];
> +//UINT32 ExtHashSel[ExtHashSelCount];
> +} SPDM_ALGORITHMS_RESPONSE;
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> +} SPDM_GET_DIGESTS_REQUEST;
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> +//UINT8 Digest[DigestSize];
> +} SPDM_DIGESTS_RESPONSE;
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> + UINT16 Offset;
> + UINT16 Length;
> +} SPDM_GET_CERTIFICATE_REQUEST;
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> +//UINT8 CertChain[CertChainSize];
> +} SPDM_CERTIFICATE_RESPONSE;
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> +//UINT8 Nonce[DigestSize];
> +} SPDM_CHALLENGE_REQUEST;
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> + UINT8 MinSPDMVersion;
> + UINT8 MaxSPDMVersion;
> + UINT8 Capabilities;
> + UINT8 Reserved;
> +//UINT8 CertChainHash[DigestSize];
> +//UINT8 Salt[DigestSize];
> +//UINT8 ContextHash[DigestSize];
> + //
> + // M1 = Concatenate(
> + // GET_CAPABILITIES_REQUEST1, CAPABILITIES_RESPONSE1,
> + // NEGOTIATE_ALGORITHMS_REQUEST1, ALGORITHMS_RESPONSE1,
> CHALLENGE_REQUEST1,
> + // CHALLENGE_AUTH_RESPONSE_WITHOUT_SIGNATURE1)
> + // Signature = Sign(SK, Hash1(M1))
> + //
> +//UINT8 Signature[KeySize];
> +} SPDM_CHALLENGE_AUTH_RESPONSE;
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> + // Param1 == Request Type
> + // Param2 == Measurement Index (0xFF == all)
> +//UINT8 Nonce[DigestSize];
> +} SPDM_GET_MEASUREMENTS_REQUEST;
> +
> +typedef struct {
> + UINT8 Index;
> + UINT8 MeasurementType;
> + UINT8 MeasurementSpecification;
> + UINT8 Reserved;
> +} SPDM_MEASUREMENT_BLOCK_HEADER;
> +
> +#define
> SPDM_MEASUREMENT_BLOCK_MEASUREMENT_TYPE_IMMUTABLE_ROM
> 1
> +#define
> SPDM_MEASUREMENT_BLOCK_MEASUREMENT_TYPE_MUTABLE_FIRMWARE
> 2
> +#define
> SPDM_MEASUREMENT_BLOCK_MEASUREMENT_TYPE_HARDWARE_CONFIGUR
> ATION 3
> +#define
> SPDM_MEASUREMENT_BLOCK_MEASUREMENT_TYPE_FIRMWARE_CONFIGUR
> ATION 4
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> + UINT8 NumberOfBlocks;
> +//SPDM_MEASUREMENT_BLOCK_STRUCT
> MeasurementRecord[NumberOfBlocks];
> +//UINT8 Salt[DigestSize];
> +//UINT8 ContextHash[DigestSize];
> + //
> + // L1 = Concatenate(
> + // GET_MEASUREMENTS_REQUEST1,
> MEASUREMENTS_RESPONSE_WITHOUT_SIGNATURE1)
> + // Signature = Sign(SK, Hash1(L1))
> + //
> +//UINT8 Signature[KeySize];
> +} SPDM_MEASUREMENTS_RESPONSE;
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> + // Param1 == Error Code
> + // Param2 == Error Data
> +//UINT8 ExtendedErrorData[];
> +} SPDM_ERROR_RESPONSE;
> +
> +#define SPDM_ERROR_CODE_INVALID_REQUEST 0x01
> +#define SPDM_ERROR_CODE_BUSY 0x03
> +#define SPDM_ERROR_CODE_UNEXPECTED_REQUEST 0x04
> +#define SPDM_ERROR_CODE_UNINITIALIZED 0x05
> +#define SPDM_ERROR_CODE_REQUESTED_INFO_TOO_LONG 0x40
> +#define SPDM_ERROR_CODE_MAJOR_VERSION_MISMATCH 0x41
> +#define SPDM_ERROR_CODE_RESPONSE_NOT_READY 0x42
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> +} SPDM_RESPONSE_IF_READY_REQUEST;
> +
> +#pragma pack()
> +
> +#endif
> +
> --
> 2.19.2.windows.1
>
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH V2 4/4] MdeModulePkg/Pci: Add DeviceSecurity support.
[not found] ` <15D2BB2E5CC7FD95.31603@groups.io>
@ 2019-11-06 6:47 ` Yao, Jiewen
0 siblings, 0 replies; 20+ messages in thread
From: Yao, Jiewen @ 2019-11-06 6:47 UTC (permalink / raw)
To: devel@edk2.groups.io, Yao, Jiewen
Cc: Wang, Jian J, Wu, Hao A, Ni, Ray, Lou, Yun
Hi Jian/Hao/Ray
Would you please review this patch?
We need this feature in next stable tag as planned.
Thank you
Yao Jiewen
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
> Sent: Thursday, October 31, 2019 8:30 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Ni, Ray <ray.ni@intel.com>; Lou, Yun <yun.lou@intel.com>
> Subject: [edk2-devel] [PATCH V2 4/4] MdeModulePkg/Pci: Add DeviceSecurity
> support.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303
>
> Whenever a PCI device is discovered, PCI bus calls the
> EDKII_DEVICE_SECURITY_PROTOCOL to authenticate it.
> If the function returns success, the PCI bus allocates
> the resource and installs the PCI_IO for the device.
> If the function returns fail, the PCI bus skips the device.
>
> It is similar to EFI_SECURITY_ARCH_PROTOCOL, which
> is used to verify an EFI image.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Yun Lou <yun.lou@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c | 12 +++-
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 +
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 4 +-
> MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 63
> +++++++++++++++++++-
> MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c | 4 +-
> 5 files changed, 77 insertions(+), 7 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> index b020ce50ce..64284ac825 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> @@ -8,7 +8,7 @@
> PCI Root Bridges. So it means platform needs install PCI Root Bridge IO
> protocol for each
> PCI Root Bus and install PCI Host Bridge Resource Allocation Protocol.
>
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -37,7 +37,7 @@ UINT64 gAllZero = 0;
> EFI_PCI_PLATFORM_PROTOCOL *gPciPlatformProtocol;
> EFI_PCI_OVERRIDE_PROTOCOL *gPciOverrideProtocol;
> EDKII_IOMMU_PROTOCOL *mIoMmuProtocol;
> -
> +EDKII_DEVICE_SECURITY_PROTOCOL *mDeviceSecurityProtocol;
>
> GLOBAL_REMOVE_IF_UNREFERENCED
> EFI_PCI_HOTPLUG_REQUEST_PROTOCOL mPciHotPlugRequest = {
> PciHotPlugRequestNotify
> @@ -293,6 +293,14 @@ PciBusDriverBindingStart (
> );
> }
>
> + if (mDeviceSecurityProtocol == NULL) {
> + gBS->LocateProtocol (
> + &gEdkiiDeviceSecurityProtocolGuid,
> + NULL,
> + (VOID **) &mDeviceSecurityProtocol
> + );
> + }
> +
> if (PcdGetBool (PcdPciDisableBusEnumeration)) {
> gFullEnumeration = FALSE;
> } else {
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> index 504a1b1c12..d4113993c8 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> @@ -27,6 +27,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Protocol/PciOverride.h>
> #include <Protocol/PciEnumerationComplete.h>
> #include <Protocol/IoMmu.h>
> +#include <Protocol/DeviceSecurity.h>
>
> #include <Library/DebugLib.h>
> #include <Library/UefiDriverEntryPoint.h>
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> index 05c22025b8..9284998f36 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> @@ -2,7 +2,7 @@
> # The PCI bus driver will probe all PCI devices and allocate MMIO and IO space
> for these devices.
> # Please use PCD feature flag PcdPciBusHotplugDeviceSupport to enable hot
> plug supporting.
> #
> -# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> @@ -90,6 +90,8 @@
> gEfiIncompatiblePciDeviceSupportProtocolGuid ## SOMETIMES_CONSUMES
> gEfiLoadFile2ProtocolGuid ## SOMETIMES_PRODUCES
> gEdkiiIoMmuProtocolGuid ## SOMETIMES_CONSUMES
> + gEdkiiDeviceSecurityProtocolGuid ## SOMETIMES_CONSUMES
> + gEdkiiDeviceIdentifierTypePciGuid ## SOMETIMES_CONSUMES
> gEfiLoadedImageDevicePathProtocolGuid ## CONSUMES
>
> [FeaturePcd]
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> index c7eafff593..df3d1c8fcc 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> @@ -10,6 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include "PciBus.h"
>
> extern CHAR16 *mBarTypeStr[];
> +extern EDKII_DEVICE_SECURITY_PROTOCOL
> *mDeviceSecurityProtocol;
>
> #define OLD_ALIGN 0xFFFFFFFFFFFFFFFFULL
> #define EVEN_ALIGN 0xFFFFFFFFFFFFFFFEULL
> @@ -2092,9 +2093,10 @@ CreatePciIoDevice (
> IN UINT8 Func
> )
> {
> - PCI_IO_DEVICE *PciIoDevice;
> - EFI_PCI_IO_PROTOCOL *PciIo;
> - EFI_STATUS Status;
> + PCI_IO_DEVICE *PciIoDevice;
> + EFI_PCI_IO_PROTOCOL *PciIo;
> + EFI_STATUS Status;
> + EDKII_DEVICE_IDENTIFIER DeviceIdentifier;
>
> PciIoDevice = AllocateZeroPool (sizeof (PCI_IO_DEVICE));
> if (PciIoDevice == NULL) {
> @@ -2156,6 +2158,61 @@ CreatePciIoDevice (
> PciIoDevice->IsPciExp = TRUE;
> }
>
> + //
> + // Now we can do the authentication check for the device.
> + //
> + if (mDeviceSecurityProtocol != NULL) {
> + //
> + // Prepare the parameter
> + //
> + DeviceIdentifier.Version = EDKII_DEVICE_IDENTIFIER_REVISION;
> + CopyGuid (&DeviceIdentifier.DeviceType,
> &gEdkiiDeviceIdentifierTypePciGuid);
> + DeviceIdentifier.DeviceHandle = NULL;
> + Status = gBS->InstallMultipleProtocolInterfaces (
> + &DeviceIdentifier.DeviceHandle,
> + &gEfiDevicePathProtocolGuid,
> + PciIoDevice->DevicePath,
> + &gEdkiiDeviceIdentifierTypePciGuid,
> + &PciIoDevice->PciIo,
> + NULL
> + );
> + if (EFI_ERROR(Status)) {
> + if (PciIoDevice->DevicePath != NULL) {
> + FreePool (PciIoDevice->DevicePath);
> + }
> + FreePool (PciIoDevice);
> + return NULL;
> + }
> +
> + //
> + // Do DeviceAuthentication
> + //
> + Status = mDeviceSecurityProtocol->DeviceAuthenticate
> (mDeviceSecurityProtocol, &DeviceIdentifier);
> + //
> + // Always uninstall, because they are only for Authentication.
> + // No need to check return Status.
> + //
> + gBS->UninstallMultipleProtocolInterfaces (
> + DeviceIdentifier.DeviceHandle,
> + &gEfiDevicePathProtocolGuid,
> + PciIoDevice->DevicePath,
> + &gEdkiiDeviceIdentifierTypePciGuid,
> + &PciIoDevice->PciIo,
> + NULL
> + );
> +
> + //
> + // If authentication fails, skip this device.
> + //
> + if (EFI_ERROR(Status)) {
> + if (PciIoDevice->DevicePath != NULL) {
> + FreePool (PciIoDevice->DevicePath);
> + }
> + FreePool (PciIoDevice);
> + return NULL;
> + }
> + }
> +
> if (PcdGetBool (PcdAriSupport)) {
> //
> // Check if the device is an ARI device.
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
> index 5b55fb5d3b..72690ab647 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
> @@ -1054,7 +1054,9 @@ PciScanBus (
> &PciDevice
> );
>
> - ASSERT (!EFI_ERROR (Status));
> + if (EFI_ERROR (Status)) {
> + continue;
> + }
>
> PciAddress = EFI_PCI_ADDRESS (StartBusNumber, Device, Func, 0);
>
> --
> 2.19.2.windows.1
>
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add DeviceSecurity.h
[not found] ` <15D2BB2DC41C838D.31603@groups.io>
@ 2019-11-06 6:47 ` Yao, Jiewen
0 siblings, 0 replies; 20+ messages in thread
From: Yao, Jiewen @ 2019-11-06 6:47 UTC (permalink / raw)
To: devel@edk2.groups.io, Yao, Jiewen; +Cc: Wang, Jian J, Wu, Hao A, Lou, Yun
Hi Jian/Hao/Ray
Would you please review this patch?
We need this feature in next stable tag as planned.
Thank you
Yao Jiewen
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
> Sent: Thursday, October 31, 2019 8:30 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Lou, Yun <yun.lou@intel.com>
> Subject: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add
> DeviceSecurity.h
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303
>
> EDKII_DEVICE_SECURITY_PROTOCOL is used for device
> measurement and/or authentication.
> It is similar to EFI_SECURITY_ARCH_PROTOCOL.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Yun Lou <yun.lou@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
> MdeModulePkg/Include/Protocol/DeviceSecurity.h | 162
> ++++++++++++++++++++
> 1 file changed, 162 insertions(+)
>
> diff --git a/MdeModulePkg/Include/Protocol/DeviceSecurity.h
> b/MdeModulePkg/Include/Protocol/DeviceSecurity.h
> new file mode 100644
> index 0000000000..c3bf624cac
> --- /dev/null
> +++ b/MdeModulePkg/Include/Protocol/DeviceSecurity.h
> @@ -0,0 +1,162 @@
> +/** @file
> + Device Security Protocol definition.
> +
> + It is used to authenticate a device based upon the platform policy.
> + It is similar to the EFI_SECURITY_ARCH_PROTOCOL, which is used to verify a
> image.
> +
> +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#ifndef __DEVICE_SECURITY_H__
> +#define __DEVICE_SECURITY_H__
> +
> +//
> +// Device Security Protocol GUID value
> +//
> +#define EDKII_DEVICE_SECURITY_PROTOCOL_GUID \
> + { \
> + 0x5d6b38c8, 0x5510, 0x4458, { 0xb4, 0x8d, 0x95, 0x81, 0xcf, 0xa7, 0xb0,
> 0xd } \
> + }
> +
> +//
> +// Forward reference for pure ANSI compatability
> +//
> +typedef struct _EDKII_DEVICE_SECURITY_PROTOCOL
> EDKII_DEVICE_SECURITY_PROTOCOL;
> +
> +//
> +// Revision The revision to which the DEVICE_SECURITY interface adheres.
> +// All future revisions must be backwards compatible.
> +// If a future version is not back wards compatible it is not the same GUID.
> +//
> +#define EDKII_DEVICE_SECURITY_PROTOCOL_REVISION 0x00010000
> +
> +//
> +// The device identifier.
> +//
> +typedef struct {
> + ///
> + /// Version of this data structure.
> + ///
> + UINT32 Version;
> + ///
> + /// Type of the device.
> + /// This field is also served as a device Access protocol GUID.
> + /// The device access protocol is installed on the DeviceHandle.
> + /// The device access protocol is device specific.
> + /// EDKII_DEVICE_IDENTIFIER_TYPE_PCI_GUID means the device access
> protocol is PciIo.
> + /// EDKII_DEVICE_IDENTIFIER_TYPE_USB_GUID means the device access
> protocol is UsbIo.
> + ///
> + EFI_GUID DeviceType;
> + ///
> + /// The handle created for this device.
> + /// NOTE: This might be a temporary handle.
> + /// If the device is not authenticated, this handle shall be uninstalled.
> + ///
> + /// As minimal requirement, there should be 2 protocols installed on the
> device handle.
> + /// 1) An EFI_DEVICE_PATH_PROTOCOL with
> EFI_DEVICE_PATH_PROTOCOL_GUID.
> + /// 2) A device access protocol with
> EDKII_DEVICE_IDENTIFIER_TYPE_xxx_GUID.
> + /// If the device is PCI device, the EFI_PCI_IO_PROTOCOL is installed with
> + /// EDKII_DEVICE_IDENTIFIER_TYPE_PCI_GUID.
> + /// If the device is USB device, the EFI_USB_IO_PROTOCOL is installed with
> + /// EDKII_DEVICE_IDENTIFIER_TYPE_USB_GUID.
> + ///
> + /// The device access protocol is required, because the verifier need have a
> way
> + /// to communciate with the device hardware to get the measurement or do
> the
> + /// challenge/response for the device authentication.
> + ///
> + /// NOTE: We don't use EFI_PCI_IO_PROTOCOL_GUID or
> EFI_USB_IO_PROTOCOL_GUID here,
> + /// because we don't want to expose a real protocol. A platform may have
> driver
> + /// register a protocol notify function. Installing a real protocol may cause
> + /// the callback function being executed before the device is authenticated.
> + ///
> + EFI_HANDLE DeviceHandle;
> +} EDKII_DEVICE_IDENTIFIER;
> +
> +//
> +// Revision The revision to which the DEVICE_IDENTIFIER interface adheres.
> +// All future revisions must be backwards compatible.
> +//
> +#define EDKII_DEVICE_IDENTIFIER_REVISION 0x00010000
> +
> +//
> +// Device Identifier GUID value
> +//
> +#define EDKII_DEVICE_IDENTIFIER_TYPE_PCI_GUID \
> + { \
> + 0x2509b2f1, 0xa022, 0x4cca, { 0xaf, 0x70, 0xf9, 0xd3, 0x21, 0xfb, 0x66,
> 0x49 } \
> + }
> +
> +#define EDKII_DEVICE_IDENTIFIER_TYPE_USB_GUID \
> + { \
> + 0x7394f350, 0x394d, 0x488c, { 0xbb, 0x75, 0xc, 0xab, 0x7b, 0x12, 0xa, 0xc5 }
> \
> + }
> +
> +/**
> + The device driver uses this service to measure and/or verify a device.
> +
> + The flow in device driver is:
> + 1) Device driver discovers a new device.
> + 2) Device driver creates an EFI_DEVICE_PATH_PROTOCOL.
> + 3) Device driver creates a device access protocol. e.g.
> + EFI_PCI_IO_PROTOCOL for PCI device.
> + EFI_USB_IO_PROTOCOL for USB device.
> + EFI_EXT_SCSI_PASS_THRU_PROTOCOL for SCSI device.
> + EFI_ATA_PASS_THRU_PROTOCOL for ATA device.
> + EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL for NVMe device.
> + EFI_SD_MMC_PASS_THRU_PROTOCOL for SD/MMC device.
> + 4) Device driver installs the EFI_DEVICE_PATH_PROTOCOL with
> EFI_DEVICE_PATH_PROTOCOL_GUID,
> + and the device access protocol with
> EDKII_DEVICE_IDENTIFIER_TYPE_xxx_GUID.
> + Once it is done, a DeviceHandle is returned.
> + 5) Device driver creates EDKII_DEVICE_IDENTIFIER with
> EDKII_DEVICE_IDENTIFIER_TYPE_xxx_GUID
> + and the DeviceHandle.
> + 6) Device driver calls DeviceAuthenticate().
> + 7) If DeviceAuthenticate() returns EFI_SECURITY_VIOLATION, the device driver
> uninstalls
> + all protocols on this handle.
> + 8) If DeviceAuthenticate() returns EFI_SUCCESS, the device driver installs the
> device access
> + protocol with a real protocol GUID. e.g.
> + EFI_PCI_IO_PROTOCOL with EFI_PCI_IO_PROTOCOL_GUID.
> + EFI_USB_IO_PROTOCOL with EFI_USB_IO_PROTOCOL_GUID.
> +
> + @param[in] This The protocol instance pointer.
> + @param[in] DeviceId The Identifier for the device.
> +
> + @retval EFI_SUCCESS The device specified by the DeviceId passed the
> measurement
> + and/or authentication based upon the platform policy.
> + If TCG measurement is required, the measurement is
> extended to TPM PCR.
> + @retval EFI_SECURITY_VIOLATION The device fails to return the
> measurement data.
> + @retval EFI_SECURITY_VIOLATION The device fails to response the
> authentication request.
> + @retval EFI_SECURITY_VIOLATION The system fails to verify the device
> based upon the authentication response.
> + @retval EFI_SECURITY_VIOLATION The system fails to extend the
> measurement to TPM PCR.
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EDKII_DEVICE_AUTHENTICATE)(
> + IN EDKII_DEVICE_SECURITY_PROTOCOL *This,
> + IN EDKII_DEVICE_IDENTIFIER *DeviceId
> + );
> +
> +///
> +/// Device Security Protocol structure.
> +/// It is similar to the EFI_SECURITY_ARCH_PROTOCOL, which is used to verify
> a image.
> +/// This protocol is used to authenticate a device based upon the platform
> policy.
> +///
> +struct _EDKII_DEVICE_SECURITY_PROTOCOL {
> + UINT64 Revision;
> + EDKII_DEVICE_AUTHENTICATE DeviceAuthenticate;
> +};
> +
> +///
> +/// Device Security Protocol GUID variable.
> +///
> +extern EFI_GUID gEdkiiDeviceSecurityProtocolGuid;
> +
> +///
> +/// Device Identifier tpye GUID variable.
> +///
> +extern EFI_GUID gEdkiiDeviceIdentifierTypePciGuid;
> +extern EFI_GUID gEdkiiDeviceIdentifierTypeUsbGuid;
> +
> +#endif
> --
> 2.19.2.windows.1
>
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH V2 3/4] MdeModulePkg/dec: Add EdkiiDeviceSecurityProtocolGuid.
[not found] ` <15D2BB2E0995721D.31603@groups.io>
@ 2019-11-06 6:47 ` Yao, Jiewen
0 siblings, 0 replies; 20+ messages in thread
From: Yao, Jiewen @ 2019-11-06 6:47 UTC (permalink / raw)
To: devel@edk2.groups.io, Yao, Jiewen; +Cc: Wang, Jian J, Wu, Hao A, Lou, Yun
Hi Jian/Hao/Ray
Would you please review this patch?
We need this feature in next stable tag as planned.
Thank you
Yao Jiewen
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
> Sent: Thursday, October 31, 2019 8:30 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Lou, Yun <yun.lou@intel.com>
> Subject: [edk2-devel] [PATCH V2 3/4] MdeModulePkg/dec: Add
> EdkiiDeviceSecurityProtocolGuid.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Yun Lou <yun.lou@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
> MdeModulePkg/MdeModulePkg.dec | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec
> index d6bac974da..b7356aa4ed 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -584,6 +584,11 @@
> ## Include/Protocol/IoMmu.h
> gEdkiiIoMmuProtocolGuid = { 0x4e939de9, 0xd948, 0x4b0f, { 0x88, 0xed, 0xe6,
> 0xe1, 0xce, 0x51, 0x7c, 0x1e } }
>
> + ## Include/Protocol/DeviceSecurity.h
> + gEdkiiDeviceSecurityProtocolGuid = { 0x5d6b38c8, 0x5510, 0x4458, { 0xb4,
> 0x8d, 0x95, 0x81, 0xcf, 0xa7, 0xb0, 0xd } }
> + gEdkiiDeviceIdentifierTypePciGuid = { 0x2509b2f1, 0xa022, 0x4cca, { 0xaf,
> 0x70, 0xf9, 0xd3, 0x21, 0xfb, 0x66, 0x49 } }
> + gEdkiiDeviceIdentifierTypeUsbGuid = { 0x7394f350, 0x394d, 0x488c, { 0xbb,
> 0x75, 0xc, 0xab, 0x7b, 0x12, 0xa, 0xc5 } }
> +
> ## Include/Protocol/SmmMemoryAttribute.h
> gEdkiiSmmMemoryAttributeProtocolGuid = { 0x69b792ea, 0x39ce, 0x402d,
> { 0xa2, 0xa6, 0xf7, 0x21, 0xde, 0x35, 0x1d, 0xfe } }
>
> --
> 2.19.2.windows.1
>
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add DeviceSecurity.h
2019-10-31 12:30 ` [PATCH V2 2/4] MdeModulePkg/Include: Add DeviceSecurity.h Yao, Jiewen
@ 2019-11-06 7:55 ` Ni, Ray
2019-11-06 8:25 ` Yao, Jiewen
0 siblings, 1 reply; 20+ messages in thread
From: Ni, Ray @ 2019-11-06 7:55 UTC (permalink / raw)
To: devel@edk2.groups.io, Yao, Jiewen; +Cc: Wang, Jian J, Wu, Hao A, Lou, Yun
> + ///
> + /// Type of the device.
> + /// This field is also served as a device Access protocol GUID.
> + /// The device access protocol is installed on the DeviceHandle.
> + /// The device access protocol is device specific.
> + /// EDKII_DEVICE_IDENTIFIER_TYPE_PCI_GUID means the device access protocol is PciIo.
> + /// EDKII_DEVICE_IDENTIFIER_TYPE_USB_GUID means the device access protocol is UsbIo.
> + ///
> + EFI_GUID DeviceType;
Do we still need DeviceType? Consumer can query the Handle to understand the device type.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add DeviceSecurity.h
2019-11-06 7:55 ` [edk2-devel] " Ni, Ray
@ 2019-11-06 8:25 ` Yao, Jiewen
2019-11-07 1:58 ` Ni, Ray
[not found] ` <15D4BEC95EBB70CB.18056@groups.io>
0 siblings, 2 replies; 20+ messages in thread
From: Yao, Jiewen @ 2019-11-06 8:25 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io; +Cc: Wang, Jian J, Wu, Hao A, Lou, Yun
HI
Good comment. Let me answer it in 2 parts.
1) The consumer may locate the deice path to know the device type. In this part, you can treat this as redundant information.
2) But we still need a new GUID, because I will install the device access protocol on this new GUID for the temporary access for the authentication driver only.
I don't want to install the device access protocol to the original UEFI spec defined GUID to notify everyone that the device is ready to use, because I have seen some device drivers have callback function (such as ATA passthru, or NVME passthru) to start access the device once the device access protocol is installed.
Thank you
Yao Jiewen
> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Wednesday, November 6, 2019 3:56 PM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Lou, Yun <yun.lou@intel.com>
> Subject: RE: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add
> DeviceSecurity.h
>
> > + ///
> > + /// Type of the device.
> > + /// This field is also served as a device Access protocol GUID.
> > + /// The device access protocol is installed on the DeviceHandle.
> > + /// The device access protocol is device specific.
> > + /// EDKII_DEVICE_IDENTIFIER_TYPE_PCI_GUID means the device access
> protocol is PciIo.
> > + /// EDKII_DEVICE_IDENTIFIER_TYPE_USB_GUID means the device access
> protocol is UsbIo.
> > + ///
> > + EFI_GUID DeviceType;
>
> Do we still need DeviceType? Consumer can query the Handle to understand the
> device type.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V2 1/4] MdePkg/Include: Add DMTF SPDM definition.
2019-10-31 12:30 ` [PATCH V2 1/4] MdePkg/Include: Add DMTF SPDM definition Yao, Jiewen
@ 2019-11-06 14:38 ` Liming Gao
2019-11-07 0:25 ` Yao, Jiewen
[not found] ` <15D4B9B24059B4F1.19610@groups.io>
0 siblings, 2 replies; 20+ messages in thread
From: Liming Gao @ 2019-11-06 14:38 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io; +Cc: Kinney, Michael D, Lou, Yun
Jiewen:
I have one minor comment. Can you specify the spec version in file header?
With this update, Reviewed-by: Liming Gao <liming.gao@intel.com>
Thanks
Liming
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Thursday, October 31, 2019 8:30 PM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Lou, Yun <yun.lou@intel.com>
> Subject: [PATCH V2 1/4] MdePkg/Include: Add DMTF SPDM definition.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Yun Lou <yun.lou@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
> MdePkg/Include/IndustryStandard/Spdm.h | 203 ++++++++++++++++++++
> 1 file changed, 203 insertions(+)
>
> diff --git a/MdePkg/Include/IndustryStandard/Spdm.h b/MdePkg/Include/IndustryStandard/Spdm.h
> new file mode 100644
> index 0000000000..d62b24e9ef
> --- /dev/null
> +++ b/MdePkg/Include/IndustryStandard/Spdm.h
> @@ -0,0 +1,203 @@
> +/** @file
> + Definitions of Security Protocol & Data Model Specification (SPDM)
> + in Distributed Management Task Force (DMTF).
> +
> +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#ifndef __SPDM_H__
> +#define __SPDM_H__
> +
> +#pragma pack(1)
> +
> +#define SPDM_DIGESTS 0x01
> +#define SPDM_CERTIFICATE 0x02
> +#define SPDM_CHALLENGE_AUTH 0x03
> +#define SPDM_MEASUREMENTS 0x60
> +#define SPDM_CAPABILITIES 0x61
> +#define SPDM_SET_CERT_RESPONSE 0x62
> +#define SPDM_ALGORITHMS 0x63
> +#define SPDM_ERROR 0x7F
> +#define SPDM_GET_DIGESTS 0x81
> +#define SPDM_GET_CERTIFICATE 0x82
> +#define SPDM_CHALLENGE 0x83
> +#define SPDM_GET_MEASUREMENTS 0xE0
> +#define SPDM_GET_CAPABILITIES 0xE1
> +#define SPDM_SET_CERTIFICATE 0xE2
> +#define SPDM_NEGOTIATE_ALGORITHMS 0xE3
> +#define SPDM_RESPOND_IF_READY 0xFF
> +
> +typedef struct {
> + UINT8 SPDMVersion;
> + UINT8 RequestResponseCode;
> + UINT8 Param1;
> + UINT8 Param2;
> +} SPDM_MESSAGE_HEADER;
> +
> +#define SPDM_VERSION 0x10
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> +} SPDM_GET_CAPABILITIES_REQUEST;
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> + UINT8 DetailedVersion;
> + UINT8 CryptographicTimeout;
> + UINT16 Reserved;
> + UINT32 Flags;
> + UINT16 SPDMMajorVersions;
> + UINT16 Reserved2;
> +} SPDM_CAPABILITIES_RESPONSE;
> +
> +#define SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_AUTH_CAP BIT1
> +#define SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_MEAS_CAP BIT3
> +#define SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_MEAS_FRESH_CAP BIT4
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> + UINT16 Length;
> + UINT8 MeasurementSpecification;
> + UINT8 Reserved;
> + UINT32 BaseAsymAlgo;
> + UINT32 BaseHashAlgo;
> + UINT64 Reserved2;
> + UINT8 ExtAsymCount;
> + UINT8 ExtHashCount;
> + UINT16 Reserved3;
> +//UINT32 ExtAsym[ExtAsymCount];
> +//UINT32 ExtHash[ExtHashCount];
> +} SPDM_NEGOTIATE_ALGORITHMS_REQUEST;
> +
> +#define SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_RSASSA_2048 BIT0
> +#define SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_RSASSA_3072 BIT1
> +#define SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_ECDSA_ECC_NIST_P256 BIT2
> +#define SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_RSASSA_4096 BIT3
> +#define SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_ECDSA_ECC_NIST_P384 BIT4
> +#define SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_ECDSA_ECC_NIST_P521 BIT5
> +
> +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA2_256 BIT0
> +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA3_256 BIT1
> +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA2_384 BIT2
> +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA3_384 BIT3
> +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA2_512 BIT4
> +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA3_512 BIT5
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> + UINT16 Length;
> + UINT8 MeasurementSpecification;
> + UINT8 MeasurementHashAlgo;
> + UINT32 BaseAsymSel;
> + UINT32 BaseHashSel;
> + UINT64 Reserved;
> + UINT8 ExtAsymSelCount;
> + UINT8 ExtHashSelCount;
> + UINT16 Reserved2;
> +//UINT32 ExtAsymSel[ExtAsymSelCount];
> +//UINT32 ExtHashSel[ExtHashSelCount];
> +} SPDM_ALGORITHMS_RESPONSE;
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> +} SPDM_GET_DIGESTS_REQUEST;
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> +//UINT8 Digest[DigestSize];
> +} SPDM_DIGESTS_RESPONSE;
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> + UINT16 Offset;
> + UINT16 Length;
> +} SPDM_GET_CERTIFICATE_REQUEST;
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> +//UINT8 CertChain[CertChainSize];
> +} SPDM_CERTIFICATE_RESPONSE;
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> +//UINT8 Nonce[DigestSize];
> +} SPDM_CHALLENGE_REQUEST;
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> + UINT8 MinSPDMVersion;
> + UINT8 MaxSPDMVersion;
> + UINT8 Capabilities;
> + UINT8 Reserved;
> +//UINT8 CertChainHash[DigestSize];
> +//UINT8 Salt[DigestSize];
> +//UINT8 ContextHash[DigestSize];
> + //
> + // M1 = Concatenate(
> + // GET_CAPABILITIES_REQUEST1, CAPABILITIES_RESPONSE1,
> + // NEGOTIATE_ALGORITHMS_REQUEST1, ALGORITHMS_RESPONSE1, CHALLENGE_REQUEST1,
> + // CHALLENGE_AUTH_RESPONSE_WITHOUT_SIGNATURE1)
> + // Signature = Sign(SK, Hash1(M1))
> + //
> +//UINT8 Signature[KeySize];
> +} SPDM_CHALLENGE_AUTH_RESPONSE;
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> + // Param1 == Request Type
> + // Param2 == Measurement Index (0xFF == all)
> +//UINT8 Nonce[DigestSize];
> +} SPDM_GET_MEASUREMENTS_REQUEST;
> +
> +typedef struct {
> + UINT8 Index;
> + UINT8 MeasurementType;
> + UINT8 MeasurementSpecification;
> + UINT8 Reserved;
> +} SPDM_MEASUREMENT_BLOCK_HEADER;
> +
> +#define SPDM_MEASUREMENT_BLOCK_MEASUREMENT_TYPE_IMMUTABLE_ROM 1
> +#define SPDM_MEASUREMENT_BLOCK_MEASUREMENT_TYPE_MUTABLE_FIRMWARE 2
> +#define SPDM_MEASUREMENT_BLOCK_MEASUREMENT_TYPE_HARDWARE_CONFIGURATION 3
> +#define SPDM_MEASUREMENT_BLOCK_MEASUREMENT_TYPE_FIRMWARE_CONFIGURATION 4
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> + UINT8 NumberOfBlocks;
> +//SPDM_MEASUREMENT_BLOCK_STRUCT MeasurementRecord[NumberOfBlocks];
> +//UINT8 Salt[DigestSize];
> +//UINT8 ContextHash[DigestSize];
> + //
> + // L1 = Concatenate(
> + // GET_MEASUREMENTS_REQUEST1, MEASUREMENTS_RESPONSE_WITHOUT_SIGNATURE1)
> + // Signature = Sign(SK, Hash1(L1))
> + //
> +//UINT8 Signature[KeySize];
> +} SPDM_MEASUREMENTS_RESPONSE;
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> + // Param1 == Error Code
> + // Param2 == Error Data
> +//UINT8 ExtendedErrorData[];
> +} SPDM_ERROR_RESPONSE;
> +
> +#define SPDM_ERROR_CODE_INVALID_REQUEST 0x01
> +#define SPDM_ERROR_CODE_BUSY 0x03
> +#define SPDM_ERROR_CODE_UNEXPECTED_REQUEST 0x04
> +#define SPDM_ERROR_CODE_UNINITIALIZED 0x05
> +#define SPDM_ERROR_CODE_REQUESTED_INFO_TOO_LONG 0x40
> +#define SPDM_ERROR_CODE_MAJOR_VERSION_MISMATCH 0x41
> +#define SPDM_ERROR_CODE_RESPONSE_NOT_READY 0x42
> +
> +typedef struct {
> + SPDM_MESSAGE_HEADER Header;
> +} SPDM_RESPONSE_IF_READY_REQUEST;
> +
> +#pragma pack()
> +
> +#endif
> +
> --
> 2.19.2.windows.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V2 1/4] MdePkg/Include: Add DMTF SPDM definition.
2019-11-06 14:38 ` Liming Gao
@ 2019-11-07 0:25 ` Yao, Jiewen
[not found] ` <15D4B9B24059B4F1.19610@groups.io>
1 sibling, 0 replies; 20+ messages in thread
From: Yao, Jiewen @ 2019-11-07 0:25 UTC (permalink / raw)
To: Gao, Liming, devel@edk2.groups.io; +Cc: Kinney, Michael D, Lou, Yun
Thanks. Agree.
Will do that. It is 0.95 now.
> -----Original Message-----
> From: Gao, Liming <liming.gao@intel.com>
> Sent: Wednesday, November 6, 2019 10:38 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Lou, Yun
> <yun.lou@intel.com>
> Subject: RE: [PATCH V2 1/4] MdePkg/Include: Add DMTF SPDM definition.
>
> Jiewen:
> I have one minor comment. Can you specify the spec version in file header?
> With this update, Reviewed-by: Liming Gao <liming.gao@intel.com>
>
> Thanks
> Liming
> > -----Original Message-----
> > From: Yao, Jiewen <jiewen.yao@intel.com>
> > Sent: Thursday, October 31, 2019 8:30 PM
> > To: devel@edk2.groups.io
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Lou, Yun <yun.lou@intel.com>
> > Subject: [PATCH V2 1/4] MdePkg/Include: Add DMTF SPDM definition.
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303
> >
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Yun Lou <yun.lou@intel.com>
> > Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> > ---
> > MdePkg/Include/IndustryStandard/Spdm.h | 203 ++++++++++++++++++++
> > 1 file changed, 203 insertions(+)
> >
> > diff --git a/MdePkg/Include/IndustryStandard/Spdm.h
> b/MdePkg/Include/IndustryStandard/Spdm.h
> > new file mode 100644
> > index 0000000000..d62b24e9ef
> > --- /dev/null
> > +++ b/MdePkg/Include/IndustryStandard/Spdm.h
> > @@ -0,0 +1,203 @@
> > +/** @file
> > + Definitions of Security Protocol & Data Model Specification (SPDM)
> > + in Distributed Management Task Force (DMTF).
> > +
> > +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> > +SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +
> > +#ifndef __SPDM_H__
> > +#define __SPDM_H__
> > +
> > +#pragma pack(1)
> > +
> > +#define SPDM_DIGESTS 0x01
> > +#define SPDM_CERTIFICATE 0x02
> > +#define SPDM_CHALLENGE_AUTH 0x03
> > +#define SPDM_MEASUREMENTS 0x60
> > +#define SPDM_CAPABILITIES 0x61
> > +#define SPDM_SET_CERT_RESPONSE 0x62
> > +#define SPDM_ALGORITHMS 0x63
> > +#define SPDM_ERROR 0x7F
> > +#define SPDM_GET_DIGESTS 0x81
> > +#define SPDM_GET_CERTIFICATE 0x82
> > +#define SPDM_CHALLENGE 0x83
> > +#define SPDM_GET_MEASUREMENTS 0xE0
> > +#define SPDM_GET_CAPABILITIES 0xE1
> > +#define SPDM_SET_CERTIFICATE 0xE2
> > +#define SPDM_NEGOTIATE_ALGORITHMS 0xE3
> > +#define SPDM_RESPOND_IF_READY 0xFF
> > +
> > +typedef struct {
> > + UINT8 SPDMVersion;
> > + UINT8 RequestResponseCode;
> > + UINT8 Param1;
> > + UINT8 Param2;
> > +} SPDM_MESSAGE_HEADER;
> > +
> > +#define SPDM_VERSION 0x10
> > +
> > +typedef struct {
> > + SPDM_MESSAGE_HEADER Header;
> > +} SPDM_GET_CAPABILITIES_REQUEST;
> > +
> > +typedef struct {
> > + SPDM_MESSAGE_HEADER Header;
> > + UINT8 DetailedVersion;
> > + UINT8 CryptographicTimeout;
> > + UINT16 Reserved;
> > + UINT32 Flags;
> > + UINT16 SPDMMajorVersions;
> > + UINT16 Reserved2;
> > +} SPDM_CAPABILITIES_RESPONSE;
> > +
> > +#define SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_AUTH_CAP BIT1
> > +#define SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_MEAS_CAP BIT3
> > +#define SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_MEAS_FRESH_CAP
> BIT4
> > +
> > +typedef struct {
> > + SPDM_MESSAGE_HEADER Header;
> > + UINT16 Length;
> > + UINT8 MeasurementSpecification;
> > + UINT8 Reserved;
> > + UINT32 BaseAsymAlgo;
> > + UINT32 BaseHashAlgo;
> > + UINT64 Reserved2;
> > + UINT8 ExtAsymCount;
> > + UINT8 ExtHashCount;
> > + UINT16 Reserved3;
> > +//UINT32 ExtAsym[ExtAsymCount];
> > +//UINT32 ExtHash[ExtHashCount];
> > +} SPDM_NEGOTIATE_ALGORITHMS_REQUEST;
> > +
> > +#define SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_RSASSA_2048
> BIT0
> > +#define SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_RSASSA_3072
> BIT1
> > +#define
> SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_ECDSA_ECC_NIST_P256
> BIT2
> > +#define SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_RSASSA_4096
> BIT3
> > +#define
> SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_ECDSA_ECC_NIST_P384
> BIT4
> > +#define
> SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_ECDSA_ECC_NIST_P521
> BIT5
> > +
> > +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA2_256
> BIT0
> > +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA3_256
> BIT1
> > +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA2_384
> BIT2
> > +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA3_384
> BIT3
> > +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA2_512
> BIT4
> > +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA3_512
> BIT5
> > +
> > +typedef struct {
> > + SPDM_MESSAGE_HEADER Header;
> > + UINT16 Length;
> > + UINT8 MeasurementSpecification;
> > + UINT8 MeasurementHashAlgo;
> > + UINT32 BaseAsymSel;
> > + UINT32 BaseHashSel;
> > + UINT64 Reserved;
> > + UINT8 ExtAsymSelCount;
> > + UINT8 ExtHashSelCount;
> > + UINT16 Reserved2;
> > +//UINT32 ExtAsymSel[ExtAsymSelCount];
> > +//UINT32 ExtHashSel[ExtHashSelCount];
> > +} SPDM_ALGORITHMS_RESPONSE;
> > +
> > +typedef struct {
> > + SPDM_MESSAGE_HEADER Header;
> > +} SPDM_GET_DIGESTS_REQUEST;
> > +
> > +typedef struct {
> > + SPDM_MESSAGE_HEADER Header;
> > +//UINT8 Digest[DigestSize];
> > +} SPDM_DIGESTS_RESPONSE;
> > +
> > +typedef struct {
> > + SPDM_MESSAGE_HEADER Header;
> > + UINT16 Offset;
> > + UINT16 Length;
> > +} SPDM_GET_CERTIFICATE_REQUEST;
> > +
> > +typedef struct {
> > + SPDM_MESSAGE_HEADER Header;
> > +//UINT8 CertChain[CertChainSize];
> > +} SPDM_CERTIFICATE_RESPONSE;
> > +
> > +typedef struct {
> > + SPDM_MESSAGE_HEADER Header;
> > +//UINT8 Nonce[DigestSize];
> > +} SPDM_CHALLENGE_REQUEST;
> > +
> > +typedef struct {
> > + SPDM_MESSAGE_HEADER Header;
> > + UINT8 MinSPDMVersion;
> > + UINT8 MaxSPDMVersion;
> > + UINT8 Capabilities;
> > + UINT8 Reserved;
> > +//UINT8 CertChainHash[DigestSize];
> > +//UINT8 Salt[DigestSize];
> > +//UINT8 ContextHash[DigestSize];
> > + //
> > + // M1 = Concatenate(
> > + // GET_CAPABILITIES_REQUEST1, CAPABILITIES_RESPONSE1,
> > + // NEGOTIATE_ALGORITHMS_REQUEST1, ALGORITHMS_RESPONSE1,
> CHALLENGE_REQUEST1,
> > + // CHALLENGE_AUTH_RESPONSE_WITHOUT_SIGNATURE1)
> > + // Signature = Sign(SK, Hash1(M1))
> > + //
> > +//UINT8 Signature[KeySize];
> > +} SPDM_CHALLENGE_AUTH_RESPONSE;
> > +
> > +typedef struct {
> > + SPDM_MESSAGE_HEADER Header;
> > + // Param1 == Request Type
> > + // Param2 == Measurement Index (0xFF == all)
> > +//UINT8 Nonce[DigestSize];
> > +} SPDM_GET_MEASUREMENTS_REQUEST;
> > +
> > +typedef struct {
> > + UINT8 Index;
> > + UINT8 MeasurementType;
> > + UINT8 MeasurementSpecification;
> > + UINT8 Reserved;
> > +} SPDM_MEASUREMENT_BLOCK_HEADER;
> > +
> > +#define
> SPDM_MEASUREMENT_BLOCK_MEASUREMENT_TYPE_IMMUTABLE_ROM
> 1
> > +#define
> SPDM_MEASUREMENT_BLOCK_MEASUREMENT_TYPE_MUTABLE_FIRMWARE
> 2
> > +#define
> SPDM_MEASUREMENT_BLOCK_MEASUREMENT_TYPE_HARDWARE_CONFIGUR
> ATION 3
> > +#define
> SPDM_MEASUREMENT_BLOCK_MEASUREMENT_TYPE_FIRMWARE_CONFIGUR
> ATION 4
> > +
> > +typedef struct {
> > + SPDM_MESSAGE_HEADER Header;
> > + UINT8 NumberOfBlocks;
> > +//SPDM_MEASUREMENT_BLOCK_STRUCT
> MeasurementRecord[NumberOfBlocks];
> > +//UINT8 Salt[DigestSize];
> > +//UINT8 ContextHash[DigestSize];
> > + //
> > + // L1 = Concatenate(
> > + // GET_MEASUREMENTS_REQUEST1,
> MEASUREMENTS_RESPONSE_WITHOUT_SIGNATURE1)
> > + // Signature = Sign(SK, Hash1(L1))
> > + //
> > +//UINT8 Signature[KeySize];
> > +} SPDM_MEASUREMENTS_RESPONSE;
> > +
> > +typedef struct {
> > + SPDM_MESSAGE_HEADER Header;
> > + // Param1 == Error Code
> > + // Param2 == Error Data
> > +//UINT8 ExtendedErrorData[];
> > +} SPDM_ERROR_RESPONSE;
> > +
> > +#define SPDM_ERROR_CODE_INVALID_REQUEST 0x01
> > +#define SPDM_ERROR_CODE_BUSY 0x03
> > +#define SPDM_ERROR_CODE_UNEXPECTED_REQUEST 0x04
> > +#define SPDM_ERROR_CODE_UNINITIALIZED 0x05
> > +#define SPDM_ERROR_CODE_REQUESTED_INFO_TOO_LONG 0x40
> > +#define SPDM_ERROR_CODE_MAJOR_VERSION_MISMATCH 0x41
> > +#define SPDM_ERROR_CODE_RESPONSE_NOT_READY 0x42
> > +
> > +typedef struct {
> > + SPDM_MESSAGE_HEADER Header;
> > +} SPDM_RESPONSE_IF_READY_REQUEST;
> > +
> > +#pragma pack()
> > +
> > +#endif
> > +
> > --
> > 2.19.2.windows.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH V2 1/4] MdePkg/Include: Add DMTF SPDM definition.
[not found] ` <15D4B9B24059B4F1.19610@groups.io>
@ 2019-11-07 0:57 ` Yao, Jiewen
0 siblings, 0 replies; 20+ messages in thread
From: Yao, Jiewen @ 2019-11-07 0:57 UTC (permalink / raw)
To: devel@edk2.groups.io, Yao, Jiewen, Gao, Liming
Cc: Kinney, Michael D, Lou, Yun
I will update to 0.99 in V3.
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
> Sent: Thursday, November 7, 2019 8:26 AM
> To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Lou, Yun
> <yun.lou@intel.com>
> Subject: Re: [edk2-devel] [PATCH V2 1/4] MdePkg/Include: Add DMTF SPDM
> definition.
>
> Thanks. Agree.
> Will do that. It is 0.95 now.
>
> > -----Original Message-----
> > From: Gao, Liming <liming.gao@intel.com>
> > Sent: Wednesday, November 6, 2019 10:38 PM
> > To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Lou, Yun
> > <yun.lou@intel.com>
> > Subject: RE: [PATCH V2 1/4] MdePkg/Include: Add DMTF SPDM definition.
> >
> > Jiewen:
> > I have one minor comment. Can you specify the spec version in file header?
> > With this update, Reviewed-by: Liming Gao <liming.gao@intel.com>
> >
> > Thanks
> > Liming
> > > -----Original Message-----
> > > From: Yao, Jiewen <jiewen.yao@intel.com>
> > > Sent: Thursday, October 31, 2019 8:30 PM
> > > To: devel@edk2.groups.io
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > <liming.gao@intel.com>; Lou, Yun <yun.lou@intel.com>
> > > Subject: [PATCH V2 1/4] MdePkg/Include: Add DMTF SPDM definition.
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303
> > >
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > Cc: Liming Gao <liming.gao@intel.com>
> > > Cc: Yun Lou <yun.lou@intel.com>
> > > Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> > > ---
> > > MdePkg/Include/IndustryStandard/Spdm.h | 203 ++++++++++++++++++++
> > > 1 file changed, 203 insertions(+)
> > >
> > > diff --git a/MdePkg/Include/IndustryStandard/Spdm.h
> > b/MdePkg/Include/IndustryStandard/Spdm.h
> > > new file mode 100644
> > > index 0000000000..d62b24e9ef
> > > --- /dev/null
> > > +++ b/MdePkg/Include/IndustryStandard/Spdm.h
> > > @@ -0,0 +1,203 @@
> > > +/** @file
> > > + Definitions of Security Protocol & Data Model Specification (SPDM)
> > > + in Distributed Management Task Force (DMTF).
> > > +
> > > +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> > > +SPDX-License-Identifier: BSD-2-Clause-Patent
> > > +
> > > +**/
> > > +
> > > +
> > > +#ifndef __SPDM_H__
> > > +#define __SPDM_H__
> > > +
> > > +#pragma pack(1)
> > > +
> > > +#define SPDM_DIGESTS 0x01
> > > +#define SPDM_CERTIFICATE 0x02
> > > +#define SPDM_CHALLENGE_AUTH 0x03
> > > +#define SPDM_MEASUREMENTS 0x60
> > > +#define SPDM_CAPABILITIES 0x61
> > > +#define SPDM_SET_CERT_RESPONSE 0x62
> > > +#define SPDM_ALGORITHMS 0x63
> > > +#define SPDM_ERROR 0x7F
> > > +#define SPDM_GET_DIGESTS 0x81
> > > +#define SPDM_GET_CERTIFICATE 0x82
> > > +#define SPDM_CHALLENGE 0x83
> > > +#define SPDM_GET_MEASUREMENTS 0xE0
> > > +#define SPDM_GET_CAPABILITIES 0xE1
> > > +#define SPDM_SET_CERTIFICATE 0xE2
> > > +#define SPDM_NEGOTIATE_ALGORITHMS 0xE3
> > > +#define SPDM_RESPOND_IF_READY 0xFF
> > > +
> > > +typedef struct {
> > > + UINT8 SPDMVersion;
> > > + UINT8 RequestResponseCode;
> > > + UINT8 Param1;
> > > + UINT8 Param2;
> > > +} SPDM_MESSAGE_HEADER;
> > > +
> > > +#define SPDM_VERSION 0x10
> > > +
> > > +typedef struct {
> > > + SPDM_MESSAGE_HEADER Header;
> > > +} SPDM_GET_CAPABILITIES_REQUEST;
> > > +
> > > +typedef struct {
> > > + SPDM_MESSAGE_HEADER Header;
> > > + UINT8 DetailedVersion;
> > > + UINT8 CryptographicTimeout;
> > > + UINT16 Reserved;
> > > + UINT32 Flags;
> > > + UINT16 SPDMMajorVersions;
> > > + UINT16 Reserved2;
> > > +} SPDM_CAPABILITIES_RESPONSE;
> > > +
> > > +#define SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_AUTH_CAP BIT1
> > > +#define SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_MEAS_CAP BIT3
> > > +#define SPDM_GET_CAPABILITIES_RESPONSE_FLAGS_MEAS_FRESH_CAP
> > BIT4
> > > +
> > > +typedef struct {
> > > + SPDM_MESSAGE_HEADER Header;
> > > + UINT16 Length;
> > > + UINT8 MeasurementSpecification;
> > > + UINT8 Reserved;
> > > + UINT32 BaseAsymAlgo;
> > > + UINT32 BaseHashAlgo;
> > > + UINT64 Reserved2;
> > > + UINT8 ExtAsymCount;
> > > + UINT8 ExtHashCount;
> > > + UINT16 Reserved3;
> > > +//UINT32 ExtAsym[ExtAsymCount];
> > > +//UINT32 ExtHash[ExtHashCount];
> > > +} SPDM_NEGOTIATE_ALGORITHMS_REQUEST;
> > > +
> > > +#define SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_RSASSA_2048
> > BIT0
> > > +#define SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_RSASSA_3072
> > BIT1
> > > +#define
> > SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_ECDSA_ECC_NIST_P256
> > BIT2
> > > +#define SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_RSASSA_4096
> > BIT3
> > > +#define
> > SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_ECDSA_ECC_NIST_P384
> > BIT4
> > > +#define
> > SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_ECDSA_ECC_NIST_P521
> > BIT5
> > > +
> > > +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA2_256
> > BIT0
> > > +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA3_256
> > BIT1
> > > +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA2_384
> > BIT2
> > > +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA3_384
> > BIT3
> > > +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA2_512
> > BIT4
> > > +#define SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA3_512
> > BIT5
> > > +
> > > +typedef struct {
> > > + SPDM_MESSAGE_HEADER Header;
> > > + UINT16 Length;
> > > + UINT8 MeasurementSpecification;
> > > + UINT8 MeasurementHashAlgo;
> > > + UINT32 BaseAsymSel;
> > > + UINT32 BaseHashSel;
> > > + UINT64 Reserved;
> > > + UINT8 ExtAsymSelCount;
> > > + UINT8 ExtHashSelCount;
> > > + UINT16 Reserved2;
> > > +//UINT32 ExtAsymSel[ExtAsymSelCount];
> > > +//UINT32 ExtHashSel[ExtHashSelCount];
> > > +} SPDM_ALGORITHMS_RESPONSE;
> > > +
> > > +typedef struct {
> > > + SPDM_MESSAGE_HEADER Header;
> > > +} SPDM_GET_DIGESTS_REQUEST;
> > > +
> > > +typedef struct {
> > > + SPDM_MESSAGE_HEADER Header;
> > > +//UINT8 Digest[DigestSize];
> > > +} SPDM_DIGESTS_RESPONSE;
> > > +
> > > +typedef struct {
> > > + SPDM_MESSAGE_HEADER Header;
> > > + UINT16 Offset;
> > > + UINT16 Length;
> > > +} SPDM_GET_CERTIFICATE_REQUEST;
> > > +
> > > +typedef struct {
> > > + SPDM_MESSAGE_HEADER Header;
> > > +//UINT8 CertChain[CertChainSize];
> > > +} SPDM_CERTIFICATE_RESPONSE;
> > > +
> > > +typedef struct {
> > > + SPDM_MESSAGE_HEADER Header;
> > > +//UINT8 Nonce[DigestSize];
> > > +} SPDM_CHALLENGE_REQUEST;
> > > +
> > > +typedef struct {
> > > + SPDM_MESSAGE_HEADER Header;
> > > + UINT8 MinSPDMVersion;
> > > + UINT8 MaxSPDMVersion;
> > > + UINT8 Capabilities;
> > > + UINT8 Reserved;
> > > +//UINT8 CertChainHash[DigestSize];
> > > +//UINT8 Salt[DigestSize];
> > > +//UINT8 ContextHash[DigestSize];
> > > + //
> > > + // M1 = Concatenate(
> > > + // GET_CAPABILITIES_REQUEST1, CAPABILITIES_RESPONSE1,
> > > + // NEGOTIATE_ALGORITHMS_REQUEST1, ALGORITHMS_RESPONSE1,
> > CHALLENGE_REQUEST1,
> > > + // CHALLENGE_AUTH_RESPONSE_WITHOUT_SIGNATURE1)
> > > + // Signature = Sign(SK, Hash1(M1))
> > > + //
> > > +//UINT8 Signature[KeySize];
> > > +} SPDM_CHALLENGE_AUTH_RESPONSE;
> > > +
> > > +typedef struct {
> > > + SPDM_MESSAGE_HEADER Header;
> > > + // Param1 == Request Type
> > > + // Param2 == Measurement Index (0xFF == all)
> > > +//UINT8 Nonce[DigestSize];
> > > +} SPDM_GET_MEASUREMENTS_REQUEST;
> > > +
> > > +typedef struct {
> > > + UINT8 Index;
> > > + UINT8 MeasurementType;
> > > + UINT8 MeasurementSpecification;
> > > + UINT8 Reserved;
> > > +} SPDM_MEASUREMENT_BLOCK_HEADER;
> > > +
> > > +#define
> > SPDM_MEASUREMENT_BLOCK_MEASUREMENT_TYPE_IMMUTABLE_ROM
> > 1
> > > +#define
> > SPDM_MEASUREMENT_BLOCK_MEASUREMENT_TYPE_MUTABLE_FIRMWARE
> > 2
> > > +#define
> >
> SPDM_MEASUREMENT_BLOCK_MEASUREMENT_TYPE_HARDWARE_CONFIGUR
> > ATION 3
> > > +#define
> >
> SPDM_MEASUREMENT_BLOCK_MEASUREMENT_TYPE_FIRMWARE_CONFIGUR
> > ATION 4
> > > +
> > > +typedef struct {
> > > + SPDM_MESSAGE_HEADER Header;
> > > + UINT8 NumberOfBlocks;
> > > +//SPDM_MEASUREMENT_BLOCK_STRUCT
> > MeasurementRecord[NumberOfBlocks];
> > > +//UINT8 Salt[DigestSize];
> > > +//UINT8 ContextHash[DigestSize];
> > > + //
> > > + // L1 = Concatenate(
> > > + // GET_MEASUREMENTS_REQUEST1,
> > MEASUREMENTS_RESPONSE_WITHOUT_SIGNATURE1)
> > > + // Signature = Sign(SK, Hash1(L1))
> > > + //
> > > +//UINT8 Signature[KeySize];
> > > +} SPDM_MEASUREMENTS_RESPONSE;
> > > +
> > > +typedef struct {
> > > + SPDM_MESSAGE_HEADER Header;
> > > + // Param1 == Error Code
> > > + // Param2 == Error Data
> > > +//UINT8 ExtendedErrorData[];
> > > +} SPDM_ERROR_RESPONSE;
> > > +
> > > +#define SPDM_ERROR_CODE_INVALID_REQUEST 0x01
> > > +#define SPDM_ERROR_CODE_BUSY 0x03
> > > +#define SPDM_ERROR_CODE_UNEXPECTED_REQUEST 0x04
> > > +#define SPDM_ERROR_CODE_UNINITIALIZED 0x05
> > > +#define SPDM_ERROR_CODE_REQUESTED_INFO_TOO_LONG 0x40
> > > +#define SPDM_ERROR_CODE_MAJOR_VERSION_MISMATCH 0x41
> > > +#define SPDM_ERROR_CODE_RESPONSE_NOT_READY 0x42
> > > +
> > > +typedef struct {
> > > + SPDM_MESSAGE_HEADER Header;
> > > +} SPDM_RESPONSE_IF_READY_REQUEST;
> > > +
> > > +#pragma pack()
> > > +
> > > +#endif
> > > +
> > > --
> > > 2.19.2.windows.1
>
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add DeviceSecurity.h
2019-11-06 8:25 ` Yao, Jiewen
@ 2019-11-07 1:58 ` Ni, Ray
[not found] ` <15D4BEC95EBB70CB.18056@groups.io>
1 sibling, 0 replies; 20+ messages in thread
From: Ni, Ray @ 2019-11-07 1:58 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io; +Cc: Wang, Jian J, Wu, Hao A, Lou, Yun
I see. PciIo or UsbIo instance is installed with a different GUID and consumer needs to parse the device path to understand what kind of instance (PciIo or UsbIo) should be used.
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Wednesday, November 6, 2019 4:25 PM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Lou, Yun <yun.lou@intel.com>
> Subject: RE: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add DeviceSecurity.h
>
> HI
> Good comment. Let me answer it in 2 parts.
>
> 1) The consumer may locate the deice path to know the device type. In this part, you can treat this as redundant
> information.
>
> 2) But we still need a new GUID, because I will install the device access protocol on this new GUID for the temporary access
> for the authentication driver only.
>
> I don't want to install the device access protocol to the original UEFI spec defined GUID to notify everyone that the device
> is ready to use, because I have seen some device drivers have callback function (such as ATA passthru, or NVME passthru)
> to start access the device once the device access protocol is installed.
>
>
> Thank you
> Yao Jiewen
>
> > -----Original Message-----
> > From: Ni, Ray <ray.ni@intel.com>
> > Sent: Wednesday, November 6, 2019 3:56 PM
> > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> > Lou, Yun <yun.lou@intel.com>
> > Subject: RE: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add
> > DeviceSecurity.h
> >
> > > + ///
> > > + /// Type of the device.
> > > + /// This field is also served as a device Access protocol GUID.
> > > + /// The device access protocol is installed on the DeviceHandle.
> > > + /// The device access protocol is device specific.
> > > + /// EDKII_DEVICE_IDENTIFIER_TYPE_PCI_GUID means the device access
> > protocol is PciIo.
> > > + /// EDKII_DEVICE_IDENTIFIER_TYPE_USB_GUID means the device access
> > protocol is UsbIo.
> > > + ///
> > > + EFI_GUID DeviceType;
> >
> > Do we still need DeviceType? Consumer can query the Handle to understand the
> > device type.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add DeviceSecurity.h
[not found] ` <15D4BEC95EBB70CB.18056@groups.io>
@ 2019-11-07 4:31 ` Ni, Ray
2019-11-07 7:13 ` Yao, Jiewen
0 siblings, 1 reply; 20+ messages in thread
From: Ni, Ray @ 2019-11-07 4:31 UTC (permalink / raw)
To: devel@edk2.groups.io, Ni, Ray, Yao, Jiewen
Cc: Wang, Jian J, Wu, Hao A, Lou, Yun
Jiewen,
"xxx_DEVICE_IDENTIFIER_TYPE_xxx" sounds a bit strange to me because
I feel "IDENTIFIER" and "TYPE" are different properties to describe a device.
Or you think some other GUIDs may be defined in future as the device ID?
Thanks,
Ray
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Thursday, November 7, 2019 9:59 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Lou, Yun <yun.lou@intel.com>
> Subject: Re: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add
> DeviceSecurity.h
>
> I see. PciIo or UsbIo instance is installed with a different GUID and consumer
> needs to parse the device path to understand what kind of instance (PciIo or
> UsbIo) should be used.
>
> > -----Original Message-----
> > From: Yao, Jiewen <jiewen.yao@intel.com>
> > Sent: Wednesday, November 6, 2019 4:25 PM
> > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > <hao.a.wu@intel.com>; Lou, Yun <yun.lou@intel.com>
> > Subject: RE: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add
> > DeviceSecurity.h
> >
> > HI
> > Good comment. Let me answer it in 2 parts.
> >
> > 1) The consumer may locate the deice path to know the device type. In
> > this part, you can treat this as redundant information.
> >
> > 2) But we still need a new GUID, because I will install the device
> > access protocol on this new GUID for the temporary access for the
> authentication driver only.
> >
> > I don't want to install the device access protocol to the original
> > UEFI spec defined GUID to notify everyone that the device is ready to
> > use, because I have seen some device drivers have callback function (such
> as ATA passthru, or NVME passthru) to start access the device once the
> device access protocol is installed.
> >
> >
> > Thank you
> > Yao Jiewen
> >
> > > -----Original Message-----
> > > From: Ni, Ray <ray.ni@intel.com>
> > > Sent: Wednesday, November 6, 2019 3:56 PM
> > > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > > <hao.a.wu@intel.com>; Lou, Yun <yun.lou@intel.com>
> > > Subject: RE: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add
> > > DeviceSecurity.h
> > >
> > > > + ///
> > > > + /// Type of the device.
> > > > + /// This field is also served as a device Access protocol GUID.
> > > > + /// The device access protocol is installed on the DeviceHandle.
> > > > + /// The device access protocol is device specific.
> > > > + /// EDKII_DEVICE_IDENTIFIER_TYPE_PCI_GUID means the device
> access
> > > protocol is PciIo.
> > > > + /// EDKII_DEVICE_IDENTIFIER_TYPE_USB_GUID means the device
> access
> > > protocol is UsbIo.
> > > > + ///
> > > > + EFI_GUID DeviceType;
> > >
> > > Do we still need DeviceType? Consumer can query the Handle to
> > > understand the device type.
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V2 4/4] MdeModulePkg/Pci: Add DeviceSecurity support.
2019-10-31 12:30 ` [PATCH V2 4/4] MdeModulePkg/Pci: Add DeviceSecurity support Yao, Jiewen
@ 2019-11-07 4:42 ` Ni, Ray
2019-11-07 7:05 ` Yao, Jiewen
0 siblings, 1 reply; 20+ messages in thread
From: Ni, Ray @ 2019-11-07 4:42 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io; +Cc: Wang, Jian J, Wu, Hao A, Lou, Yun
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Thursday, October 31, 2019 8:30 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Ni, Ray <ray.ni@intel.com>; Lou, Yun <yun.lou@intel.com>
> Subject: [PATCH V2 4/4] MdeModulePkg/Pci: Add DeviceSecurity support.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303
>
> Whenever a PCI device is discovered, PCI bus calls the
> EDKII_DEVICE_SECURITY_PROTOCOL to authenticate it.
> If the function returns success, the PCI bus allocates the resource and installs
> the PCI_IO for the device.
> If the function returns fail, the PCI bus skips the device.
>
> It is similar to EFI_SECURITY_ARCH_PROTOCOL, which is used to verify an EFI
> image.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Yun Lou <yun.lou@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c | 12 +++-
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 +
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 4 +-
> MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 63
> +++++++++++++++++++-
> MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c | 4 +-
> 5 files changed, 77 insertions(+), 7 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> index b020ce50ce..64284ac825 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> @@ -8,7 +8,7 @@
> PCI Root Bridges. So it means platform needs install PCI Root Bridge IO
> protocol for each
> PCI Root Bus and install PCI Host Bridge Resource Allocation Protocol.
>
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -37,7 +37,7 @@ UINT64 gAllZero = 0;
> EFI_PCI_PLATFORM_PROTOCOL *gPciPlatformProtocol;
> EFI_PCI_OVERRIDE_PROTOCOL *gPciOverrideProtocol;
> EDKII_IOMMU_PROTOCOL *mIoMmuProtocol;
> -
> +EDKII_DEVICE_SECURITY_PROTOCOL *mDeviceSecurityProtocol;
>
> GLOBAL_REMOVE_IF_UNREFERENCED
> EFI_PCI_HOTPLUG_REQUEST_PROTOCOL mPciHotPlugRequest = {
> PciHotPlugRequestNotify
> @@ -293,6 +293,14 @@ PciBusDriverBindingStart (
> );
> }
>
> + if (mDeviceSecurityProtocol == NULL) {
> + gBS->LocateProtocol (
> + &gEdkiiDeviceSecurityProtocolGuid,
> + NULL,
> + (VOID **) &mDeviceSecurityProtocol
> + );
> + }
> +
> if (PcdGetBool (PcdPciDisableBusEnumeration)) {
> gFullEnumeration = FALSE;
> } else {
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> index 504a1b1c12..d4113993c8 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> @@ -27,6 +27,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include
> <Protocol/PciOverride.h> #include <Protocol/PciEnumerationComplete.h>
> #include <Protocol/IoMmu.h>
> +#include <Protocol/DeviceSecurity.h>
>
> #include <Library/DebugLib.h>
> #include <Library/UefiDriverEntryPoint.h> diff --git
> a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> index 05c22025b8..9284998f36 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> @@ -2,7 +2,7 @@
> # The PCI bus driver will probe all PCI devices and allocate MMIO and IO
> space for these devices.
> # Please use PCD feature flag PcdPciBusHotplugDeviceSupport to enable
> hot plug supporting.
> #
> -# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2006 - 2019, Intel Corporation. All rights
> +reserved.<BR>
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -90,6 +90,8 @@
> gEfiIncompatiblePciDeviceSupportProtocolGuid ##
> SOMETIMES_CONSUMES
> gEfiLoadFile2ProtocolGuid ## SOMETIMES_PRODUCES
> gEdkiiIoMmuProtocolGuid ## SOMETIMES_CONSUMES
> + gEdkiiDeviceSecurityProtocolGuid ## SOMETIMES_CONSUMES
> + gEdkiiDeviceIdentifierTypePciGuid ## SOMETIMES_CONSUMES
> gEfiLoadedImageDevicePathProtocolGuid ## CONSUMES
>
> [FeaturePcd]
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> index c7eafff593..df3d1c8fcc 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> @@ -10,6 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include
> "PciBus.h"
>
> extern CHAR16 *mBarTypeStr[];
> +extern EDKII_DEVICE_SECURITY_PROTOCOL
> *mDeviceSecurityProtocol;
>
> #define OLD_ALIGN 0xFFFFFFFFFFFFFFFFULL
> #define EVEN_ALIGN 0xFFFFFFFFFFFFFFFEULL @@ -2092,9 +2093,10 @@
> CreatePciIoDevice (
> IN UINT8 Func
> )
> {
> - PCI_IO_DEVICE *PciIoDevice;
> - EFI_PCI_IO_PROTOCOL *PciIo;
> - EFI_STATUS Status;
> + PCI_IO_DEVICE *PciIoDevice;
> + EFI_PCI_IO_PROTOCOL *PciIo;
> + EFI_STATUS Status;
> + EDKII_DEVICE_IDENTIFIER DeviceIdentifier;
>
> PciIoDevice = AllocateZeroPool (sizeof (PCI_IO_DEVICE));
> if (PciIoDevice == NULL) {
> @@ -2156,6 +2158,61 @@ CreatePciIoDevice (
> PciIoDevice->IsPciExp = TRUE;
> }
>
> + //
> + // Now we can do the authentication check for the device.
> + //
> + if (mDeviceSecurityProtocol != NULL) {
> + //
> + // Prepare the parameter
> + //
> + DeviceIdentifier.Version = EDKII_DEVICE_IDENTIFIER_REVISION;
> + CopyGuid (&DeviceIdentifier.DeviceType,
> &gEdkiiDeviceIdentifierTypePciGuid);
> + DeviceIdentifier.DeviceHandle = NULL;
> + Status = gBS->InstallMultipleProtocolInterfaces (
> + &DeviceIdentifier.DeviceHandle,
> + &gEfiDevicePathProtocolGuid,
> + PciIoDevice->DevicePath,
> + &gEdkiiDeviceIdentifierTypePciGuid,
> + &PciIoDevice->PciIo,
> + NULL
> + );
> + if (EFI_ERROR(Status)) {
> + if (PciIoDevice->DevicePath != NULL) {
> + FreePool (PciIoDevice->DevicePath);
> + }
> + FreePool (PciIoDevice);
> + return NULL;
> + }
> +
> + //
> + // Do DeviceAuthentication
> + //
> + Status = mDeviceSecurityProtocol->DeviceAuthenticate
> (mDeviceSecurityProtocol, &DeviceIdentifier);
> + //
> + // Always uninstall, because they are only for Authentication.
> + // No need to check return Status.
> + //
> + gBS->UninstallMultipleProtocolInterfaces (
> + DeviceIdentifier.DeviceHandle,
> + &gEfiDevicePathProtocolGuid,
> + PciIoDevice->DevicePath,
> + &gEdkiiDeviceIdentifierTypePciGuid,
> + &PciIoDevice->PciIo,
> + NULL
> + );
> +
> + //
> + // If authentication fails, skip this device.
> + //
> + if (EFI_ERROR(Status)) {
> + if (PciIoDevice->DevicePath != NULL) {
> + FreePool (PciIoDevice->DevicePath);
> + }
> + FreePool (PciIoDevice);
> + return NULL;
> + }
> + }
> +
Jiewen,
I have no other comments with the logic except one minor request:
Can you please create a standalone function like PciDeviceAuthenticate() and
move the new code to that function then call it from CreatePciIoDevice?
With that, Reviewed-by: Ray Ni <ray.ni@intel.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V2 4/4] MdeModulePkg/Pci: Add DeviceSecurity support.
2019-11-07 4:42 ` Ni, Ray
@ 2019-11-07 7:05 ` Yao, Jiewen
0 siblings, 0 replies; 20+ messages in thread
From: Yao, Jiewen @ 2019-11-07 7:05 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io; +Cc: Wang, Jian J, Wu, Hao A, Lou, Yun
Good idea.
I will do that in V3.
> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Thursday, November 7, 2019 12:42 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Lou, Yun <yun.lou@intel.com>
> Subject: RE: [PATCH V2 4/4] MdeModulePkg/Pci: Add DeviceSecurity support.
>
>
>
> > -----Original Message-----
> > From: Yao, Jiewen <jiewen.yao@intel.com>
> > Sent: Thursday, October 31, 2019 8:30 PM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> > Ni, Ray <ray.ni@intel.com>; Lou, Yun <yun.lou@intel.com>
> > Subject: [PATCH V2 4/4] MdeModulePkg/Pci: Add DeviceSecurity support.
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303
> >
> > Whenever a PCI device is discovered, PCI bus calls the
> > EDKII_DEVICE_SECURITY_PROTOCOL to authenticate it.
> > If the function returns success, the PCI bus allocates the resource and installs
> > the PCI_IO for the device.
> > If the function returns fail, the PCI bus skips the device.
> >
> > It is similar to EFI_SECURITY_ARCH_PROTOCOL, which is used to verify an EFI
> > image.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Yun Lou <yun.lou@intel.com>
> > Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> > ---
> > MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c | 12 +++-
> > MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 +
> > MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 4 +-
> > MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 63
> > +++++++++++++++++++-
> > MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c | 4 +-
> > 5 files changed, 77 insertions(+), 7 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> > index b020ce50ce..64284ac825 100644
> > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> > @@ -8,7 +8,7 @@
> > PCI Root Bridges. So it means platform needs install PCI Root Bridge IO
> > protocol for each
> > PCI Root Bus and install PCI Host Bridge Resource Allocation Protocol.
> >
> > -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> > +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > **/
> > @@ -37,7 +37,7 @@ UINT64 gAllZero = 0;
> > EFI_PCI_PLATFORM_PROTOCOL *gPciPlatformProtocol;
> > EFI_PCI_OVERRIDE_PROTOCOL *gPciOverrideProtocol;
> > EDKII_IOMMU_PROTOCOL *mIoMmuProtocol;
> > -
> > +EDKII_DEVICE_SECURITY_PROTOCOL *mDeviceSecurityProtocol;
> >
> > GLOBAL_REMOVE_IF_UNREFERENCED
> > EFI_PCI_HOTPLUG_REQUEST_PROTOCOL mPciHotPlugRequest = {
> > PciHotPlugRequestNotify
> > @@ -293,6 +293,14 @@ PciBusDriverBindingStart (
> > );
> > }
> >
> > + if (mDeviceSecurityProtocol == NULL) {
> > + gBS->LocateProtocol (
> > + &gEdkiiDeviceSecurityProtocolGuid,
> > + NULL,
> > + (VOID **) &mDeviceSecurityProtocol
> > + );
> > + }
> > +
> > if (PcdGetBool (PcdPciDisableBusEnumeration)) {
> > gFullEnumeration = FALSE;
> > } else {
> > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> > index 504a1b1c12..d4113993c8 100644
> > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> > @@ -27,6 +27,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include
> > <Protocol/PciOverride.h> #include <Protocol/PciEnumerationComplete.h>
> > #include <Protocol/IoMmu.h>
> > +#include <Protocol/DeviceSecurity.h>
> >
> > #include <Library/DebugLib.h>
> > #include <Library/UefiDriverEntryPoint.h> diff --git
> > a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> > index 05c22025b8..9284998f36 100644
> > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> > @@ -2,7 +2,7 @@
> > # The PCI bus driver will probe all PCI devices and allocate MMIO and IO
> > space for these devices.
> > # Please use PCD feature flag PcdPciBusHotplugDeviceSupport to enable
> > hot plug supporting.
> > #
> > -# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2006 - 2019, Intel Corporation. All rights
> > +reserved.<BR>
> > #
> > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -90,6 +90,8 @@
> > gEfiIncompatiblePciDeviceSupportProtocolGuid ##
> > SOMETIMES_CONSUMES
> > gEfiLoadFile2ProtocolGuid ## SOMETIMES_PRODUCES
> > gEdkiiIoMmuProtocolGuid ## SOMETIMES_CONSUMES
> > + gEdkiiDeviceSecurityProtocolGuid ## SOMETIMES_CONSUMES
> > + gEdkiiDeviceIdentifierTypePciGuid ## SOMETIMES_CONSUMES
> > gEfiLoadedImageDevicePathProtocolGuid ## CONSUMES
> >
> > [FeaturePcd]
> > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > index c7eafff593..df3d1c8fcc 100644
> > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > @@ -10,6 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include
> > "PciBus.h"
> >
> > extern CHAR16 *mBarTypeStr[];
> > +extern EDKII_DEVICE_SECURITY_PROTOCOL
> > *mDeviceSecurityProtocol;
> >
> > #define OLD_ALIGN 0xFFFFFFFFFFFFFFFFULL
> > #define EVEN_ALIGN 0xFFFFFFFFFFFFFFFEULL @@ -2092,9 +2093,10 @@
> > CreatePciIoDevice (
> > IN UINT8 Func
> > )
> > {
> > - PCI_IO_DEVICE *PciIoDevice;
> > - EFI_PCI_IO_PROTOCOL *PciIo;
> > - EFI_STATUS Status;
> > + PCI_IO_DEVICE *PciIoDevice;
> > + EFI_PCI_IO_PROTOCOL *PciIo;
> > + EFI_STATUS Status;
> > + EDKII_DEVICE_IDENTIFIER DeviceIdentifier;
> >
> > PciIoDevice = AllocateZeroPool (sizeof (PCI_IO_DEVICE));
> > if (PciIoDevice == NULL) {
> > @@ -2156,6 +2158,61 @@ CreatePciIoDevice (
> > PciIoDevice->IsPciExp = TRUE;
> > }
> >
> > + //
> > + // Now we can do the authentication check for the device.
> > + //
> > + if (mDeviceSecurityProtocol != NULL) {
> > + //
> > + // Prepare the parameter
> > + //
> > + DeviceIdentifier.Version = EDKII_DEVICE_IDENTIFIER_REVISION;
> > + CopyGuid (&DeviceIdentifier.DeviceType,
> > &gEdkiiDeviceIdentifierTypePciGuid);
> > + DeviceIdentifier.DeviceHandle = NULL;
> > + Status = gBS->InstallMultipleProtocolInterfaces (
> > + &DeviceIdentifier.DeviceHandle,
> > + &gEfiDevicePathProtocolGuid,
> > + PciIoDevice->DevicePath,
> > + &gEdkiiDeviceIdentifierTypePciGuid,
> > + &PciIoDevice->PciIo,
> > + NULL
> > + );
> > + if (EFI_ERROR(Status)) {
> > + if (PciIoDevice->DevicePath != NULL) {
> > + FreePool (PciIoDevice->DevicePath);
> > + }
> > + FreePool (PciIoDevice);
> > + return NULL;
> > + }
> > +
> > + //
> > + // Do DeviceAuthentication
> > + //
> > + Status = mDeviceSecurityProtocol->DeviceAuthenticate
> > (mDeviceSecurityProtocol, &DeviceIdentifier);
> > + //
> > + // Always uninstall, because they are only for Authentication.
> > + // No need to check return Status.
> > + //
> > + gBS->UninstallMultipleProtocolInterfaces (
> > + DeviceIdentifier.DeviceHandle,
> > + &gEfiDevicePathProtocolGuid,
> > + PciIoDevice->DevicePath,
> > + &gEdkiiDeviceIdentifierTypePciGuid,
> > + &PciIoDevice->PciIo,
> > + NULL
> > + );
> > +
> > + //
> > + // If authentication fails, skip this device.
> > + //
> > + if (EFI_ERROR(Status)) {
> > + if (PciIoDevice->DevicePath != NULL) {
> > + FreePool (PciIoDevice->DevicePath);
> > + }
> > + FreePool (PciIoDevice);
> > + return NULL;
> > + }
> > + }
> > +
>
> Jiewen,
> I have no other comments with the logic except one minor request:
> Can you please create a standalone function like PciDeviceAuthenticate() and
> move the new code to that function then call it from CreatePciIoDevice?
> With that, Reviewed-by: Ray Ni <ray.ni@intel.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add DeviceSecurity.h
2019-11-07 4:31 ` Ni, Ray
@ 2019-11-07 7:13 ` Yao, Jiewen
2019-11-07 7:16 ` Ni, Ray
0 siblings, 1 reply; 20+ messages in thread
From: Yao, Jiewen @ 2019-11-07 7:13 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io; +Cc: Wang, Jian J, Wu, Hao A, Lou, Yun
I have thought that before.
The rule I take is always to add the original data structure name as prefix, to avoid name space collision.
The meaning of " EDKII_DEVICE_IDENTIFIER_TYPE_PCI_GUID" is actually "EDKII_DEVICE_IDENTIFIER.TYPE_PCI_GUID"
I have a little concern if I name it to "EDKII_DEVICE_TYPE_PCI", it may conflict with other generic name.
As such, I prefer to keep the current definition.
Thank you
Yao Jiewen
> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Thursday, November 7, 2019 12:32 PM
> To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Lou, Yun <yun.lou@intel.com>
> Subject: RE: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add
> DeviceSecurity.h
>
> Jiewen,
> "xxx_DEVICE_IDENTIFIER_TYPE_xxx" sounds a bit strange to me because
> I feel "IDENTIFIER" and "TYPE" are different properties to describe a device.
> Or you think some other GUIDs may be defined in future as the device ID?
>
> Thanks,
> Ray
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> > Sent: Thursday, November 7, 2019 9:59 AM
> > To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> > Lou, Yun <yun.lou@intel.com>
> > Subject: Re: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add
> > DeviceSecurity.h
> >
> > I see. PciIo or UsbIo instance is installed with a different GUID and consumer
> > needs to parse the device path to understand what kind of instance (PciIo or
> > UsbIo) should be used.
> >
> > > -----Original Message-----
> > > From: Yao, Jiewen <jiewen.yao@intel.com>
> > > Sent: Wednesday, November 6, 2019 4:25 PM
> > > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > > <hao.a.wu@intel.com>; Lou, Yun <yun.lou@intel.com>
> > > Subject: RE: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add
> > > DeviceSecurity.h
> > >
> > > HI
> > > Good comment. Let me answer it in 2 parts.
> > >
> > > 1) The consumer may locate the deice path to know the device type. In
> > > this part, you can treat this as redundant information.
> > >
> > > 2) But we still need a new GUID, because I will install the device
> > > access protocol on this new GUID for the temporary access for the
> > authentication driver only.
> > >
> > > I don't want to install the device access protocol to the original
> > > UEFI spec defined GUID to notify everyone that the device is ready to
> > > use, because I have seen some device drivers have callback function (such
> > as ATA passthru, or NVME passthru) to start access the device once the
> > device access protocol is installed.
> > >
> > >
> > > Thank you
> > > Yao Jiewen
> > >
> > > > -----Original Message-----
> > > > From: Ni, Ray <ray.ni@intel.com>
> > > > Sent: Wednesday, November 6, 2019 3:56 PM
> > > > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > > > <hao.a.wu@intel.com>; Lou, Yun <yun.lou@intel.com>
> > > > Subject: RE: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add
> > > > DeviceSecurity.h
> > > >
> > > > > + ///
> > > > > + /// Type of the device.
> > > > > + /// This field is also served as a device Access protocol GUID.
> > > > > + /// The device access protocol is installed on the DeviceHandle.
> > > > > + /// The device access protocol is device specific.
> > > > > + /// EDKII_DEVICE_IDENTIFIER_TYPE_PCI_GUID means the device
> > access
> > > > protocol is PciIo.
> > > > > + /// EDKII_DEVICE_IDENTIFIER_TYPE_USB_GUID means the device
> > access
> > > > protocol is UsbIo.
> > > > > + ///
> > > > > + EFI_GUID DeviceType;
> > > >
> > > > Do we still need DeviceType? Consumer can query the Handle to
> > > > understand the device type.
> >
> >
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add DeviceSecurity.h
2019-11-07 7:13 ` Yao, Jiewen
@ 2019-11-07 7:16 ` Ni, Ray
0 siblings, 0 replies; 20+ messages in thread
From: Ni, Ray @ 2019-11-07 7:16 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io; +Cc: Wang, Jian J, Wu, Hao A, Lou, Yun
I didn't think about that naming space issue. I agree with your current name.
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Thursday, November 7, 2019 3:13 PM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Lou, Yun <yun.lou@intel.com>
> Subject: RE: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add
> DeviceSecurity.h
>
> I have thought that before.
>
> The rule I take is always to add the original data structure name as prefix, to
> avoid name space collision.
>
> The meaning of " EDKII_DEVICE_IDENTIFIER_TYPE_PCI_GUID" is actually
> "EDKII_DEVICE_IDENTIFIER.TYPE_PCI_GUID"
>
> I have a little concern if I name it to "EDKII_DEVICE_TYPE_PCI", it may conflict
> with other generic name.
>
> As such, I prefer to keep the current definition.
>
> Thank you
> Yao Jiewen
>
>
> > -----Original Message-----
> > From: Ni, Ray <ray.ni@intel.com>
> > Sent: Thursday, November 7, 2019 12:32 PM
> > To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Yao, Jiewen
> > <jiewen.yao@intel.com>
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > <hao.a.wu@intel.com>; Lou, Yun <yun.lou@intel.com>
> > Subject: RE: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add
> > DeviceSecurity.h
> >
> > Jiewen,
> > "xxx_DEVICE_IDENTIFIER_TYPE_xxx" sounds a bit strange to me because I
> > feel "IDENTIFIER" and "TYPE" are different properties to describe a device.
> > Or you think some other GUIDs may be defined in future as the device ID?
> >
> > Thanks,
> > Ray
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni,
> > > Ray
> > > Sent: Thursday, November 7, 2019 9:59 AM
> > > To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > > <hao.a.wu@intel.com>; Lou, Yun <yun.lou@intel.com>
> > > Subject: Re: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add
> > > DeviceSecurity.h
> > >
> > > I see. PciIo or UsbIo instance is installed with a different GUID
> > > and consumer needs to parse the device path to understand what kind
> > > of instance (PciIo or
> > > UsbIo) should be used.
> > >
> > > > -----Original Message-----
> > > > From: Yao, Jiewen <jiewen.yao@intel.com>
> > > > Sent: Wednesday, November 6, 2019 4:25 PM
> > > > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> > > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > > > <hao.a.wu@intel.com>; Lou, Yun <yun.lou@intel.com>
> > > > Subject: RE: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add
> > > > DeviceSecurity.h
> > > >
> > > > HI
> > > > Good comment. Let me answer it in 2 parts.
> > > >
> > > > 1) The consumer may locate the deice path to know the device type.
> > > > In this part, you can treat this as redundant information.
> > > >
> > > > 2) But we still need a new GUID, because I will install the device
> > > > access protocol on this new GUID for the temporary access for the
> > > authentication driver only.
> > > >
> > > > I don't want to install the device access protocol to the original
> > > > UEFI spec defined GUID to notify everyone that the device is ready
> > > > to use, because I have seen some device drivers have callback
> > > > function (such
> > > as ATA passthru, or NVME passthru) to start access the device once
> > > the device access protocol is installed.
> > > >
> > > >
> > > > Thank you
> > > > Yao Jiewen
> > > >
> > > > > -----Original Message-----
> > > > > From: Ni, Ray <ray.ni@intel.com>
> > > > > Sent: Wednesday, November 6, 2019 3:56 PM
> > > > > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > > > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > > > > <hao.a.wu@intel.com>; Lou, Yun <yun.lou@intel.com>
> > > > > Subject: RE: [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include:
> > > > > Add DeviceSecurity.h
> > > > >
> > > > > > + ///
> > > > > > + /// Type of the device.
> > > > > > + /// This field is also served as a device Access protocol GUID.
> > > > > > + /// The device access protocol is installed on the DeviceHandle.
> > > > > > + /// The device access protocol is device specific.
> > > > > > + /// EDKII_DEVICE_IDENTIFIER_TYPE_PCI_GUID means the
> device
> > > access
> > > > > protocol is PciIo.
> > > > > > + /// EDKII_DEVICE_IDENTIFIER_TYPE_USB_GUID means the
> device
> > > access
> > > > > protocol is UsbIo.
> > > > > > + ///
> > > > > > + EFI_GUID DeviceType;
> > > > >
> > > > > Do we still need DeviceType? Consumer can query the Handle to
> > > > > understand the device type.
> > >
> > >
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2019-11-07 7:16 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-10-31 12:30 [PATCH V2 0/4] Add SPDM device security Yao, Jiewen
2019-10-31 12:30 ` [PATCH V2 1/4] MdePkg/Include: Add DMTF SPDM definition Yao, Jiewen
2019-11-06 14:38 ` Liming Gao
2019-11-07 0:25 ` Yao, Jiewen
[not found] ` <15D4B9B24059B4F1.19610@groups.io>
2019-11-07 0:57 ` [edk2-devel] " Yao, Jiewen
2019-10-31 12:30 ` [PATCH V2 2/4] MdeModulePkg/Include: Add DeviceSecurity.h Yao, Jiewen
2019-11-06 7:55 ` [edk2-devel] " Ni, Ray
2019-11-06 8:25 ` Yao, Jiewen
2019-11-07 1:58 ` Ni, Ray
[not found] ` <15D4BEC95EBB70CB.18056@groups.io>
2019-11-07 4:31 ` Ni, Ray
2019-11-07 7:13 ` Yao, Jiewen
2019-11-07 7:16 ` Ni, Ray
2019-10-31 12:30 ` [PATCH V2 3/4] MdeModulePkg/dec: Add EdkiiDeviceSecurityProtocolGuid Yao, Jiewen
2019-10-31 12:30 ` [PATCH V2 4/4] MdeModulePkg/Pci: Add DeviceSecurity support Yao, Jiewen
2019-11-07 4:42 ` Ni, Ray
2019-11-07 7:05 ` Yao, Jiewen
[not found] ` <15D2BB2D773DBDBA.23805@groups.io>
2019-11-06 6:47 ` [edk2-devel] [PATCH V2 1/4] MdePkg/Include: Add DMTF SPDM definition Yao, Jiewen
[not found] ` <15D2BB2E5CC7FD95.31603@groups.io>
2019-11-06 6:47 ` [edk2-devel] [PATCH V2 4/4] MdeModulePkg/Pci: Add DeviceSecurity support Yao, Jiewen
[not found] ` <15D2BB2DC41C838D.31603@groups.io>
2019-11-06 6:47 ` [edk2-devel] [PATCH V2 2/4] MdeModulePkg/Include: Add DeviceSecurity.h Yao, Jiewen
[not found] ` <15D2BB2E0995721D.31603@groups.io>
2019-11-06 6:47 ` [edk2-devel] [PATCH V2 3/4] MdeModulePkg/dec: Add EdkiiDeviceSecurityProtocolGuid Yao, Jiewen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox