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

Handling subgroup checks #179

Closed
tplooker opened this issue Jun 20, 2022 · 4 comments
Closed

Handling subgroup checks #179

tplooker opened this issue Jun 20, 2022 · 4 comments
Labels
core Associated to the core spec pending-close ready-for-pr

Comments

@tplooker
Copy link
Member

Currently in the draft there are several key places we need to do subgroup checks on points, for example to validate a public key post de-serialization. However in some places we are implicitly relying on the octet_to_point function to perform this role and other places we are doing it as an explicit step in the procedure. We need to make this consistent, the options are

  1. Consistently rely on the octet_to_point_g* implementation and add a security consideration that documents this function MUST perform the relevant subgroup check.
  2. Explicitly document anywhere in the procedure where a subgroup check is required.

IMO 2 feels like a safer but more verbose option to me.

@tplooker tplooker added ready-for-pr core Associated to the core spec labels Jun 20, 2022
@BasileiosKal
Copy link
Contributor

Maybe a middle-of-the-road solution would be to define explicitly octets_to_point as an operation in the spec, that calls octets_to_point_g1 and does the checks. For example, something like:

result = octets_to_point_g1(point_octets)

Inputs:
- point_octets, octet string

Parameters:
- SubgroupCheck_g1, a function that on input a point P, returns VALID if P 
                    is in the G1 subgroup and INVALID in any other case.

Procedure:

1. P = octets_to_point_g1(point_octets)

2. if P is INVALID, return INVALID

3. if SubgroupCheck_g1(P) is INVALID, return INVALID

4. if P = Identity_G1, return INVALID

5. return P 

Note: the identity check is not always necessary above, but it can be nice to have.

@tplooker
Copy link
Member Author

tplooker commented Jul 4, 2022

Labeling as pending close ahead of WG call on the 4th of july

@tplooker
Copy link
Member Author

tplooker commented Aug 8, 2022

Discussed on WG 8th August, there is potentially some inconsistencies that we need to address before we can close this issue

@tplooker
Copy link
Member Author

Discussed on WG call 20th of September, we believe the major inconsistencies around this check are resolved, closing for now and will re-open if some where missed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Associated to the core spec pending-close ready-for-pr
Projects
None yet
Development

No branches or pull requests

2 participants