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

[1.1.0 backport] Remove DSA digest length checks when no digest is passed #6750

Closed
wants to merge 2 commits into from

Conversation

bdonlan
Copy link
Contributor

@bdonlan bdonlan commented Jul 19, 2018

This change fixes #6748 on the 1.1.0 stable branch, and introduces a test to prevent future regressions. See the bug for more details.

As this bug was a regression in 1.1.0, a 1.0.x backport is not required.

Checklist
  • tests are added or updated

Copy link
Contributor

@richsalz richsalz left a comment

Choose a reason for hiding this comment

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

small formatting nits. if you want detailed help ping me.

static int genkeys(void)
{
dsa = DSA_new();
if (!dsa) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use NULL not ! for pointer tests; there are multiple places to change.

dataToSign = OPENSSL_malloc(len);
paddedData = OPENSSL_malloc(digestlen);

if (!dataToSign || !paddedData) goto end;
Copy link
Contributor

Choose a reason for hiding this comment

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

put the goto on a separate line. there's a couple of other places where statements are joined, that we want on separate lines.

FIPS 186-4 does not specify a hard requirement on DSA digest lengths, and in
any case the current check rejects the FIPS recommended digest lengths for key
sizes != 1024 bits.

Backport for: openssl#6748
@bdonlan bdonlan force-pushed the dsa_len_bp branch 3 times, most recently from 1f131ed to e14321c Compare July 23, 2018 22:07

ok = 1;
end:
if (!ok) ERR_print_errors_fp(stderr);
Copy link
Contributor

Choose a reason for hiding this comment

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

please put this on two lines.

fprintf(stderr, "Key generation failed\n");
return 1;
}
if (!dsa_exact_size_test()) return 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

please put these all on two lines each.

@richsalz
Copy link
Contributor

As andy says, #6749 had the first commit cherry-picked, so let's close this. Thanks for doing the test, anyway!

@richsalz richsalz closed this Jul 29, 2018
@bdonlan bdonlan deleted the dsa_len_bp branch July 30, 2018 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants