Re: [PATCH V4 01/15] MdeModulePkg/Include: Add FmpAuthenticationLib header.

["Kinney, Michael D" <michael.d.kinney@intel.com>]

Jiewen,

I agree.  Please just update function description.

Mike

From: Yao, Jiewen
Sent: Tuesday, October 25, 2016 5:51 PM
To: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org
Cc: Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>
Subject: RE: [PATCH V4 01/15] MdeModulePkg/Include: Add FmpAuthenticationLib header.

Hi Mike
Let me clarify:

The function is used to do the authentication for FMP capsule based upon   EFI_FIRMWARE_IMAGE_AUTHENTICATION.
This sentence is correct, because the capsule image input should be started by EFI_FIRMWARE_IMAGE_AUTHENTICATION.

LastAttemptStatus is the result of capsule process. The caller is expected convert the return Status to LastAttemptStatus.
It is recorded in ESRT entry and FMP.EFI_FIRMWARE_IMAGE_DESCRIPTOR in next boot.
Current consumer is SignedCapsulePkg\Library\EdkiiSystemCapsuleLib\ EdkiiSystemCapsuleLib.c, ExtractAuthenticatedImage() function.

I agree with you that it is confusing. I will add more sentence to describe. And I do not see any need to update function prototype. Do you think so?

The typo is fixed. Thanks.


From: Kinney, Michael D
Sent: Wednesday, October 26, 2016 7:54 AM
To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; Zhang, Chao B <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
Subject: RE: [PATCH V4 01/15] MdeModulePkg/Include: Add FmpAuthenticationLib header.

Jiewen,

I am confused by the description of this API.

I refers to the LastAttemptStatus field, but that field is not in
EFI_FIRMWARE_IMAGE_AUTHENTICATION structure.  Instead, it is in the
EFI_FIRMWARE_IMAGE_DESCRIPTOR structure.

Is the prototype to this function correct?

Can you also update the description to include which structure the
LastAttemptStatus field is in and how it is found from the input
parameters?

One typo noted inline below.

Thanks,

Mike


> -----Original Message-----
> From: Yao, Jiewen
> Sent: Saturday, October 22, 2016 7:20 PM
> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; Kinney, Michael
> D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; Zhang, Chao B
> <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
> Subject: [PATCH V4 01/15] MdeModulePkg/Include: Add FmpAuthenticationLib header.
>
> This library is used to authenticate a UEFI defined FMP Capsule.
>
> Cc: Feng Tian <feng.tian@intel.com<mailto:feng.tian@intel.com>>
> Cc: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com>>
> Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> Cc: Liming Gao <liming.gao@intel.com<mailto:liming.gao@intel.com>>
> Cc: Chao Zhang <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> Reviewed-by: Liming Gao <liming.gao@intel.com<mailto:liming.gao@intel.com>>
> ---
>  MdeModulePkg/Include/Library/FmpAuthenticationLib.h | 57 ++++++++++++++++++++
>  1 file changed, 57 insertions(+)
>
> diff --git a/MdeModulePkg/Include/Library/FmpAuthenticationLib.h
> b/MdeModulePkg/Include/Library/FmpAuthenticationLib.h
> new file mode 100644
> index 0000000..ed098d4
> --- /dev/null
> +++ b/MdeModulePkg/Include/Library/FmpAuthenticationLib.h
> @@ -0,0 +1,57 @@
> +/** @file
> +  FMP capsule authenitcation Library.
> +
> +Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
> +This program and the accompanying materials
> +are licensed and made available under the terms and conditions of the BSD License
> +which accompanies this distribution.  The full text of the license may be found at
> +http://opensource.org/licenses/bsd-license.php
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +
> +#ifndef __FMP_AUTHENTICATION_LIB_H__
> +#define __FMP_AUTHENTICATION_LIB_H__
> +
> +#include <Protocol/FirmwareManagement.h>
> +
> +/**
> +  The fucntion is used to do the authentication for FMP capsule based upon

Typo.  Should be "The function is".

> +  EFI_FIRMWARE_IMAGE_AUTHENTICATION.
> +
> +  The caller may convert the RETURN_STATUS to ESRT/FMP LastAttemptStatus.
> +
> +  Caution: This function may receive untrusted input.
> +
> +  @param[in]  Image                   Points to an FMP authentication image, started
> from EFI_FIRMWARE_IMAGE_AUTHENTICATION.
> +  @param[in]  ImageSize               Size of the authentication image in bytes.
> +  @param[in]  PublicKeyData           The public key data used to validate the
> signature.
> +  @param[in]  PublicKeyDataLength     The length of the public key data.
> +
> +  @retval RETURN_SUCCESS            Authentication pass.
> +                                    The LastAttemptStatus should be
> LAST_ATTEMPT_STATUS_SUCCESS.
> +  @retval RETURN_SECURITY_VIOLATION Authentication fail.
> +                                    The LastAttemptStatus should be
> LAST_ATTEMPT_STATUS_ERROR_AUTH_ERROR.
> +  @retval RETURN_INVALID_PARAMETER  The image is in an invalid format.
> +                                    The LastAttemptStatus should be
> LAST_ATTEMPT_STATUS_ERROR_INVALID_FORMAT.
> +  @retval RETURN_UNSUPPORTED        No Authentication handler associated with
> CertType.
> +                                    The LastAttemptStatus should be
> LAST_ATTEMPT_STATUS_ERROR_INVALID_FORMAT.
> +  @retval RETURN_UNSUPPORTED        Image or ImageSize is invalid.
> +                                    The LastAttemptStatus should be
> LAST_ATTEMPT_STATUS_ERROR_INVALID_FORMAT.
> +  @retval RETURN_OUT_OF_RESOURCES   No Authentication handler associated with
> CertType.
> +                                    The LastAttemptStatus should be
> LAST_ATTEMPT_STATUS_ERROR_INSUFFICIENT_RESOURCES.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +AuthenticateFmpImage (
> +  IN EFI_FIRMWARE_IMAGE_AUTHENTICATION *Image,
> +  IN UINTN                             ImageSize,
> +  IN CONST UINT8                       *PublicKeyData,
> +  IN UINTN                             PublicKeyDataLength
> +  );
> +
> +#endif
> +
> --
> 2.7.4.windows.1