Pairwise check for DH keys import as part of FIPS #28058
Conversation
github-actions
bot
added
the
severity: fips change
|
Our FIPS lab informed us that pairwise tests on import were not required. The clause that mandated them in the IGs was apparently introduced in error and NIST was planning on rectifying this. See #26785 and others for the removal of these tests. That was a while ago, so it's possible things have changed. If they have changed, could you point us to an authoritative source for it? |
|
@paulidale can you see the issue that is linked here? |
|
@paulidale Our lab has referenced Section 5.6.2.1.4 of SP 800-56Ar3 as the authoritative souce of the need for this test. While it offers the option to assure pairwise consistency either via a PCT or via key generation following the steps in section Section 5.6.1 for both static and ephemeral keys, they indicate that in their experience proving conformance to Section 5.6.1 is sufficiently contentious that doing the PCT makes the validation process go more smoothly. There is apparently some discussion underway that would relax this requirement, but until then, they have advised us that the PCT will make our validation easier in the long run. |
|
I suspect there has been a miscommunication somewhere. A test after key generation is sufficient to satisfy 5.6.2.1.4 via clause a, there is no need for the pairwise test on import based on 56A. The relevant text requiring the test on import is in additional comment 1 in section 10.3.A of the FIPS 140-3 IGs. It reads: The PCT shall be performed either when keys are generated/imported, prior to the first exportation, or prior to the first operational use (if not exported before the first use).. This is what the lab said was a mistake and that NIST was going to revise. Still, if the lab asked for it, best to oblige them. |
| static const unsigned char dh_pub[] = { | ||
| 0x95, 0xdd, 0x33, 0x8d, 0x29, 0xe5, 0x71, 0x04, | ||
| 0x92, 0xb9, 0x18, 0x31, 0x7b, 0x72, 0xa3, 0x69, |
There was a problem hiding this comment.
NOTE for other reviewers: And so forth, with the extant dh_pub replaced by a new correct value. It turns out that the old value did not in fact match the dh_priv above, but this did not matter, because it was never used (the CAST only used the host private key in a key agreement with a peer public key, the host public key could have been NULL).
Only the pull request I linked to. |
|
Pauli is not imagining it. That was definitely the advice we received not very long ago, otherwise it would have already been done for 3.5. |
|
@nhorman I have sent you the email response Pauli got from the previous question. If the Lab changed its mind then that is ok. |
The reality is that it was done for 3.5 but backed out after the lab's advice. The PRs in question could be reverted/resurrected. I'll also note that if this is mandated for DH, then it would also apply to all other asymmetric algorithms. The IG text isn't selective about only being applicable to DH. Again, I had done them all. |
t8m
added
approval: done
|
24 hours has passed since 'approval: done' was set, but this PR has failing CI tests. Once the tests pass it will get moved to 'approval: ready to merge' automatically,alternatively please review and set the label manually. |
Signed-off-by: Nikola Pajkovsky <nikolap@openssl.org>
For DH key import, it appears the PCT and assurances are implemented in the source but are not be performed by default. Signed-off-by: Nikola Pajkovsky <nikolap@openssl.org>
openssl-machine
added
approval: ready to merge
|
This pull request is ready to merge |
|
Merged to the master and 3.5 branches. Thank you. |
Signed-off-by: Nikola Pajkovsky <nikolap@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Viktor Dukhovni <viktor@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #28058)
For DH key import, it appears the PCT and assurances are implemented in the source but are not be performed by default. Signed-off-by: Nikola Pajkovsky <nikolap@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Viktor Dukhovni <viktor@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #28058)
For DH key import, it appears the PCT and assurances are implemented in the source but are not be performed by default. Signed-off-by: Nikola Pajkovsky <nikolap@openssl.org> Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Viktor Dukhovni <viktor@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #28058) (cherry picked from commit e08b83c)
|
Why was this merged? Did you see Pauli's comments? |
The PR gained sufficient approvals, and the latest advice from the current lab is that the PCT on import is required. If their advice changes, we may have an opportunity to change the code accordingly. |
|
Perhaps it was about my comment about having done all the work for the on import PCTs for all asymmetric types in other PRs. See e.g. #26581 and others. This one looks like it executes the PCT during the power on tests which is both completely pointless (since it's a KAT) and a waste of time at a point where things are quite busy. In other words, it's at best half of what's required. |
For DH key import, it appears the PCT and assurances are implemented in
the source but are not be performed by default.