[Skiboot] [PATCH v2 2/5] secvar/backend: clarify variables in process_update

[Nick Child]

Reviewed-by: Nick Child <nick.child at ibm.com>

Also, I used secvarctl's test suite to test the new edk2-compat-process file.
Not sure if that counts as Tested-by material since I did not flash
a skiboot image onto any POWER NV machines but if so:

Tested-by: Nick Child <nick.child at ibm.com>

-Nick

On Mon, Jun 21, 2021 at 9:21 PM Nayna <nayna at linux.vnet.ibm.com> wrote:
>
>
> On 6/21/21 4:26 AM, Daniel Axtens wrote:
> > process_update() has tbhbuffer and tbhbuffersize. However, tbhbuffer
> > doesn't contain to-be-hashed data, but a hash:
> >
> > /* Prepare the data to be verified */
> > tbhbuffer = get_hash_to_verify(update->key, *newesl, *new_data_size,
> >                       timestamp);
> >
> > And tbhbuffersize is initalised to zero and then never filled with
> > the actual length, so 0 is passed through to verify_signature.
> > verify_signature will in turn pass that to mbedtls, which will interpret
> > it as "figure out the length yourself based on the hash type". So this
> > has always worked, but by accident.
> >
> > Rename tbhbuffer to hash, as that's what it is.
> >
> > Drop tbhbuffersize, and pass 32 directly to verify_signature. We only
> > support SHA-256, and SHA-256 is 32 bytes long.
> >
> > Signed-off-by: Daniel Axtens <dja at axtens.net>
> > ---
> >   libstb/secvar/backend/edk2-compat-process.c | 14 ++++++--------
> >   1 file changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/libstb/secvar/backend/edk2-compat-process.c b/libstb/secvar/backend/edk2-compat-process.c
> > index 8324dc068b8e..b39f35e0c3dc 100644
> > --- a/libstb/secvar/backend/edk2-compat-process.c
> > +++ b/libstb/secvar/backend/edk2-compat-process.c
> > @@ -662,8 +662,7 @@ int process_update(const struct secvar *update, char **newesl,
> >       void *auth_buffer = NULL;
> >       int auth_buffer_size = 0;
> >       const char *key_authority[3];
> > -     char *tbhbuffer = NULL;
> > -     size_t tbhbuffersize = 0;
> > +     char *hash = NULL;
> >       struct secvar *avar = NULL;
> >       int rc = 0;
> >       int i;
> > @@ -723,9 +722,9 @@ int process_update(const struct secvar *update, char **newesl,
> >       }
> >
> >       /* Prepare the data to be verified */
> > -     tbhbuffer = get_hash_to_verify(update->key, *newesl, *new_data_size,
> > +     hash = get_hash_to_verify(update->key, *newesl, *new_data_size,
> >                               timestamp);
>
> Thanks Daniel.
>
> I think the comment also needs to be updated accordingly.
>
> Thanks & Regards,
>
>       - Nayna
>
>
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot