Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIPS compatibility for tests #740

Merged
merged 5 commits into from
Mar 5, 2021
Merged

FIPS compatibility for tests #740

merged 5 commits into from
Mar 5, 2021

Conversation

tomato42
Copy link
Member

@tomato42 tomato42 commented Feb 24, 2021

Description

Update test-certificate-verify-malformed-sig.py so that it doesn't require RSA key exchange
Also fix the bug because of which it wasn't running the sanity cases

Motivation and Context

work towards #563

Checklist

  • I have read the CONTRIBUTING.md document and my PR follows change requirements therein
  • the changes are also reflected in documentation and code comments
  • all new and existing tests pass (see CI results)
  • test script checklist was followed for new scripts
  • new test script added to tlslite-ng.json and tlslite-ng-random-subset.json
  • new and modified scripts were ran against popular TLS implementations:
    • OpenSSL
    • NSS
    • GnuTLS
  • required version of tlslite-ng updated in requirements.txt and README.md

This change is Reviewable

@tomato42 tomato42 added the enhancement new feature to be implemented label Feb 24, 2021
@tomato42 tomato42 self-assigned this Feb 24, 2021
@tomato42 tomato42 added the bug unintented behaviour in tlsfuzzer code label Feb 24, 2021
@tomato42 tomato42 force-pushed the fips-compat branch 2 times, most recently from 20e3686 to f31b671 Compare March 1, 2021 16:25
@tomato42 tomato42 requested a review from ueno March 1, 2021 17:23
Copy link
Collaborator

@ueno ueno left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1, 3 of 3 files at r2, 1 of 1 files at r3, 3 of 3 files at r4, 1 of 1 files at r5, 1 of 1 files at r6.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @tomato42)


scripts/test-certificate-verify-malformed-sig.py, line 168 at r2 (raw file):

        conversations["sanity - {0}".format(hash_alg)] = conversation

    # place SHA-1 sig with SHA-256 indicator

Does this comment still hold, after adding the two additional cases (SHA-256 in SHA-1, SHA-384 in SHA-256)?

also allow the server to quickly close the connection right after
the bad CV message
since server can abort right after Certificate or CertificateVerify
it doesn't like then and close connection then, we may not be able
to write to the socket, reporting a pipe error when in reality
server did send an Alert message
Copy link
Member Author

@tomato42 tomato42 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @ueno)


scripts/test-certificate-verify-malformed-sig.py, line 168 at r2 (raw file):

Previously, ueno (Daiki Ueno) wrote…

Does this comment still hold, after adding the two additional cases (SHA-256 in SHA-1, SHA-384 in SHA-256)?

true, fixed

Copy link
Collaborator

@ueno ueno left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r7, 3 of 3 files at r8, 3 of 3 files at r9, 1 of 1 files at r10, 1 of 1 files at r11.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@tomato42 tomato42 merged commit a3b886c into master Mar 5, 2021
@tomato42 tomato42 deleted the fips-compat branch March 5, 2021 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug unintented behaviour in tlsfuzzer code enhancement new feature to be implemented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants