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

[test] ectest: check custom generators #12096

Closed
wants to merge 1 commit into from

Conversation

@bbbrumley
Copy link
Contributor

@bbbrumley bbbrumley commented Jun 9, 2020

I was doing some custom EC_METHOD work that should've obviously failed because it was oblivious to EC_GROUP_set_generator.

But it passed all tests.

So here's a PR for that.

A handful of the NIST curves have similar tests in ectest.c that I'll deprecate as part of a larger cleanup of this test harness in the future.

@romen
Copy link
Member

@romen romen commented Jun 30, 2020

I missed this before!

This is supposed to be also in 1.1.1 right? Can you check if it cherry-picks cleanly?

@romen
romen approved these changes Jun 30, 2020
Copy link
Member

@romen romen left a comment

LGTM, I can confirm this cherry picks cleanly to 1.1.1 so far.

@bbbrumley
Copy link
Contributor Author

@bbbrumley bbbrumley commented Jul 11, 2020

This is holding up #12201, which still needs effort after this gets merged. Can I get a second review please?

Copy link
Member

@beldmit beldmit left a comment

LGTM

@openssl-machine
Copy link

@openssl-machine openssl-machine commented Jul 12, 2020

This pull request is ready to merge

openssl-machine pushed a commit that referenced this pull request Jul 12, 2020
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from #12096)
openssl-machine pushed a commit that referenced this pull request Jul 12, 2020
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from #12096)

(cherry picked from commit a01cae9)
@beldmit
Copy link
Member

@beldmit beldmit commented Jul 12, 2020

Merged to both branches. Thanks!

@beldmit beldmit closed this Jul 12, 2020
test/ectest.c Show resolved Hide resolved
@romen romen mentioned this pull request Jul 13, 2020
romen added a commit to romen/openssl that referenced this pull request Jul 13, 2020
Backport of openssl#12096 to 1.1.1 broke
the build as the following functions are missing:

    const BIGNUM *EC_GROUP_get0_field(const EC_GROUP *group);
    int EC_GROUP_get_field_type(const EC_GROUP *group);

Turns out that for the purposes of the test code, we don't really need
to differentiate between prime and binary fields, and we can directly
use the existing `EC_GROUP_get_degree()` in the same fashion as was
being done for binary fields also for prime fields.

Fixes openssl#12432
@romen romen mentioned this pull request Jul 13, 2020
1 of 1 task complete
openssl-machine pushed a commit that referenced this pull request Jul 14, 2020
Backport of #12096 to 1.1.1 broke
the build as the following functions are missing:

    const BIGNUM *EC_GROUP_get0_field(const EC_GROUP *group);
    int EC_GROUP_get_field_type(const EC_GROUP *group);

Turns out that for the purposes of the test code, we don't really need
to differentiate between prime and binary fields, and we can directly
use the existing `EC_GROUP_get_degree()` in the same fashion as was
being done for binary fields also for prime fields.

Fixes #12432

Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from #12433)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants