Re: [PATCH V2 03/50] MdeModulePkg/Include: Add FmpAuthenticationLib header.

[Sean Brogan <sean.brogan@microsoft.com>]

I think this library and the design of registering different auth handlers is not the right design for FMP auth verification.   This isn't something that needs extension thru registration. This is a controlled environment.  I also don't think the capsule runtime should be using these auth services.  How I see it the design abstraction of FMP is that the FMP instance does the verification and unwrapping of the capsule in its checkimage/set image routines.  By keeping FMP self-contained a platform gains a lot of flexibility.  FMP SetImage can be called from the UEFI shell or other application before exit boot services so it must always verify the image before applying anyway.   
 
I would ask that this too be moved to your new sample package or removed from the design.  

Thanks
Sean
 



> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Jiewen Yao
> Sent: Friday, September 30, 2016 5:21 AM
> To: edk2-devel@lists.01.org
> Cc: Michael D Kinney <michael.d.kinney@intel.com>; Feng Tian
> <feng.tian@intel.com>; Chao Zhang <chao.b.zhang@intel.com>; Liming Gao
> <liming.gao@intel.com>; Star Zeng <star.zeng@intel.com>
> Subject: [edk2] [PATCH V2 03/50] MdeModulePkg/Include: Add
> FmpAuthenticationLib header.
> 
> This library is used to authenticate a UEFI defined FMP Capsule.
> 
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> Reviewed-by: Liming Gao <liming.gao@intel.com>
> ---
>  MdeModulePkg/Include/Library/FmpAuthenticationLib.h | 91
> ++++++++++++++++++++
>  1 file changed, 91 insertions(+)
> 
> diff --git a/MdeModulePkg/Include/Library/FmpAuthenticationLib.h
> b/MdeModulePkg/Include/Library/FmpAuthenticationLib.h
> new file mode 100644
> index 0000000..895698e
> --- /dev/null
> +++ b/MdeModulePkg/Include/Library/FmpAuthenticationLib.h
> @@ -0,0 +1,91 @@
> +/** @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__
> +
> +/**
> +  The handler is used to do the authentication for FMP capsule based
> +upon
> +  EFI_FIRMWARE_IMAGE_AUTHENTICATION.
> +
> +  Caution: This function may receive untrusted input.
> +
> +  @param[in]    Image              Points to the new FMP authentication image,
> +                                   start from EFI_FIRMWARE_IMAGE_AUTHENTICATION.
> +  @param[in]    ImageSize          Size of the authentication image in bytes.
> +  @param[out]   LastAttemptStatus  The last attempt status, which will be
> recorded
> +                                   in ESRT and FMP EFI_FIRMWARE_IMAGE_DESCRIPTOR.
> +
> +  @retval RETURN_SUCCESS            Authentication pass.
> +  @retval RETURN_SECURITY_VIOLATION Authentication fail.
> +                                    The detail reson is recorded in LastAttemptStatus.
> +**/
> +typedef
> +RETURN_STATUS
> +(EFIAPI *FMP_AUTHENTICATION_HANDLER) (
> +  IN VOID                         *Image,
> +  IN UINTN                        ImageSize,
> +  OUT UINT32                      *LastAttemptStatus
> +  );
> +
> +/**
> +  Register FMP authentication handler with CertType.
> +
> +  If CertType is NULL, then ASSERT().
> +  If FmpAuthenticationHandler is NULL, then ASSERT().
> +
> +  @param[in]  CertType                   The certificate type associated with the
> FMP auth handler.
> +  @param[in]  FmpAuthenticationHandler   The FMP authentication handler to
> be registered.
> +
> +  @retval  RETURN_SUCCESS           The handlers were registered.
> +  @retval  RETURN_OUT_OF_RESOURCES  There are not enough resources
> available to register the handlers.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +RegisterFmpAuthenticationHandler(
> +  IN GUID                         *CertType,
> +  IN FMP_AUTHENTICATION_HANDLER   FmpAuthenticationHandler
> +  );
> +
> +/**
> +  Execute FMP authentication handlers.
> +
> +  Caution: This function may receive untrusted input.
> +
> +  If Image is NULL, then ASSERT().
> +  If ImageSize is 0, then ASSERT().
> +  If LastAttemptStatus is NULL, then ASSERT().
> +
> +  @param[in]    Image              Points to the new FMP authentication image,
> +                                   start from EFI_FIRMWARE_IMAGE_AUTHENTICATION.
> +  @param[in]    ImageSize          Size of the authentication image in bytes.
> +  @param[out]   LastAttemptStatus  The last attempt status, which will be
> recorded
> +                                   in ESRT and FMP EFI_FIRMWARE_IMAGE_DESCRIPTOR.
> +
> +  @retval RETURN_SUCCESS            Authentication pass.
> +  @retval RETURN_SECURITY_VIOLATION Authentication fail.
> +                                    The detail reson is recorded in LastAttemptStatus.
> +  @retval RETURN_UNSUPPORTED        No Authentication handler associated
> with CertType.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +ExecuteFmpAuthenticationHandler(
> +  IN  VOID                         *Image,
> +  IN  UINTN                        ImageSize,
> +  OUT UINT32                       *LastAttemptStatus
> +  );
> +
> +#endif
> +
> --
> 2.7.4.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel