Re: [PATCH 3/4] NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate attributes retrieval

[Laszlo Ersek <lersek@redhat.com>]

On 08/20/18 08:29, Li, Songpeng wrote:
> It worked on my end.
> 
> Tested-by: Songpeng Li <songpeng.li@intel.com>

Thank you!

Jiaxin, Siyuan, are you guys OK with this patch?

Thanks
Laszlo

>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Friday, August 17, 2018 10:36 PM
>> To: edk2-devel-01 <edk2-devel@lists.01.org>
>> Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Li,
>> Songpeng <songpeng.li@intel.com>
>> Subject: [PATCH 3/4] NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate
>> attributes retrieval
>>
>> Per spec, the GetVariable() runtime service is not required to populate
>> (*Attributes) on output when it fails with EFI_BUFFER_TOO_SMALL.
>>
>> Therefore we have to fetch the full contents of the TlsCaCertificate
>> variable temporarily, just so we can (a) get the current attributes, and
>> (b) add EFI_VARIABLE_APPEND_WRITE to them for the subsequent
>> SetVariable()
>> call.
>>
>> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
>> Cc: Siyuan Fu <siyuan.fu@intel.com>
>> Cc: Songpeng Li <songpeng.li@intel.com>
>> Reported-by: Songpeng Li <songpeng.li@intel.com>
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1090
>> Fixes: b90c335fbbb674470fbf09601cc522bf61564c30
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>
>> Notes:
>>     Tested via loading the same CA cert .pem file twice in a row, using the
>>     HII form, first without any pre-existent TlsCaCertificate variable.
>>
>>     Songpeng, can you please test this patch as well, and confirm if it
>>     works on your end? Thanks!
>>
>>  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c | 27
>> +++++++++++++++++++-
>>  1 file changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
>> b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
>> index 7259c5e82f61..0780b03bbab4 100644
>> --- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
>> +++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
>> @@ -663,6 +663,7 @@ EnrollX509toVariable (
>>    EFI_SIGNATURE_LIST                *CACert;
>>    EFI_SIGNATURE_DATA                *CACertData;
>>    VOID                              *Data;
>> +  VOID                              *CurrentData;
>>    UINTN                             DataSize;
>>    UINTN                             SigDataSize;
>>    UINT32                            Attr;
>> @@ -674,6 +675,7 @@ EnrollX509toVariable (
>>    CACert        = NULL;
>>    CACertData    = NULL;
>>    Data          = NULL;
>> +  CurrentData   = NULL;
>>    Attr          = 0;
>>
>>    Status = ReadFileContent (
>> @@ -716,11 +718,30 @@ EnrollX509toVariable (
>>    Status = gRT->GetVariable(
>>                    VariableName,
>>                    &gEfiTlsCaCertificateGuid,
>> -                  &Attr,
>> +                  NULL,
>>                    &DataSize,
>>                    NULL
>>                    );
>>    if (Status == EFI_BUFFER_TOO_SMALL) {
>> +    //
>> +    // Per spec, we have to fetch the variable's contents, even though we're
>> +    // only interested in the variable's attributes.
>> +    //
>> +    CurrentData = AllocatePool (DataSize);
>> +    if (CurrentData == NULL) {
>> +      Status = EFI_OUT_OF_RESOURCES;
>> +      goto ON_EXIT;
>> +    }
>> +    Status = gRT->GetVariable(
>> +                    VariableName,
>> +                    &gEfiTlsCaCertificateGuid,
>> +                    &Attr,
>> +                    &DataSize,
>> +                    CurrentData
>> +                    );
>> +    if (EFI_ERROR (Status)) {
>> +      goto ON_EXIT;
>> +    }
>>      Attr |= EFI_VARIABLE_APPEND_WRITE;
>>    } else if (Status == EFI_NOT_FOUND) {
>>      Attr = TLS_AUTH_CONFIG_VAR_BASE_ATTR;
>> @@ -751,6 +772,10 @@ ON_EXIT:
>>      FreePool (Data);
>>    }
>>
>> +  if (CurrentData != NULL) {
>> +    FreePool (CurrentData);
>> +  }
>> +
>>    if (X509Data != NULL) {
>>      FreePool (X509Data);
>>    }
>> --
>> 2.14.1.3.gb7cf6e02401b
>>
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>