public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* Re: [edk2-devel] [PATCH] SecurityPkg/RngLibNull: add null version of RngLib
       [not found] <15D6549BEEE2C5E9.20285@groups.io>
@ 2019-11-12  6:01 ` Wang, Jian J
  0 siblings, 0 replies; 2+ messages in thread
From: Wang, Jian J @ 2019-11-12  6:01 UTC (permalink / raw)
  To: devel@edk2.groups.io, Wang, Jian J
  Cc: Yao, Jiewen, Zhang, Chao B, Kinney, Michael D, Gao, Liming,
	Laszlo Ersek, Ard Biesheuvel, Ni, Ray

Required by BZ1871, this null version of RngLib needs to be pushed first to solve
build problem once we removed the dependency of TimerLib from OpensslLib.

I have to send this patch separately from other patches for bz1871 because there
will be patches cross repo.

Regards,
Jian

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wang, Jian
> J
> Sent: Tuesday, November 12, 2019 1:56 PM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Zhang, Chao B
> <chao.b.zhang@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Gao, Liming <liming.gao@intel.com>; Laszlo Ersek <lersek@redhat.com>; Ard
> Biesheuvel <ard.biesheuvel@linaro.org>; Ni, Ray <ray.ni@intel.com>
> Subject: [edk2-devel] [PATCH] SecurityPkg/RngLibNull: add null version of RngLib
> 
> This is null version of RngLib which is used for those platforms or
> components which don't need random number.
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> ---
>  .../RngLibNull/RngLibNull.c                   | 95 +++++++++++++++++++
>  .../RngLibNull/RngLibNull.inf                 | 31 ++++++
>  .../RngLibNull/RngLibNull.uni                 | 14 +++
>  3 files changed, 140 insertions(+)
>  create mode 100644
> SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.c
>  create mode 100644
> SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.inf
>  create mode 100644
> SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.uni
> 
> diff --git a/SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.c
> b/SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.c
> new file mode 100644
> index 0000000000..13677abc84
> --- /dev/null
> +++ b/SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.c
> @@ -0,0 +1,95 @@
> +/** @file
> +  Null version of Random number generator services.
> +
> +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/RngLib.h>
> +
> +/**
> +  Generates a 16-bit random number.
> +
> +  if Rand is NULL, then ASSERT().
> +
> +  @param[out] Rand     Buffer pointer to store the 16-bit random value.
> +
> +  @retval TRUE         Random number generated successfully.
> +  @retval FALSE        Failed to generate the random number.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +GetRandomNumber16 (
> +  OUT     UINT16                    *Rand
> +  )
> +{
> +  ASSERT (FALSE);
> +  return FALSE;
> +}
> +
> +/**
> +  Generates a 32-bit random number.
> +
> +  if Rand is NULL, then ASSERT().
> +
> +  @param[out] Rand     Buffer pointer to store the 32-bit random value.
> +
> +  @retval TRUE         Random number generated successfully.
> +  @retval FALSE        Failed to generate the random number.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +GetRandomNumber32 (
> +  OUT     UINT32                    *Rand
> +  )
> +{
> +  ASSERT (FALSE);
> +  return FALSE;
> +}
> +
> +/**
> +  Generates a 64-bit random number.
> +
> +  if Rand is NULL, then ASSERT().
> +
> +  @param[out] Rand     Buffer pointer to store the 64-bit random value.
> +
> +  @retval TRUE         Random number generated successfully.
> +  @retval FALSE        Failed to generate the random number.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +GetRandomNumber64 (
> +  OUT     UINT64                    *Rand
> +  )
> +{
> +  ASSERT (FALSE);
> +  return FALSE;
> +}
> +
> +/**
> +  Generates a 128-bit random number.
> +
> +  if Rand is NULL, then ASSERT().
> +
> +  @param[out] Rand     Buffer pointer to store the 128-bit random value.
> +
> +  @retval TRUE         Random number generated successfully.
> +  @retval FALSE        Failed to generate the random number.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +GetRandomNumber128 (
> +  OUT     UINT64                    *Rand
> +  )
> +{
> +  ASSERT (FALSE);
> +  return FALSE;
> +}
> diff --git a/SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.inf
> b/SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.inf
> new file mode 100644
> index 0000000000..f6494cdb82
> --- /dev/null
> +++ b/SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.inf
> @@ -0,0 +1,31 @@
> +## @file
> +#  Null instance of RNG (Random Number Generator) Library.
> +#
> +#  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005
> +  BASE_NAME                      = RngLibNull
> +  MODULE_UNI_FILE                = RngLibNull.uni
> +  FILE_GUID                      = CD8991F8-2061-4084-8C9E-9C6F352DC58D
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = RngLib
> +
> +#
> +#  VALID_ARCHITECTURES           = IA32 X64 ARM AARCH64
> +#
> +
> +[Sources]
> +  RngLibNull.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> diff --git a/SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.uni
> b/SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.uni
> new file mode 100644
> index 0000000000..40b2ec3fe1
> --- /dev/null
> +++ b/SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.uni
> @@ -0,0 +1,14 @@
> +// /** @file
> +// Null Instance of RNG (Random Number Generator) Library.
> +//
> +// Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +//
> +// SPDX-License-Identifier: BSD-2-Clause-Patent
> +//
> +// **/
> +
> +
> +#string STR_MODULE_ABSTRACT             #language en-US "Null Instance of
> RNG Library"
> +
> +#string STR_MODULE_DESCRIPTION          #language en-US "Caution: This is a
> null version of RNG library and SHOULD NOT be used on any product ever."
> +
> --
> 2.17.1.windows.2
> 
> 
> 


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [edk2-devel] [PATCH] SecurityPkg/RngLibNull: add null version of RngLib
  2019-11-12  7:50 ` Laszlo Ersek
@ 2019-11-12  7:56   ` Wang, Jian J
  0 siblings, 0 replies; 2+ messages in thread
From: Wang, Jian J @ 2019-11-12  7:56 UTC (permalink / raw)
  To: devel@edk2.groups.io, lersek@redhat.com
  Cc: Yao, Jiewen, Zhang, Chao B, Kinney, Michael D, Gao, Liming,
	Ard Biesheuvel, Ni, Ray

Laszlo,


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
> Sent: Tuesday, November 12, 2019 3:50 PM
> To: Wang, Jian J <jian.j.wang@intel.com>; devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Zhang, Chao B
> <chao.b.zhang@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Gao, Liming <liming.gao@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; Ni, Ray <ray.ni@intel.com>
> Subject: Re: [edk2-devel] [PATCH] SecurityPkg/RngLibNull: add null version of
> RngLib
> 
> On 11/12/19 06:55, Jian J Wang wrote:
> > This is null version of RngLib which is used for those platforms or
> > components which don't need random number.
> >
> > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Chao Zhang <chao.b.zhang@intel.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> > ---
> >  .../RngLibNull/RngLibNull.c                   | 95 +++++++++++++++++++
> >  .../RngLibNull/RngLibNull.inf                 | 31 ++++++
> >  .../RngLibNull/RngLibNull.uni                 | 14 +++
> >  3 files changed, 140 insertions(+)
> >  create mode 100644
> SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.c
> >  create mode 100644
> SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.inf
> >  create mode 100644
> SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.uni
> 
> (1) I don't see any reason why this library instance should not be added
> under MdePkg/Library. The other library instance is already there (and
> the lib class header too is from MdePkg):
> 
>   MdePkg/Library/BaseRngLib
> 
> (2) I think this library instance should be called "BaseRngLibNull", not
> just "RngLibNull".
> 

I have no strong opinion on this.

Liming, do you have any comments?

> More below:
> 
> > diff --git a/SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.c
> b/SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.c
> > new file mode 100644
> > index 0000000000..13677abc84
> > --- /dev/null
> > +++ b/SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.c
> > @@ -0,0 +1,95 @@
> > +/** @file
> > +  Null version of Random number generator services.
> > +
> > +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> > +SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#include <Library/BaseLib.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/RngLib.h>
> > +
> > +/**
> > +  Generates a 16-bit random number.
> > +
> > +  if Rand is NULL, then ASSERT().
> > +
> > +  @param[out] Rand     Buffer pointer to store the 16-bit random value.
> > +
> > +  @retval TRUE         Random number generated successfully.
> > +  @retval FALSE        Failed to generate the random number.
> > +
> > +**/
> > +BOOLEAN
> > +EFIAPI
> > +GetRandomNumber16 (
> > +  OUT     UINT16                    *Rand
> > +  )
> > +{
> > +  ASSERT (FALSE);
> > +  return FALSE;
> > +}
> > +
> > +/**
> > +  Generates a 32-bit random number.
> > +
> > +  if Rand is NULL, then ASSERT().
> > +
> > +  @param[out] Rand     Buffer pointer to store the 32-bit random value.
> > +
> > +  @retval TRUE         Random number generated successfully.
> > +  @retval FALSE        Failed to generate the random number.
> > +
> > +**/
> > +BOOLEAN
> > +EFIAPI
> > +GetRandomNumber32 (
> > +  OUT     UINT32                    *Rand
> > +  )
> > +{
> > +  ASSERT (FALSE);
> > +  return FALSE;
> > +}
> > +
> > +/**
> > +  Generates a 64-bit random number.
> > +
> > +  if Rand is NULL, then ASSERT().
> > +
> > +  @param[out] Rand     Buffer pointer to store the 64-bit random value.
> > +
> > +  @retval TRUE         Random number generated successfully.
> > +  @retval FALSE        Failed to generate the random number.
> > +
> > +**/
> > +BOOLEAN
> > +EFIAPI
> > +GetRandomNumber64 (
> > +  OUT     UINT64                    *Rand
> > +  )
> > +{
> > +  ASSERT (FALSE);
> > +  return FALSE;
> > +}
> > +
> > +/**
> > +  Generates a 128-bit random number.
> > +
> > +  if Rand is NULL, then ASSERT().
> > +
> > +  @param[out] Rand     Buffer pointer to store the 128-bit random value.
> > +
> > +  @retval TRUE         Random number generated successfully.
> > +  @retval FALSE        Failed to generate the random number.
> > +
> > +**/
> > +BOOLEAN
> > +EFIAPI
> > +GetRandomNumber128 (
> > +  OUT     UINT64                    *Rand
> > +  )
> > +{
> > +  ASSERT (FALSE);
> > +  return FALSE;
> > +}
> > diff --git a/SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.inf
> b/SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.inf
> > new file mode 100644
> > index 0000000000..f6494cdb82
> > --- /dev/null
> > +++ b/SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.inf
> > @@ -0,0 +1,31 @@
> > +## @file
> > +#  Null instance of RNG (Random Number Generator) Library.
> > +#
> > +#  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> > +#
> > +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> > +#
> > +##
> > +
> > +[Defines]
> > +  INF_VERSION                    = 0x00010005
> > +  BASE_NAME                      = RngLibNull
> > +  MODULE_UNI_FILE                = RngLibNull.uni
> > +  FILE_GUID                      = CD8991F8-2061-4084-8C9E-9C6F352DC58D
> > +  MODULE_TYPE                    = BASE
> > +  VERSION_STRING                 = 1.0
> > +  LIBRARY_CLASS                  = RngLib
> > +
> > +#
> > +#  VALID_ARCHITECTURES           = IA32 X64 ARM AARCH64
> > +#
> > +
> > +[Sources]
> > +  RngLibNull.c
> > +
> > +[Packages]
> > +  MdePkg/MdePkg.dec
> > +
> > +[LibraryClasses]
> > +  BaseLib
> > +  DebugLib
> > diff --git a/SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.uni
> b/SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.uni
> > new file mode 100644
> > index 0000000000..40b2ec3fe1
> > --- /dev/null
> > +++ b/SecurityPkg/RandomNumberGenerator/RngLibNull/RngLibNull.uni
> > @@ -0,0 +1,14 @@
> > +// /** @file
> > +// Null Instance of RNG (Random Number Generator) Library.
> > +//
> > +// Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> > +//
> > +// SPDX-License-Identifier: BSD-2-Clause-Patent
> > +//
> > +// **/
> > +
> > +
> > +#string STR_MODULE_ABSTRACT             #language en-US "Null Instance of
> RNG Library"
> > +
> > +#string STR_MODULE_DESCRIPTION          #language en-US "Caution: This is a
> null version of RNG library and SHOULD NOT be used on any product ever."
> > +
> >
> 
> (3) I disagree with STR_MODULE_DESCRIPTION.
> 
> This library instance is appropriate even in production, namely for such
> modules that *inherit* a dependency on RngLib -- for example, through
> another library instance --, but, in practice, they never consume
> randomness, and/or they never *must* consume randomness.
> 
> In other words, this library instance should be used with modules that
> should, in practice, never *reach* any calls to GetRandomNumberXX()
> APIs, but it is difficult to remove the call sites themselves -- for
> example, because they are inherited (i.e., indirectly) through another
> library class.
> 
> With that in mind, the ASSERT()s seem justified -- these functions
> should never be reached.
> 
> Note: I'm not saying that the ASSERT()s are *required*. Luckily, all
> these APIs are able to report failure, and so if all client code checks
> the return values, no actual functionality will be misled. (The
> functions in this lib instance all return FALSE, correctly.) But, the
> ASSERT()s are good for pointing out the larger issue: if a module
> actually calls these functions (because it needs actual randomness),
> then the module / platform configuration (= DSC file) is broken.
> 
> In summary, STR_MODULE_DESCRIPTION should state, "this library instance
> should be used with modules that inherit an (indirect) dependency on the
> RngLib class, but never actually call RngLib APIs for consuming randomness".
> 

Good explanation. Thanks. And I agree with your version of STR_MODULE_DESCRIPTION.
I'll update it in v2.

Regards,
Jian

> Thanks,
> Laszlo
> 
> 
> 


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-11-12  7:57 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <15D6549BEEE2C5E9.20285@groups.io>
2019-11-12  6:01 ` [edk2-devel] [PATCH] SecurityPkg/RngLibNull: add null version of RngLib Wang, Jian J
2019-11-12  5:55 Wang, Jian J
2019-11-12  7:50 ` Laszlo Ersek
2019-11-12  7:56   ` [edk2-devel] " Wang, Jian J

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox