-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Backport 2.28: ssl-opt.sh, compat.sh: Error out if not executing any tests #9204
Backport 2.28: ssl-opt.sh, compat.sh: Error out if not executing any tests #9204
Conversation
Alert if all tests are filtered out or skipped: that probably indicates a test script that set up an unintended configuration or an overly strict filter. You can pass `--min 0` to bypass this check. You can pass `--min` with a larger value to require that many test cases to run. Signed-off-by: Gilles Peskine <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Faithfull backport - LGTM
It seems the change revealed an issue in |
@ronald-cron-arm Yes, there's a |
With GnuTLS servers, 3DES-CBC cipher suites are enabled by default under our GNUTLS_LEGACY (3.3.8), but disabled by default under more recent versions including the one we use by default on the CI (3.4.6). Even modern versions (I checked 3.7.2) support 3DES if explicitly enabled. So unconditionally enable 3DES-CBC for GnuTLS. Signed-off-by: Gilles Peskine <[email protected]>
We were only requesting 3DES cipher suites (which is weirdly restrictive since the configuration also includes AES), but DES is in the default exclusion list for compat.sh, so we ended up having no acceptable cipher suites. Fix this. Signed-off-by: Gilles Peskine <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -27,7 +27,8 @@ | |||
'test_again_with_use_psa' => 1 | |||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked what compat.sh
test cases we're running for each configuration here, and I think there are more omissions than the one I fixed, but for the other configurations we're running at least one test case so the CI is passing now:
- The CCM-PSK configurations aren't testing OpenSSL interoperability, but that's a broader problem.
- For suite-b, we aren't doing GnuTLS or OpenSSL interoperability. It's deliberate but I don't know why. Both GnuTLS and OpenSSL interop fail, which I've filed as a separate issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one thing I am not sure about.
Filtering on cipher suites that have RSA in their name excludes a few old RSA-based cipher suites whose name doesn't contain RSA. Signed-off-by: Gilles Peskine <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks.
@paul-elliott-arm please have a look to the last version, thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - apologies, I could have sworn I reviewed this (in this state) before.
Backport of #9149, and fix some 2.28-only issues that it reveals.
PR checklist