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.1][test] Avoid missing EC_GROUP wrappers #12433

Closed

Conversation

@romen
Copy link
Member

@romen romen commented Jul 13, 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

Checklist
  • tests are added or updated
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
@romen romen self-assigned this Jul 13, 2020
@romen romen linked an issue that may be closed by this pull request Jul 13, 2020
@beldmit
Copy link
Member

@beldmit beldmit commented Jul 13, 2020

My fault :(

Copy link
Member

@beldmit beldmit left a comment

Approve if Travis is successful

@romen
Copy link
Member Author

@romen romen commented Jul 13, 2020

My fault :(

Actually not, it was my fault, I endorsed backporting to 1.1.1 as cherry-picking cleanly, but clearly my dev worktree must have been in some weird state to not trigger build failures!

bsize = (EC_GROUP_get_field_type(group) == NID_X9_62_prime_field) ?
BN_num_bytes(EC_GROUP_get0_field(group)) :
(EC_GROUP_get_degree(group) + 7) / 8;
bsize = (EC_GROUP_get_degree(group) + 7) / 8;
Comment on lines -2126 to +2126

This comment has been minimized.

@romen

romen Jul 13, 2020
Author Member

@bbbrumley Am I missing why (other than for clarity) you did not use EC_GROUP_get_degree() in both cases?

This comment has been minimized.

@bbbrumley

bbbrumley Jul 14, 2020
Contributor

Not really. It started out with no EC_GROUP_get_degree or branches at all but then failed for some old binary curves. So then I went digging for how to make that work and didn't back up to reconsider. (I would not have expected EC_GROUP_get_degree to do anything sane for GFp curves, but looks like I'm wrong.)

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)
@romen
Copy link
Member Author

@romen romen commented Jul 14, 2020

Merged to 1.1.1 as:

  • a5b8c19 [1.1.1][test] Avoid missing EC_GROUP wrappers

Thanks!

@romen romen closed this Jul 14, 2020
@romen romen mentioned this pull request Jul 28, 2020
1 of 1 task complete
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.

5 participants