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.

Sign up for GitHub

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

Jump to bottom

ffdhe: add option to set default socket timeout #782

Merged
merged 1 commit into from
Feb 9, 2022
Merged

ffdhe: add option to set default socket timeout #782

merged 1 commit into from
Feb 9, 2022

Conversation

ueno
Copy link
Collaborator

@ueno ueno commented Dec 20, 2021 •
edited by tomato42

The tests using ffdhe8192 take long time on some CI setup (e.g., with
software emulated GMP) and cause intermittent timeout. This adds -t
option to control the timeout value from the test invocation.

Signed-off-by: Daiki Ueno dueno@redhat.com

Description

Motivation and Context

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

Confirmed at:
https://gitlab.com/dueno/gnutls/-/jobs/1903136178

This change is Reviewable

tomato42
tomato42 requested changes Dec 20, 2021
View reviewed changes
scripts/test-ffdhe-expected-params.py Outdated Show resolved Hide resolved
scripts/test-ffdhe-expected-params.py Outdated Show resolved Hide resolved
scripts/test-ffdhe-negotiation.py Outdated Show resolved Hide resolved
scripts/test-ffdhe-negotiation.py Outdated Show resolved Hide resolved
@ueno
Copy link
Collaborator Author

ueno commented Dec 20, 2021

@tomato42 I've addressed the issues; could you take a second look?

tomato42
tomato42 previously approved these changes Dec 20, 2021
View reviewed changes
@tomato42
Copy link
Member

Looks good but will need to fix py3.3 before merge

@tomato42
Copy link
Member

tomato42 commented Jan 5, 2022 •
edited

@ueno I've updated the CI config, it should pass now, could you rebase?

EDIT: ah. sorry, that's tlsfuzzer while I fixed tlslite-ng, don't rebase just yet

@tomato42
Copy link
Member

tomato42 commented Jan 5, 2022

@ueno Now it's fixed: https://github.com/tlsfuzzer/tlsfuzzer/actions/runs/1660011256

@tomato42 tomato42 added the bug unintented behaviour in tlsfuzzer code label Jan 6, 2022
@ueno
Copy link
Collaborator Author

ueno commented Feb 4, 2022

@tomato42 what else should I do to get this merged?

@tomato42
Copy link
Member

tomato42 commented Feb 8, 2022 •
edited

what else should I do to get this merged?

You need to rebase this branch on top of master, as master has py3.3 CI fixed (430ddb0)

@tomato42
Copy link
Member

tomato42 commented Feb 8, 2022

sigh 3.9 has a new release (3.9.10) so we'll need another rebase after #787 is merged...
That 3.3 failure looks like a python crash, not sure what causes it

@tomato42
Copy link
Member

tomato42 commented Feb 9, 2022

It looks like the py3.3 crash was caused by gmpy or gmpy2, I've disabled them for now on that python version, please rebase

@ueno
ffdhe: add option to set default socket timeout
1b3c9d5 
The tests using ffdhe8192 take long time on some CI setup (e.g., with
software emulated GMP) and cause intermittent timeout.  This adds -t
option to control the timeout value from the test invocation.

Signed-off-by: Daiki Ueno <dueno@redhat.com>
@tomato42 tomato42 merged commit 4216d2c into tlsfuzzer:master Feb 9, 2022
@tomato42
Copy link
Member

tomato42 commented Feb 9, 2022

thanks! and sorry for the CI failures

@tomato42 tomato42 mentioned this pull request Feb 16, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomato42 tomato42 tomato42 approved these changes

Assignees
No one assigned
Labels
bug unintented behaviour in tlsfuzzer code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@ueno @tomato42