public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Wu, Jiaxin" <jiaxin.wu@intel.com>
To: Gary Lin <glin@suse.com>
Cc: "Justen, Jordan L" <jordan.l.justen@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	Laszlo Ersek <lersek@redhat.com>,
	"Long, Qin" <qin.long@intel.com>
Subject: Re: [PATCH] OvmfPkg: Enable HTTPS for Ovmf
Date: Mon, 16 Jan 2017 09:15:14 +0000	[thread overview]
Message-ID: <895558F6EA4E3B41AC93A00D163B727416293FDC@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <20170116064058.nieuzoxlozwjqlcv@GaryWorkstation>

> > More: TLS feature should not be limit to HTTP(S) feature.
> >
> Is there any other planned usage for TLS?

Currently, we only have the HTTP over TLS support, but I think TLS also can be treated as independent module, which can be leveraged by third part drivers/apps (e.g. EAP-TLS). 

> 
> > !if $(HTTP_BOOT_ENABLE) == TRUE
> >    !if $(TLS_ENABLE) == TRUE
> >       ...
> >    !endif
> > !endif
> >
> I checked my patch again and found it'd be better to include the HTTP and
> TLS drivers in this way:
> 
> !if $(HTTP_BOOT_ENABLE) == TRUE || $(TLS_ENABLE) == TRUE
>   <HTTP drivers>
> !endif
> !if $(TLS_ENABLE) == TRUE
>   {TLS drivers}
> !endif
> 
> Therefore, Enabling TLS_ENABLE also means to enable HTTP_BOOT_ENABLE.
> Make it less error-prone.

I don't think there is any issue if we only include the TLS drivers or HTTP driver, but only no TLS means no HTTPS (refer to NT32). So, let's keep the logic clean and easy:). 

> 
> Will send a v2 patch after your patch is merged.

Thanks the contribution.

Jiaxin



> 
> Thanks,
> 
> Gary Lin
> 
> > Best Regard!
> > Jiaxin
> >
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Wu,
> > > Jiaxin
> > > Sent: Monday, January 16, 2017 1:45 PM
> > > To: Gary Lin <glin@suse.com>; edk2-devel@lists.01.org
> > > Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Laszlo Ersek
> > > <lersek@redhat.com>; Long, Qin <qin.long@intel.com>
> > > Subject: Re: [edk2] [PATCH] OvmfPkg: Enable HTTPS for Ovmf
> > >
> > > Hi Gary,
> > >
> > > Before we enable the HTTPS/TLS for OVMF, We need remove the
> > > 'SECURE_BOOT_ENABLE' flag control for the CryptoPkg librarie. Not only
> the
> > > secure boot feature requires the CryptoPkg libraries (e.g, OpensslLib,
> > > BaseCryptLib), but also ISCSI, IpSec and HTTPS/TLS features. If we not
> remove
> > > that dependency, we must set both SECURE_BOOT_ENABLE and
> TLS_ENABLE to
> > > support TLS feature. That's unreasonable.
> > >
> > > Attached patch is to remove the flag control for the CryptoPkg libraries. I
> > > suggest to wait that patch commit, then go ahead to enable the HTTPS for
> > > OVMF.
> > >
> > > Thanks,
> > > Jiaxin
> > >
> > > > -----Original Message-----
> > > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > > Gary
> > > > Lin
> > > > Sent: Monday, January 16, 2017 12:10 PM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Wu, Jiaxin
> > > > <jiaxin.wu@intel.com>; Laszlo Ersek <lersek@redhat.com>
> > > > Subject: [edk2] [PATCH] OvmfPkg: Enable HTTPS for Ovmf
> > > >
> > > > This commit introduces a new build option to OvmfPkg: TLS_ENABLE.
> > > > When setting the option, the TLS drivers will be included to support
> > > > HTTPS.
> > > >
> > > > NOTE: HTTP_BOOT_ENABLE is needed to enable HTTPS support since it's
> > > >       pointless to enable TLS alone.
> > > >
> > > > Cc: Laszlo Ersek <lersek@redhat.com>
> > > > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > > > Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> > > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > > Signed-off-by: Gary Lin <glin@suse.com>
> > > > ---
> > > >  OvmfPkg/OvmfPkgIa32.dsc    | 8 ++++++++
> > > >  OvmfPkg/OvmfPkgIa32.fdf    | 4 ++++
> > > >  OvmfPkg/OvmfPkgIa32X64.dsc | 8 ++++++++
> > > >  OvmfPkg/OvmfPkgIa32X64.fdf | 4 ++++
> > > >  OvmfPkg/OvmfPkgX64.dsc     | 8 ++++++++
> > > >  OvmfPkg/OvmfPkgX64.fdf     | 4 ++++
> > > >  6 files changed, 36 insertions(+)
> > > >
> > > > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> > > > index e97f7f0262..363f143c68 100644
> > > > --- a/OvmfPkg/OvmfPkgIa32.dsc
> > > > +++ b/OvmfPkg/OvmfPkgIa32.dsc
> > > > @@ -38,6 +38,7 @@ [Defines]
> > > >    DEFINE NETWORK_IP6_ENABLE      = FALSE
> > > >    DEFINE HTTP_BOOT_ENABLE        = FALSE
> > > >    DEFINE SMM_REQUIRE             = FALSE
> > > > +  DEFINE TLS_ENABLE              = FALSE
> > > >
> > > >  [BuildOptions]
> > > >    GCC:*_UNIXGCC_*_CC_FLAGS             = -DMDEPKG_NDEBUG
> > > > @@ -158,6 +159,9 @@ [LibraryClasses]
> > > >
> > > >  !if $(HTTP_BOOT_ENABLE) == TRUE
> > > >    HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
> > > > +!if $(TLS_ENABLE) == TRUE
> > > > +  TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
> > > > +!endif
> > > >  !endif
> > > >
> > > >
> > > >
> > >
> S3BootScriptLib|MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScrip
> > > > tLib.inf
> > > > @@ -715,6 +719,10 @@ [Components]
> > > >    NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
> > > >    NetworkPkg/HttpDxe/HttpDxe.inf
> > > >    NetworkPkg/HttpBootDxe/HttpBootDxe.inf
> > > > +!if $(TLS_ENABLE) == TRUE
> > > > +  NetworkPkg/TlsDxe/TlsDxe.inf
> > > > +  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
> > > > +!endif
> > > >  !endif
> > > >    OvmfPkg/VirtioNetDxe/VirtioNet.inf
> > > >
> > > > diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
> > > > index 34d57a6079..30c8800932 100644
> > > > --- a/OvmfPkg/OvmfPkgIa32.fdf
> > > > +++ b/OvmfPkg/OvmfPkgIa32.fdf
> > > > @@ -329,6 +329,10 @@ [FV.DXEFV]
> > > >    INF  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
> > > >    INF  NetworkPkg/HttpDxe/HttpDxe.inf
> > > >    INF  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
> > > > +!if $(TLS_ENABLE) == TRUE
> > > > +  INF  NetworkPkg/TlsDxe/TlsDxe.inf
> > > > +  INF  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
> > > > +!endif
> > > >  !endif
> > > >    INF  OvmfPkg/VirtioNetDxe/VirtioNet.inf
> > > >
> > > > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc
> b/OvmfPkg/OvmfPkgIa32X64.dsc
> > > > index 8e3e04c135..f22bad309a 100644
> > > > --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> > > > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> > > > @@ -38,6 +38,7 @@ [Defines]
> > > >    DEFINE NETWORK_IP6_ENABLE      = FALSE
> > > >    DEFINE HTTP_BOOT_ENABLE        = FALSE
> > > >    DEFINE SMM_REQUIRE             = FALSE
> > > > +  DEFINE TLS_ENABLE              = FALSE
> > > >
> > > >  [BuildOptions]
> > > >    GCC:*_UNIXGCC_*_CC_FLAGS             = -DMDEPKG_NDEBUG
> > > > @@ -163,6 +164,9 @@ [LibraryClasses]
> > > >
> > > >  !if $(HTTP_BOOT_ENABLE) == TRUE
> > > >    HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
> > > > +!if $(TLS_ENABLE) == TRUE
> > > > +  TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
> > > > +!endif
> > > >  !endif
> > > >
> > > >
> > > >
> > >
> S3BootScriptLib|MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScrip
> > > > tLib.inf
> > > > @@ -724,6 +728,10 @@ [Components.X64]
> > > >    NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
> > > >    NetworkPkg/HttpDxe/HttpDxe.inf
> > > >    NetworkPkg/HttpBootDxe/HttpBootDxe.inf
> > > > +!if $(TLS_ENABLE) == TRUE
> > > > +  NetworkPkg/TlsDxe/TlsDxe.inf
> > > > +  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
> > > > +!endif
> > > >  !endif
> > > >    OvmfPkg/VirtioNetDxe/VirtioNet.inf
> > > >
> > > > diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf
> b/OvmfPkg/OvmfPkgIa32X64.fdf
> > > > index df55c2b210..7bc31d42ba 100644
> > > > --- a/OvmfPkg/OvmfPkgIa32X64.fdf
> > > > +++ b/OvmfPkg/OvmfPkgIa32X64.fdf
> > > > @@ -329,6 +329,10 @@ [FV.DXEFV]
> > > >    INF  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
> > > >    INF  NetworkPkg/HttpDxe/HttpDxe.inf
> > > >    INF  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
> > > > +!if $(TLS_ENABLE) == TRUE
> > > > +  INF  NetworkPkg/TlsDxe/TlsDxe.inf
> > > > +  INF  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
> > > > +!endif
> > > >  !endif
> > > >    INF  OvmfPkg/VirtioNetDxe/VirtioNet.inf
> > > >
> > > > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> > > > index 6ec3fe050d..8eca6fd557 100644
> > > > --- a/OvmfPkg/OvmfPkgX64.dsc
> > > > +++ b/OvmfPkg/OvmfPkgX64.dsc
> > > > @@ -38,6 +38,7 @@ [Defines]
> > > >    DEFINE NETWORK_IP6_ENABLE      = FALSE
> > > >    DEFINE HTTP_BOOT_ENABLE        = FALSE
> > > >    DEFINE SMM_REQUIRE             = FALSE
> > > > +  DEFINE TLS_ENABLE              = FALSE
> > > >
> > > >  [BuildOptions]
> > > >    GCC:*_UNIXGCC_*_CC_FLAGS             = -DMDEPKG_NDEBUG
> > > > @@ -163,6 +164,9 @@ [LibraryClasses]
> > > >
> > > >  !if $(HTTP_BOOT_ENABLE) == TRUE
> > > >    HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
> > > > +!if $(TLS_ENABLE) == TRUE
> > > > +  TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
> > > > +!endif
> > > >  !endif
> > > >
> > > >
> > > >
> > >
> S3BootScriptLib|MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScrip
> > > > tLib.inf
> > > > @@ -722,6 +726,10 @@ [Components]
> > > >    NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
> > > >    NetworkPkg/HttpDxe/HttpDxe.inf
> > > >    NetworkPkg/HttpBootDxe/HttpBootDxe.inf
> > > > +!if $(TLS_ENABLE) == TRUE
> > > > +  NetworkPkg/TlsDxe/TlsDxe.inf
> > > > +  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
> > > > +!endif
> > > >  !endif
> > > >    OvmfPkg/VirtioNetDxe/VirtioNet.inf
> > > >
> > > > diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> > > > index 5e2e1dfaf5..cb7ca131e8 100644
> > > > --- a/OvmfPkg/OvmfPkgX64.fdf
> > > > +++ b/OvmfPkg/OvmfPkgX64.fdf
> > > > @@ -329,6 +329,10 @@ [FV.DXEFV]
> > > >    INF  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
> > > >    INF  NetworkPkg/HttpDxe/HttpDxe.inf
> > > >    INF  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
> > > > +!if $(TLS_ENABLE) == TRUE
> > > > +  INF  NetworkPkg/TlsDxe/TlsDxe.inf
> > > > +  INF  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
> > > > +!endif
> > > >  !endif
> > > >    INF  OvmfPkg/VirtioNetDxe/VirtioNet.inf
> > > >
> > > > --
> > > > 2.11.0
> > > >
> > > > _______________________________________________
> > > > edk2-devel mailing list
> > > > edk2-devel@lists.01.org
> > > > https://lists.01.org/mailman/listinfo/edk2-devel
> >
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


  reply	other threads:[~2017-01-16  9:15 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-16  4:10 [PATCH] OvmfPkg: Enable HTTPS for Ovmf Gary Lin
2017-01-16  5:44 ` Wu, Jiaxin
2017-01-16  6:15   ` Wu, Jiaxin
2017-01-16  6:40     ` Gary Lin
2017-01-16  9:15       ` Wu, Jiaxin [this message]
2017-01-16  6:32   ` Gary Lin
2017-01-16 23:01     ` Laszlo Ersek
2017-01-17  1:22       ` Wu, Jiaxin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=895558F6EA4E3B41AC93A00D163B727416293FDC@SHSMSX103.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox