Skip to content

Comments

Revert "fips: remove redundant RSA encrypt/decrypt KAT"#28676

Closed
nhorman wants to merge 2 commits intoopenssl:masterfrom
nhorman:832
Closed

Revert "fips: remove redundant RSA encrypt/decrypt KAT"#28676
nhorman wants to merge 2 commits intoopenssl:masterfrom
nhorman:832

Conversation

@nhorman
Copy link
Contributor

@nhorman nhorman commented Sep 26, 2025

This reverts commit 635bf49.

During code review for FIPS-140-3 certification, our lab noticed that the known answer test for RSA was removed. This was done in the above commit, as part of
#25988

Under the assertion that FIPS 140-3 Implementation Guidance section D.G had relaxed the requirements for testing, obviating the need for this test.

However, for the 3.5 FIPS-140-3 certification we are adding assertions for support of KAS-IFC-SSC, which follows FIPS-140-3 I.G section D.F, which does not contain the same relaxed constraints. As such we need to reintroduce the test.

While the specifics of the I.G requirements are slightly different in D.F (allowing for other, potentially less time-consuming tests), the most expedient path forward here is to simply re-introduce the test as it existed previously, hence the reversion of the above commit.

Fixes openssl/private#832

Checklist
  • tests are added or updated

This reverts commit 635bf49.

During code review for FIPS-140-3 certification, our lab noticed that
the known answer test for RSA was removed.  This was done in the above
commit, as part of
openssl#25988

Under the assertion that FIPS 140-3 Imlementation Guidance section D.G
had relaxed the requirements for testing, obviating the need for this
test.

However, for the 3.5 FIPS-140-3 certification we are adding assertions
for support of KAS-IFC-SSC, which follows FIPS-140-e I.G section D.F,
which does not contain the same relaxed constraints.  As such we need to
reintroduce the test.

While the specifics of the I.G requirements are slightly different in
D.F (allowing for other, potentially less time-consuming tests), the
most expedient path forward here is to simply re-introduce the test as
it existed previously, hence the reversion of the above commit.

Fixes openssl/private#832
@nhorman nhorman added this to the 3.6.0 Release blocker milestone Sep 26, 2025
@nhorman nhorman self-assigned this Sep 26, 2025
@nhorman nhorman added branch: master Applies to master branch approval: review pending This pull request needs review by a committer branch: 3.5 Applies to openssl-3.5 branch: 3.6 Applies to openssl-3.6 labels Sep 26, 2025
Revert "fips: remove redundant RSA encrypt/decrypt KAT"

This reverts commit 635bf49.

During code review for FIPS-140-3 certification, our lab noticed that
the known answer test for RSA was removed.  This was done in the above
commit, as part of
openssl#25988

Under the assertion that FIPS 140-3 Implementation Guidance section D.G
had relaxed the requirements for testing, obviating the need for this
test.

However, for the 3.5 FIPS-140-3 certification we are adding assertions
for support of KAS-IFC-SSC, which follows FIPS-140-3 I.G section D.F,
which does not contain the same relaxed constraints.  As such we need to
reintroduce the test.

While the specifics of the I.G requirements are slightly different in
D.F (allowing for other, potentially less time-consuming tests), the
most expedient path forward here is to simply re-introduce the test as
it existed previously, hence the reversion of the above commit.

Fixes openssl/private#832
@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Sep 26, 2025
@t8m t8m added triaged: bug The issue/pr is/fixes a bug tests: exempted The PR is exempt from requirements for testing labels Sep 26, 2025
Copy link
Member

@t8m t8m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK for 3.5 and 3.6 (the backports CI failure is not relevant as the job does not cope well with empty commits).

@mattcaswell mattcaswell added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Sep 26, 2025
@t-j-h
Copy link
Member

t-j-h commented Sep 26, 2025

Okay for backfit to noted branches

@FdaSilvaYY
Copy link
Contributor

Typo in PR text and commit message : Imlementation

@nhorman
Copy link
Contributor Author

nhorman commented Sep 26, 2025

@FdaSilvaYY see the fixup reword commit, in which that is fixed

@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

@nhorman nhorman added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Sep 27, 2025
openssl-machine pushed a commit that referenced this pull request Sep 27, 2025
This reverts commit 635bf49.

During code review for FIPS-140-3 certification, our lab noticed that
the known answer test for RSA was removed.  This was done in the above
commit, as part of
#25988

Under the assertion that FIPS 140-3 Implementation Guidance section D.G
had relaxed the requirements for testing, obviating the need for this
test.

However, for the 3.5 FIPS-140-3 certification we are adding assertions
for support of KAS-IFC-SSC, which follows FIPS-140-3 I.G section D.F,
which does not contain the same relaxed constraints.  As such we need to
reintroduce the test.

While the specifics of the I.G requirements are slightly different in
D.F (allowing for other, potentially less time-consuming tests), the
most expedient path forward here is to simply re-introduce the test as
it existed previously, hence the reversion of the above commit.

Fixes openssl/private#832

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
(Merged from #28676)
@nhorman
Copy link
Contributor Author

nhorman commented Sep 27, 2025

merged to master/3.6/3.5, thank you!

@nhorman nhorman closed this Sep 27, 2025
openssl-machine pushed a commit that referenced this pull request Sep 27, 2025
This reverts commit 635bf49.

During code review for FIPS-140-3 certification, our lab noticed that
the known answer test for RSA was removed.  This was done in the above
commit, as part of
#25988

Under the assertion that FIPS 140-3 Implementation Guidance section D.G
had relaxed the requirements for testing, obviating the need for this
test.

However, for the 3.5 FIPS-140-3 certification we are adding assertions
for support of KAS-IFC-SSC, which follows FIPS-140-3 I.G section D.F,
which does not contain the same relaxed constraints.  As such we need to
reintroduce the test.

While the specifics of the I.G requirements are slightly different in
D.F (allowing for other, potentially less time-consuming tests), the
most expedient path forward here is to simply re-introduce the test as
it existed previously, hence the reversion of the above commit.

Fixes openssl/private#832

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
(Merged from #28676)

(cherry picked from commit 3206bb7)
openssl-machine pushed a commit that referenced this pull request Sep 27, 2025
This reverts commit 635bf49.

During code review for FIPS-140-3 certification, our lab noticed that
the known answer test for RSA was removed.  This was done in the above
commit, as part of
#25988

Under the assertion that FIPS 140-3 Implementation Guidance section D.G
had relaxed the requirements for testing, obviating the need for this
test.

However, for the 3.5 FIPS-140-3 certification we are adding assertions
for support of KAS-IFC-SSC, which follows FIPS-140-3 I.G section D.F,
which does not contain the same relaxed constraints.  As such we need to
reintroduce the test.

While the specifics of the I.G requirements are slightly different in
D.F (allowing for other, potentially less time-consuming tests), the
most expedient path forward here is to simply re-introduce the test as
it existed previously, hence the reversion of the above commit.

Fixes openssl/private#832

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
(Merged from #28676)

(cherry picked from commit 3206bb7)
esyr added a commit to esyr/openssl that referenced this pull request Sep 30, 2025
3.5.4 CHANGES.md includes the following:
 * openssl#28098
 * openssl#28415
 * openssl#28504
 * openssl#28535
 * openssl#28569
 * openssl#28573
 * openssl#28576
 * openssl#28591
 * openssl#28603
 * openssl#28624
 * openssl#28642
 * openssl#28676

3.5.4 NEWS.md includes the following:
 * openssl#28603

Updated the changes and news in the previous branches.

Removed the attribution in NEWS.md incorrectly introduced in e551da6
"Update news and changes for the 3.5.3 release".

Release: Yes
Signed-off-by: Eugene Syromiatnikov <[email protected]>
esyr added a commit to esyr/openssl that referenced this pull request Sep 30, 2025
3.5.4 CHANGES.md includes the following:
 * openssl#28098
 * openssl#28415
 * openssl#28504
 * openssl#28535
 * openssl#28569
 * openssl#28573
 * openssl#28576
 * openssl#28591
 * openssl#28603
 * openssl#28624
 * openssl#28642
 * openssl#28676

3.5.4 NEWS.md includes the following:
 * openssl#28603

Updated the changes and news in the previous branches.

Removed the attribution in NEWS.md incorrectly introduced in e551da6
"Update news and changes for the 3.5.3 release".

Release: Yes
Signed-off-by: Eugene Syromiatnikov <[email protected]>
esyr added a commit to esyr/openssl that referenced this pull request Sep 30, 2025
3.5.4 CHANGES.md includes the following:
 * openssl#28098
 * openssl#28415
 * openssl#28504
 * openssl#28535
 * openssl#28569
 * openssl#28573
 * openssl#28576
 * openssl#28591
 * openssl#28603
 * openssl#28624
 * openssl#28642
 * openssl#28676

3.5.4 NEWS.md includes the following:
 * openssl#28603

Updated the changes and news in the previous branches.

Removed the attribution in NEWS.md incorrectly introduced in e551da6
"Update news and changes for the 3.5.3 release".

Release: Yes
Signed-off-by: Eugene Syromiatnikov <[email protected]>
esyr added a commit to esyr/openssl that referenced this pull request Sep 30, 2025
3.5.4 CHANGES.md includes the following:
 * openssl#28098
 * openssl#28415
 * openssl#28504
 * openssl#28535
 * openssl#28569
 * openssl#28573
 * openssl#28576
 * openssl#28591
 * openssl#28603
 * openssl#28624
 * openssl#28642
 * openssl#28676

3.5.4 NEWS.md includes the following:
 * openssl#28603

Updated the changes and news in the previous branches.

Removed the attribution in NEWS.md incorrectly introduced in e551da6
"Update news and changes for the 3.5.3 release".

Release: Yes
Signed-off-by: Eugene Syromiatnikov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Applies to master branch branch: 3.5 Applies to openssl-3.5 branch: 3.6 Applies to openssl-3.6 severity: fips change The pull request changes FIPS provider sources tests: exempted The PR is exempt from requirements for testing triaged: bug The issue/pr is/fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants