Read the list of trusted cipher suites from fw_cfg and to store it to
EFI_TLS_CA_CERTIFICATE_VARIABLE.

The fw_cfg file is formatted by the "update-crypto-policies" utility on
the host side, so that the host settings take effect in guest HTTPS boot
as well. QEMU forwards the file intact to the firmware. The contents are
forwarded by NetworkPkg/HttpDxe (in TlsConfigCipherList()) to
NetworkPkg/TlsDxe (TlsSetSessionData()) and TlsLib (TlsSetCipherList()).

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Gary Ching-Pang Lin <glin@suse.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v2:
    - no change

 OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf |  3 +-
 OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.c   | 98 ++++++++++++++++++++
 2 files changed, 100 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf b/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf
index 5f83582a8313..40754ea5a2f3 100644
--- a/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf
+++ b/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf
@@ -46,10 +46,11 @@ [LibraryClasses]
   DebugLib
   MemoryAllocationLib
   QemuFwCfgLib
   UefiRuntimeServicesTableLib
 
 [Guids]
-  gEfiTlsCaCertificateGuid ## PRODUCES ## Variable:L"TlsCaCertificate"
+  gEdkiiHttpTlsCipherListGuid ## PRODUCES ## Variable:L"HttpTlsCipherList"
+  gEfiTlsCaCertificateGuid    ## PRODUCES ## Variable:L"TlsCaCertificate"
 
 [Depex]
   gEfiVariableWriteArchProtocolGuid
diff --git a/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.c b/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.c
index b5b33bc4fc69..74c393e5462f 100644
--- a/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.c
+++ b/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.c
@@ -17,12 +17,13 @@
 
 **/
 
 #include <Uefi/UefiBaseType.h>
 #include <Uefi/UefiSpec.h>
 
+#include <Guid/HttpTlsCipherList.h>
 #include <Guid/TlsAuthentication.h>
 
 #include <Library/BaseLib.h>
 #include <Library/DebugLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/QemuFwCfgLib.h>
@@ -118,16 +119,113 @@ SetCaCerts (
     gEfiCallerBaseName, __FUNCTION__, (UINT64)HttpsCaCertsSize));
 
 FreeHttpsCaCerts:
   FreePool (HttpsCaCerts);
 }
 
+/**
+  Read the list of trusted cipher suites from the fw_cfg file
+  "etc/edk2/https/ciphers", and store it to
+  gEdkiiHttpTlsCipherListGuid:EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE.
+
+  The contents are propagated by NetworkPkg/HttpDxe to NetworkPkg/TlsDxe; the
+  list is processed by the latter.
+**/
+STATIC
+VOID
+SetCipherSuites (
+  VOID
+  )
+{
+  EFI_STATUS           Status;
+  FIRMWARE_CONFIG_ITEM HttpsCiphersItem;
+  UINTN                HttpsCiphersSize;
+  VOID                 *HttpsCiphers;
+
+  Status = QemuFwCfgFindFile ("etc/edk2/https/ciphers", &HttpsCiphersItem,
+             &HttpsCiphersSize);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_VERBOSE, "%a:%a: not touching cipher suites\n",
+      gEfiCallerBaseName, __FUNCTION__));
+    return;
+  }
+  //
+  // From this point on, any failure is fatal. An ordered cipher preference
+  // list is available from QEMU, thus we cannot let the firmware attempt HTTPS
+  // boot with either pre-existent or non-existent preferences. An empty set of
+  // cipher suites does not fail HTTPS boot automatically; the default cipher
+  // suite preferences would take effect, and we must prevent that.
+  //
+  // Delete the current EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE if it exists. If
+  // the variable exists with EFI_VARIABLE_NON_VOLATILE attribute, we cannot
+  // make it volatile without deleting it first.
+  //
+  Status = gRT->SetVariable (
+                  EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE, // VariableName
+                  &gEdkiiHttpTlsCipherListGuid,        // VendorGuid
+                  0,                                   // Attributes
+                  0,                                   // DataSize
+                  NULL                                 // Data
+                  );
+  if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
+    DEBUG ((DEBUG_ERROR, "%a:%a: failed to delete %g:\"%s\"\n",
+      gEfiCallerBaseName, __FUNCTION__, &gEdkiiHttpTlsCipherListGuid,
+      EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE));
+    goto Done;
+  }
+
+  if (HttpsCiphersSize == 0) {
+    DEBUG ((DEBUG_ERROR, "%a:%a: list of cipher suites must not be empty\n",
+      gEfiCallerBaseName, __FUNCTION__));
+    Status = EFI_INVALID_PARAMETER;
+    goto Done;
+  }
+
+  HttpsCiphers = AllocatePool (HttpsCiphersSize);
+  if (HttpsCiphers == NULL) {
+    DEBUG ((DEBUG_ERROR, "%a:%a: failed to allocate HttpsCiphers\n",
+      gEfiCallerBaseName, __FUNCTION__));
+    Status = EFI_OUT_OF_RESOURCES;
+    goto Done;
+  }
+
+  QemuFwCfgSelectItem (HttpsCiphersItem);
+  QemuFwCfgReadBytes (HttpsCiphersSize, HttpsCiphers);
+
+  Status = gRT->SetVariable (
+                  EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE, // VariableName
+                  &gEdkiiHttpTlsCipherListGuid,        // VendorGuid
+                  EFI_VARIABLE_BOOTSERVICE_ACCESS,     // Attributes
+                  HttpsCiphersSize,                    // DataSize
+                  HttpsCiphers                         // Data
+                  );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a:%a: failed to set %g:\"%s\"\n",
+      gEfiCallerBaseName, __FUNCTION__, &gEdkiiHttpTlsCipherListGuid,
+      EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE));
+    goto FreeHttpsCiphers;
+  }
+
+  DEBUG ((DEBUG_VERBOSE, "%a:%a: stored list of cipher suites (%Lu byte(s))\n",
+    gEfiCallerBaseName, __FUNCTION__, (UINT64)HttpsCiphersSize));
+
+FreeHttpsCiphers:
+  FreePool (HttpsCiphers);
+
+Done:
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    CpuDeadLoop ();
+  }
+}
+
 RETURN_STATUS
 EFIAPI
 TlsAuthConfigInit (
   VOID
   )
 {
   SetCaCerts ();
+  SetCipherSuites ();
 
   return RETURN_SUCCESS;
 }
-- 
2.14.1.3.gb7cf6e02401b


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On Wed, Apr 11, 2018 at 12:42:39PM +0200, Laszlo Ersek wrote:
> Read the list of trusted cipher suites from fw_cfg and to store it to
> EFI_TLS_CA_CERTIFICATE_VARIABLE.
> 
> The fw_cfg file is formatted by the "update-crypto-policies" utility on
> the host side, so that the host settings take effect in guest HTTPS boot
> as well. QEMU forwards the file intact to the firmware. The contents are
> forwarded by NetworkPkg/HttpDxe (in TlsConfigCipherList()) to
> NetworkPkg/TlsDxe (TlsSetSessionData()) and TlsLib (TlsSetCipherList()).
> 
Hi Laszlo,

The description mentioned "update-crypto-policies" to format the cipher
list. The command is not available in openSUSE and I downloaded the command
from github repo[*]. However, I didn't find any command in the repo
could create the binary cipher list. Anyway, I found you also mentioned
"openssl ciphers -V" in the cover letter, and I managed to convert the
plaintext cipher list to the binary array. Maybe the description can be
improved to avoid the confusion. (Or, I just found the wrong program...)

BTW, the code looks good and works for me.

Reviewed-by: Gary Lin <glin@suse.com>
Tested-by: Gary Lin <glin@suse.com>

