1608250 - KBKDF - broken fipstest handling of KI_len

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Alexander Scheel]

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:71.0) Gecko/20100101 Firefox/71.0

Steps to reproduce:

When testing Bug 1608245, I realized that I had inadvertently broken fipstest.c's handling of KI and KI_len. This lead to it passing bogus keys (with unusually large lengths exceeding the bounds of sizeof KI) to kbkdf_Dispatch(...). Luckily this was an easy fix (and the underlying KBKDF implementation is fine; only the test code was broken).

I should've taken Bob's suggestion in the first place: when parsing the mech, store KI_len there, which makes the KI reading easy.

This does a little cleanup around the clearing the other fields while I'm here (to make everything consistent).

Actual results:

KBKDF CAVP tests fail to execute.

Expected results:

KBKDF CAVP tests should execute.

Comment 1

[Alexander Scheel]

Attachment: Bug 1608250 - KBKDF - Fix broken fipstest handling of KI_len r=rrelyea

When testing Bug 1608245, I realized that I had inadvertently broken
fipstest.c's handling of KI and KI_len. This lead to it passing bogus
keys (with unusually large lengths exceeding the bounds of sizeof KI)
to kbkdf_Dispatch(...).

This uses Bob Relyea's suggestion on how to handle this: detect the
size of KI when processing the mech selection, storing KI_len there.
This simplifies reading of the KI value in later code.

Comment 2

[Alexander Scheel]

Bob -- is there some place I should've add KBKDF tests so this could've be detected by NSS's CI? Maybe I just didn't look in the right places in Moz-Phab when submitting this to see failing tests.

I assume this would've gotten caught sooner or later though.

Comment 3

[Robert Relyea]

Sigh, this is a mozilla CI issue. We need a CI that runs the fips tests. We do this for RHEL/Fedora (one of our issues when we rebase is catching all the FIPS issues in that release- NSS builds don't even complete if it can't pass), but there should be at least one mozilla test that includes FIPS. I think that issue was on jc's plate, but there are lots of issues on jc's plate, so it's probably not surprising it hasn't been fixed yet.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:jcj, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 5

[J.C. Jones [:jcj] (he/him)]

We've put the FIPS tests in taskcluster back near the upper part of our todo list.

Comment 6

[Robert Relyea]

Patch checked in: https://hg.mozilla.org/projects/nss/rev/d7b12847a6503ebcb3f7df56badc79149207d476