Cheers,

Gary Lin

[*] https://github.com/nmav/fedora-crypto-policies

> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Gary Ching-Pang Lin <glin@suse.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     v2:
>     - no change
> 
>  OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf |  3 +-
>  OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.c   | 98 ++++++++++++++++++++
>  2 files changed, 100 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf b/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf
> index 5f83582a8313..40754ea5a2f3 100644
> --- a/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf
> +++ b/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf
> @@ -46,10 +46,11 @@ [LibraryClasses]
>    DebugLib
>    MemoryAllocationLib
>    QemuFwCfgLib
>    UefiRuntimeServicesTableLib
>  
>  [Guids]
> -  gEfiTlsCaCertificateGuid ## PRODUCES ## Variable:L"TlsCaCertificate"
> +  gEdkiiHttpTlsCipherListGuid ## PRODUCES ## Variable:L"HttpTlsCipherList"
> +  gEfiTlsCaCertificateGuid    ## PRODUCES ## Variable:L"TlsCaCertificate"
>  
>  [Depex]
>    gEfiVariableWriteArchProtocolGuid
> diff --git a/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.c b/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.c
> index b5b33bc4fc69..74c393e5462f 100644
> --- a/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.c
> +++ b/OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.c
> @@ -17,12 +17,13 @@
>  
>  **/
>  
>  #include <Uefi/UefiBaseType.h>
>  #include <Uefi/UefiSpec.h>
>  
> +#include <Guid/HttpTlsCipherList.h>
>  #include <Guid/TlsAuthentication.h>
>  
>  #include <Library/BaseLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/QemuFwCfgLib.h>
> @@ -118,16 +119,113 @@ SetCaCerts (
>      gEfiCallerBaseName, __FUNCTION__, (UINT64)HttpsCaCertsSize));
>  
>  FreeHttpsCaCerts:
>    FreePool (HttpsCaCerts);
>  }
>  
> +/**
> +  Read the list of trusted cipher suites from the fw_cfg file
> +  "etc/edk2/https/ciphers", and store it to
> +  gEdkiiHttpTlsCipherListGuid:EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE.
> +
> +  The contents are propagated by NetworkPkg/HttpDxe to NetworkPkg/TlsDxe; the
> +  list is processed by the latter.
> +**/
> +STATIC
> +VOID
> +SetCipherSuites (
> +  VOID
> +  )
> +{
> +  EFI_STATUS           Status;
> +  FIRMWARE_CONFIG_ITEM HttpsCiphersItem;
> +  UINTN                HttpsCiphersSize;
> +  VOID                 *HttpsCiphers;
> +
> +  Status = QemuFwCfgFindFile ("etc/edk2/https/ciphers", &HttpsCiphersItem,
> +             &HttpsCiphersSize);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_VERBOSE, "%a:%a: not touching cipher suites\n",
> +      gEfiCallerBaseName, __FUNCTION__));
> +    return;
> +  }
> +  //
> +  // From this point on, any failure is fatal. An ordered cipher preference
> +  // list is available from QEMU, thus we cannot let the firmware attempt HTTPS
> +  // boot with either pre-existent or non-existent preferences. An empty set of
> +  // cipher suites does not fail HTTPS boot automatically; the default cipher
> +  // suite preferences would take effect, and we must prevent that.
> +  //
> +  // Delete the current EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE if it exists. If
> +  // the variable exists with EFI_VARIABLE_NON_VOLATILE attribute, we cannot
> +  // make it volatile without deleting it first.
> +  //
> +  Status = gRT->SetVariable (
> +                  EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE, // VariableName
> +                  &gEdkiiHttpTlsCipherListGuid,        // VendorGuid
> +                  0,                                   // Attributes
> +                  0,                                   // DataSize
> +                  NULL                                 // Data
> +                  );
> +  if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
> +    DEBUG ((DEBUG_ERROR, "%a:%a: failed to delete %g:\"%s\"\n",
> +      gEfiCallerBaseName, __FUNCTION__, &gEdkiiHttpTlsCipherListGuid,
> +      EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE));
> +    goto Done;
> +  }
> +
> +  if (HttpsCiphersSize == 0) {
> +    DEBUG ((DEBUG_ERROR, "%a:%a: list of cipher suites must not be empty\n",
> +      gEfiCallerBaseName, __FUNCTION__));
> +    Status = EFI_INVALID_PARAMETER;
> +    goto Done;
> +  }
> +
> +  HttpsCiphers = AllocatePool (HttpsCiphersSize);
> +  if (HttpsCiphers == NULL) {
> +    DEBUG ((DEBUG_ERROR, "%a:%a: failed to allocate HttpsCiphers\n",
> +      gEfiCallerBaseName, __FUNCTION__));
> +    Status = EFI_OUT_OF_RESOURCES;
> +    goto Done;
> +  }
> +
> +  QemuFwCfgSelectItem (HttpsCiphersItem);
> +  QemuFwCfgReadBytes (HttpsCiphersSize, HttpsCiphers);
> +
> +  Status = gRT->SetVariable (
> +                  EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE, // VariableName
> +                  &gEdkiiHttpTlsCipherListGuid,        // VendorGuid
> +                  EFI_VARIABLE_BOOTSERVICE_ACCESS,     // Attributes
> +                  HttpsCiphersSize,                    // DataSize
> +                  HttpsCiphers                         // Data
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "%a:%a: failed to set %g:\"%s\"\n",
> +      gEfiCallerBaseName, __FUNCTION__, &gEdkiiHttpTlsCipherListGuid,
> +      EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE));
> +    goto FreeHttpsCiphers;
> +  }
> +
> +  DEBUG ((DEBUG_VERBOSE, "%a:%a: stored list of cipher suites (%Lu byte(s))\n",
> +    gEfiCallerBaseName, __FUNCTION__, (UINT64)HttpsCiphersSize));
> +
> +FreeHttpsCiphers:
> +  FreePool (HttpsCiphers);
> +
> +Done:
> +  if (EFI_ERROR (Status)) {
> +    ASSERT_EFI_ERROR (Status);
> +    CpuDeadLoop ();
> +  }
> +}
> +
>  RETURN_STATUS
>  EFIAPI
>  TlsAuthConfigInit (
>    VOID
>    )
>  {
>    SetCaCerts ();
> +  SetCipherSuites ();
>  
>    return RETURN_SUCCESS;
>  }
> -- 
> 2.14.1.3.gb7cf6e02401b
> 
> 
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On 04/12/18 09:08, Gary Lin wrote:
> On Wed, Apr 11, 2018 at 12:42:39PM +0200, Laszlo Ersek wrote:
>> Read the list of trusted cipher suites from fw_cfg and to store it to
>> EFI_TLS_CA_CERTIFICATE_VARIABLE.
>>
>> The fw_cfg file is formatted by the "update-crypto-policies" utility on
>> the host side, so that the host settings take effect in guest HTTPS boot
>> as well. QEMU forwards the file intact to the firmware. The contents are
>> forwarded by NetworkPkg/HttpDxe (in TlsConfigCipherList()) to
>> NetworkPkg/TlsDxe (TlsSetSessionData()) and TlsLib (TlsSetCipherList()).
>>
> Hi Laszlo,
> 
> The description mentioned "update-crypto-policies" to format the cipher
> list. The command is not available in openSUSE and I downloaded the command
> from github repo[*]. However, I didn't find any command in the repo
> could create the binary cipher list.

Right, that feature is underway, and the Crypto team has agreed to
implement it for me. My apologies for being unclear about it. Until
then, a small shell script like the following can be used:

-----
export LC_ALL=C

openssl ciphers -V \
| sed -r -n \
    -e 's/^ *0x([0-9A-F]{2}),0x([0-9A-F]{2}) - .*$/\\\\x\1 \\\\x\2/p' \
| xargs -r -- printf -- '%b' > ciphers.bin
-----

> Anyway, I found you also mentioned
> "openssl ciphers -V" in the cover letter, and I managed to convert the
> plaintext cipher list to the binary array. Maybe the description can be
> improved to avoid the confusion. (Or, I just found the wrong program...)

No, you are right; I figured I'd describe the end-state in the commit
mesage. I guess I can replace

  The fw_cfg file is formatted by the "update-crypto-policies" utility

with

  The fw_cfg file will be formatted by the "update-crypto-policies"
  utility

in the commit message.

> 
> BTW, the code looks good and works for me.
> 
> Reviewed-by: Gary Lin <glin@suse.com>
> Tested-by: Gary Lin <glin@suse.com>

Thanks Gary!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On Thu, Apr 12, 2018 at 10:49:15AM +0200, Laszlo Ersek wrote:
> On 04/12/18 09:08, Gary Lin wrote:
> > On Wed, Apr 11, 2018 at 12:42:39PM +0200, Laszlo Ersek wrote:
> >> Read the list of trusted cipher suites from fw_cfg and to store it to
> >> EFI_TLS_CA_CERTIFICATE_VARIABLE.
> >>
> >> The fw_cfg file is formatted by the "update-crypto-policies" utility on
> >> the host side, so that the host settings take effect in guest HTTPS boot
> >> as well. QEMU forwards the file intact to the firmware. The contents are
> >> forwarded by NetworkPkg/HttpDxe (in TlsConfigCipherList()) to
> >> NetworkPkg/TlsDxe (TlsSetSessionData()) and TlsLib (TlsSetCipherList()).
> >>
> > Hi Laszlo,
> > 
> > The description mentioned "update-crypto-policies" to format the cipher
> > list. The command is not available in openSUSE and I downloaded the command
> > from github repo[*]. However, I didn't find any command in the repo
> > could create the binary cipher list.
> 
> Right, that feature is underway, and the Crypto team has agreed to
> implement it for me. My apologies for being unclear about it. Until
> then, a small shell script like the following can be used:
> 
> -----
> export LC_ALL=C
> 
> openssl ciphers -V \
> | sed -r -n \
>     -e 's/^ *0x([0-9A-F]{2}),0x([0-9A-F]{2}) - .*$/\\\\x\1 \\\\x\2/p' \
> | xargs -r -- printf -- '%b' > ciphers.bin
> -----
> 
It would be good to have this script in the description or in the README
so that the person who doesn't have the updated update-crypto-policies,
like me, can easily generate the cipher list.

Cheers,

Gary Lin

> > Anyway, I found you also mentioned
> > "openssl ciphers -V" in the cover letter, and I managed to convert the
> > plaintext cipher list to the binary array. Maybe the description can be
> > improved to avoid the confusion. (Or, I just found the wrong program...)
> 
> No, you are right; I figured I'd describe the end-state in the commit
> mesage. I guess I can replace
> 
>   The fw_cfg file is formatted by the "update-crypto-policies" utility
> 
> with
> 
>   The fw_cfg file will be formatted by the "update-crypto-policies"
>   utility
> 
> in the commit message.
> 
> > 
> > BTW, the code looks good and works for me.
> > 
> > Reviewed-by: Gary Lin <glin@suse.com>
> > Tested-by: Gary Lin <glin@suse.com>
> 
> Thanks Gary!
> Laszlo
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On 04/12/18 11:10, Gary Lin wrote:
> On Thu, Apr 12, 2018 at 10:49:15AM +0200, Laszlo Ersek wrote:
>> On 04/12/18 09:08, Gary Lin wrote:
>>> On Wed, Apr 11, 2018 at 12:42:39PM +0200, Laszlo Ersek wrote:
>>>> Read the list of trusted cipher suites from fw_cfg and to store it to
>>>> EFI_TLS_CA_CERTIFICATE_VARIABLE.
>>>>
>>>> The fw_cfg file is formatted by the "update-crypto-policies" utility on
>>>> the host side, so that the host settings take effect in guest HTTPS boot
>>>> as well. QEMU forwards the file intact to the firmware. The contents are
>>>> forwarded by NetworkPkg/HttpDxe (in TlsConfigCipherList()) to
>>>> NetworkPkg/TlsDxe (TlsSetSessionData()) and TlsLib (TlsSetCipherList()).
>>>>
>>> Hi Laszlo,
>>>
>>> The description mentioned "update-crypto-policies" to format the cipher
>>> list. The command is not available in openSUSE and I downloaded the command
>>> from github repo[*]. However, I didn't find any command in the repo
>>> could create the binary cipher list.
>>
>> Right, that feature is underway, and the Crypto team has agreed to
>> implement it for me. My apologies for being unclear about it. Until
>> then, a small shell script like the following can be used:
>>
>> -----
>> export LC_ALL=C
>>
>> openssl ciphers -V \
>> | sed -r -n \
>>     -e 's/^ *0x([0-9A-F]{2}),0x([0-9A-F]{2}) - .*$/\\\\x\1 \\\\x\2/p' \
>> | xargs -r -- printf -- '%b' > ciphers.bin
>> -----
>>
> It would be good to have this script in the description or in the README
> so that the person who doesn't have the updated update-crypto-policies,
> like me, can easily generate the cipher list.

I can include this in the commit message, sure.

If you think OvmfPkg/README would be a better place for it: can you
please submit a patch? ;) It's not just that I'm overloaded (although I
am), but I always welcome documentation contributions with enthusiasm.
If the documentation captures real life "user stories", that's for the best.

You could introduce an "HTTPS Boot" section to the README, between
"Network Support" and "OVMF Flash Layout". You contributed quite a bit
to HTTPS enablement anyway!

It's up to you, of course :) If you don't have the time, I'll add the
script to the commit message.

Thanks,
Laszlo

> 
> Cheers,
> 
> Gary Lin
> 
>>> Anyway, I found you also mentioned
>>> "openssl ciphers -V" in the cover letter, and I managed to convert the
>>> plaintext cipher list to the binary array. Maybe the description can be
>>> improved to avoid the confusion. (Or, I just found the wrong program...)
>>
>> No, you are right; I figured I'd describe the end-state in the commit
>> mesage. I guess I can replace
>>
>>   The fw_cfg file is formatted by the "update-crypto-policies" utility
>>
>> with
>>
>>   The fw_cfg file will be formatted by the "update-crypto-policies"
>>   utility
>>
>> in the commit message.
>>
>>>
>>> BTW, the code looks good and works for me.
>>>
>>> Reviewed-by: Gary Lin <glin@suse.com>
>>> Tested-by: Gary Lin <glin@suse.com>
>>
>> Thanks Gary!
>> Laszlo
>>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On Thu, Apr 12, 2018 at 11:43:35AM +0200, Laszlo Ersek wrote:
> On 04/12/18 11:10, Gary Lin wrote:
> > On Thu, Apr 12, 2018 at 10:49:15AM +0200, Laszlo Ersek wrote:
> >> On 04/12/18 09:08, Gary Lin wrote:
> >>> On Wed, Apr 11, 2018 at 12:42:39PM +0200, Laszlo Ersek wrote:
> >>>> Read the list of trusted cipher suites from fw_cfg and to store it to
> >>>> EFI_TLS_CA_CERTIFICATE_VARIABLE.
> >>>>
> >>>> The fw_cfg file is formatted by the "update-crypto-policies" utility on
> >>>> the host side, so that the host settings take effect in guest HTTPS boot
> >>>> as well. QEMU forwards the file intact to the firmware. The contents are
> >>>> forwarded by NetworkPkg/HttpDxe (in TlsConfigCipherList()) to
> >>>> NetworkPkg/TlsDxe (TlsSetSessionData()) and TlsLib (TlsSetCipherList()).
> >>>>
> >>> Hi Laszlo,
> >>>
> >>> The description mentioned "update-crypto-policies" to format the cipher
> >>> list. The command is not available in openSUSE and I downloaded the command
> >>> from github repo[*]. However, I didn't find any command in the repo
> >>> could create the binary cipher list.
> >>
> >> Right, that feature is underway, and the Crypto team has agreed to
> >> implement it for me. My apologies for being unclear about it. Until
> >> then, a small shell script like the following can be used:
> >>
> >> -----
> >> export LC_ALL=C
> >>
> >> openssl ciphers -V \
> >> | sed -r -n \
> >>     -e 's/^ *0x([0-9A-F]{2}),0x([0-9A-F]{2}) - .*$/\\\\x\1 \\\\x\2/p' \
> >> | xargs -r -- printf -- '%b' > ciphers.bin
> >> -----
> >>
> > It would be good to have this script in the description or in the README
> > so that the person who doesn't have the updated update-crypto-policies,
> > like me, can easily generate the cipher list.
> 
> I can include this in the commit message, sure.
> 
> If you think OvmfPkg/README would be a better place for it: can you
> please submit a patch? ;) It's not just that I'm overloaded (although I
> am), but I always welcome documentation contributions with enthusiasm.
> If the documentation captures real life "user stories", that's for the best.
> 
> You could introduce an "HTTPS Boot" section to the README, between
> "Network Support" and "OVMF Flash Layout". You contributed quite a bit
> to HTTPS enablement anyway!
> 
Sounds good. I'm also thinking about collecting the fw_cfg entries in
OVMF and documenting them in README. Currently, those entries look like
black magic to the users.

> It's up to you, of course :) If you don't have the time, I'll add the
> script to the commit message.
> 
I can find some time next week. No guarantee though ;)

Thanks,

Gary Lin

> Thanks,
> Laszlo
> 
> > 
> > Cheers,
> > 
> > Gary Lin
> > 
> >>> Anyway, I found you also mentioned
> >>> "openssl ciphers -V" in the cover letter, and I managed to convert the
> >>> plaintext cipher list to the binary array. Maybe the description can be
> >>> improved to avoid the confusion. (Or, I just found the wrong program...)
> >>
> >> No, you are right; I figured I'd describe the end-state in the commit
> >> mesage. I guess I can replace
> >>
> >>   The fw_cfg file is formatted by the "update-crypto-policies" utility
> >>
> >> with
> >>
> >>   The fw_cfg file will be formatted by the "update-crypto-policies"
> >>   utility
> >>
> >> in the commit message.
> >>
> >>>
> >>> BTW, the code looks good and works for me.
> >>>
> >>> Reviewed-by: Gary Lin <glin@suse.com>
> >>> Tested-by: Gary Lin <glin@suse.com>
> >>
> >> Thanks Gary!
> >> Laszlo
> >>
> 
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

(comment/question at the end for Ard and Jordan)

On 04/12/18 12:17, Gary Lin wrote:
> On Thu, Apr 12, 2018 at 11:43:35AM +0200, Laszlo Ersek wrote:
>> On 04/12/18 11:10, Gary Lin wrote:
>>> On Thu, Apr 12, 2018 at 10:49:15AM +0200, Laszlo Ersek wrote:
>>>> On 04/12/18 09:08, Gary Lin wrote:
>>>>> On Wed, Apr 11, 2018 at 12:42:39PM +0200, Laszlo Ersek wrote:
>>>>>> Read the list of trusted cipher suites from fw_cfg and to store it to
>>>>>> EFI_TLS_CA_CERTIFICATE_VARIABLE.
>>>>>>
>>>>>> The fw_cfg file is formatted by the "update-crypto-policies" utility on
>>>>>> the host side, so that the host settings take effect in guest HTTPS boot
>>>>>> as well. QEMU forwards the file intact to the firmware. The contents are
>>>>>> forwarded by NetworkPkg/HttpDxe (in TlsConfigCipherList()) to
>>>>>> NetworkPkg/TlsDxe (TlsSetSessionData()) and TlsLib (TlsSetCipherList()).
>>>>>>
>>>>> Hi Laszlo,
>>>>>
>>>>> The description mentioned "update-crypto-policies" to format the cipher
>>>>> list. The command is not available in openSUSE and I downloaded the command
>>>>> from github repo[*]. However, I didn't find any command in the repo
>>>>> could create the binary cipher list.
>>>>
>>>> Right, that feature is underway, and the Crypto team has agreed to
>>>> implement it for me. My apologies for being unclear about it. Until
>>>> then, a small shell script like the following can be used:
>>>>
>>>> -----
>>>> export LC_ALL=C
>>>>
>>>> openssl ciphers -V \
>>>> | sed -r -n \
>>>>     -e 's/^ *0x([0-9A-F]{2}),0x([0-9A-F]{2}) - .*$/\\\\x\1 \\\\x\2/p' \
>>>> | xargs -r -- printf -- '%b' > ciphers.bin
>>>> -----
>>>>
>>> It would be good to have this script in the description or in the README
>>> so that the person who doesn't have the updated update-crypto-policies,
>>> like me, can easily generate the cipher list.
>>
>> I can include this in the commit message, sure.
>>
>> If you think OvmfPkg/README would be a better place for it: can you
>> please submit a patch? ;) It's not just that I'm overloaded (although I
>> am), but I always welcome documentation contributions with enthusiasm.
>> If the documentation captures real life "user stories", that's for the best.
>>
>> You could introduce an "HTTPS Boot" section to the README, between
>> "Network Support" and "OVMF Flash Layout". You contributed quite a bit
>> to HTTPS enablement anyway!
>>
> Sounds good. I'm also thinking about collecting the fw_cfg entries in
> OVMF and documenting them in README. Currently, those entries look like
> black magic to the users.

Indeed, the fw_cfg entries should ultimately not be used by end-users
directly. If that had been the case, I would have chosen different
fw_cfg pathnames. Please refer to the following file under QEMU:

  docs/specs/fw_cfg.txt
  = Externally Provided Items =

That is, if the -fw_cfg cmdline option had been the intended user
interface for this feature, then I would have chosen an "opt/" prefix. I
chose "etc/" because the pathnames are QEMU-defined (not user-defined),
except the QEMU patches will be written later (and it might not be me
either).

It's too bad that the RHBZs that track this feature (across multiple
components) are all private, so I couldn't link them in the blurb and/or
the commit messages. Those RHBZs provide a more comprehensive
background. They are private because... well don't ask me. I didn't make
them private, they got auto-created like that, and I'm sure you know
that, if *you* flip a BZ from private to public, then the burden is on
*you* to defend your decision facing unpredictable parts of your
organization. While I totally disagree with any of these RHBZs being
private, I know better than to make them public myself :)

>> It's up to you, of course :) If you don't have the time, I'll add the
>> script to the commit message.
>>
> I can find some time next week. No guarantee though ;)

Sure, please take your time. (And thank you for taking on the README
update.) Meanwhile I'll push these patches with the commit message
updates discussed.

Ard, Jordan: Gary tested and reviewed this patch (bunch of kudos for
that!), but still, can one of you guys please ACK it too? I prefer the
patch to go in with maintainer blessing. Similarly to commit
9c7d0d499296 ("OvmfPkg/TlsAuthConfigLib: configure trusted CA certs for
HTTPS boot", 2018-03-30).

Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel