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.

Sign up for GitHub

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

tighten group magnitude limits, save normalize_weak calls in group add methods (revival of #1032) #1348

Merged
merged 6 commits into from
Aug 16, 2023
Merged

tighten group magnitude limits, save normalize_weak calls in group add methods (revival of #1032) #1348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
690b0fc
add missing group element invariant checks
theStack Jun 14, 2023
4e9661f
Add _fe_verify_magnitude (no-op unless VERIFY is enabled)
peterdettman Dec 5, 2021
49afd2f
Take use of _fe_verify_magnitude in field_impl.h
theStack Jul 13, 2023
173e8d0
Implement current magnitude assumptions
peterdettman Dec 5, 2021
c83afa6
Tighten group magnitude limits
peterdettman Dec 5, 2021
b7c685e
Save _normalize_weak calls in group add methods
peterdettman Dec 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/field.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,4 +352,7 @@ static int secp256k1_fe_is_square_var(const secp256k1_fe *a);
/** Check invariants on a field element (no-op unless VERIFY is enabled). */
static void secp256k1_fe_verify(const secp256k1_fe *a);

/** Check that magnitude of a is at most m (no-op unless VERIFY is enabled). */
static void secp256k1_fe_verify_magnitude(const secp256k1_fe *a, int m);

#endif /* SECP256K1_FIELD_H */
31 changes: 19 additions & 12 deletions src/field_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ SECP256K1_INLINE static int secp256k1_fe_equal(const secp256k1_fe *a, const secp
#ifdef VERIFY
secp256k1_fe_verify(a);
secp256k1_fe_verify(b);
VERIFY_CHECK(a->magnitude <= 1);
VERIFY_CHECK(b->magnitude <= 31);
secp256k1_fe_verify_magnitude(a, 1);
secp256k1_fe_verify_magnitude(b, 31);
#endif
secp256k1_fe_negate(&na, a, 1);
secp256k1_fe_add(&na, b);
Expand All @@ -36,8 +36,8 @@ SECP256K1_INLINE static int secp256k1_fe_equal_var(const secp256k1_fe *a, const
#ifdef VERIFY
secp256k1_fe_verify(a);
secp256k1_fe_verify(b);
VERIFY_CHECK(a->magnitude <= 1);
VERIFY_CHECK(b->magnitude <= 31);
secp256k1_fe_verify_magnitude(a, 1);
secp256k1_fe_verify_magnitude(b, 31);
#endif
secp256k1_fe_negate(&na, a, 1);
secp256k1_fe_add(&na, b);
Expand All @@ -60,7 +60,7 @@ static int secp256k1_fe_sqrt(secp256k1_fe * SECP256K1_RESTRICT r, const secp256k
#ifdef VERIFY
VERIFY_CHECK(r != a);
secp256k1_fe_verify(a);
VERIFY_CHECK(a->magnitude <= 8);
secp256k1_fe_verify_magnitude(a, 8);
#endif

/** The binary representation of (p + 1)/4 has 3 blocks of 1s, with lengths in
Expand Down Expand Up @@ -159,19 +159,26 @@ static int secp256k1_fe_sqrt(secp256k1_fe * SECP256K1_RESTRICT r, const secp256k

#ifndef VERIFY
static void secp256k1_fe_verify(const secp256k1_fe *a) { (void)a; }
static void secp256k1_fe_verify_magnitude(const secp256k1_fe *a, int m) { (void)a; (void)m; }
#else
static void secp256k1_fe_impl_verify(const secp256k1_fe *a);
static void secp256k1_fe_verify(const secp256k1_fe *a) {
/* Magnitude between 0 and 32. */
VERIFY_CHECK((a->magnitude >= 0) && (a->magnitude <= 32));
secp256k1_fe_verify_magnitude(a, 32);
/* Normalized is 0 or 1. */
VERIFY_CHECK((a->normalized == 0) || (a->normalized == 1));
/* If normalized, magnitude must be 0 or 1. */
if (a->normalized) VERIFY_CHECK(a->magnitude <= 1);
if (a->normalized) secp256k1_fe_verify_magnitude(a, 1);
/* Invoke implementation-specific checks. */
secp256k1_fe_impl_verify(a);
}

static void secp256k1_fe_verify_magnitude(const secp256k1_fe *a, int m) {
VERIFY_CHECK(m >= 0);
VERIFY_CHECK(m <= 32);
VERIFY_CHECK(a->magnitude <= m);
}

static void secp256k1_fe_impl_normalize(secp256k1_fe *r);
SECP256K1_INLINE static void secp256k1_fe_normalize(secp256k1_fe *r) {
secp256k1_fe_verify(r);
Expand Down Expand Up @@ -293,7 +300,7 @@ static void secp256k1_fe_impl_negate_unchecked(secp256k1_fe *r, const secp256k1_
SECP256K1_INLINE static void secp256k1_fe_negate_unchecked(secp256k1_fe *r, const secp256k1_fe *a, int m) {
secp256k1_fe_verify(a);
VERIFY_CHECK(m >= 0 && m <= 31);
VERIFY_CHECK(a->magnitude <= m);
secp256k1_fe_verify_magnitude(a, m);
secp256k1_fe_impl_negate_unchecked(r, a, m);
r->magnitude = m + 1;
r->normalized = 0;
Expand Down Expand Up @@ -326,8 +333,8 @@ static void secp256k1_fe_impl_mul(secp256k1_fe *r, const secp256k1_fe *a, const
SECP256K1_INLINE static void secp256k1_fe_mul(secp256k1_fe *r, const secp256k1_fe *a, const secp256k1_fe * SECP256K1_RESTRICT b) {
secp256k1_fe_verify(a);
secp256k1_fe_verify(b);
VERIFY_CHECK(a->magnitude <= 8);
VERIFY_CHECK(b->magnitude <= 8);
secp256k1_fe_verify_magnitude(a, 8);
secp256k1_fe_verify_magnitude(b, 8);
VERIFY_CHECK(r != b);
VERIFY_CHECK(a != b);
secp256k1_fe_impl_mul(r, a, b);
Expand All @@ -339,7 +346,7 @@ SECP256K1_INLINE static void secp256k1_fe_mul(secp256k1_fe *r, const secp256k1_f
static void secp256k1_fe_impl_sqr(secp256k1_fe *r, const secp256k1_fe *a);
SECP256K1_INLINE static void secp256k1_fe_sqr(secp256k1_fe *r, const secp256k1_fe *a) {
secp256k1_fe_verify(a);
VERIFY_CHECK(a->magnitude <= 8);
secp256k1_fe_verify_magnitude(a, 8);
secp256k1_fe_impl_sqr(r, a);
r->magnitude = 1;
r->normalized = 0;
Expand Down Expand Up @@ -418,7 +425,7 @@ SECP256K1_INLINE static void secp256k1_fe_get_bounds(secp256k1_fe* r, int m) {
static void secp256k1_fe_impl_half(secp256k1_fe *r);
SECP256K1_INLINE static void secp256k1_fe_half(secp256k1_fe *r) {
secp256k1_fe_verify(r);
VERIFY_CHECK(r->magnitude < 32);
secp256k1_fe_verify_magnitude(r, 31);
secp256k1_fe_impl_half(r);
r->magnitude = (r->magnitude >> 1) + 1;
r->normalized = 0;
Expand Down
8 changes: 8 additions & 0 deletions src/group.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ typedef struct {

#define SECP256K1_GE_STORAGE_CONST_GET(t) SECP256K1_FE_STORAGE_CONST_GET(t.x), SECP256K1_FE_STORAGE_CONST_GET(t.y)

/** Maximum allowed magnitudes for group element coordinates
* in affine (x, y) and jacobian (x, y, z) representation. */
#define SECP256K1_GE_X_MAGNITUDE_MAX 4
#define SECP256K1_GE_Y_MAGNITUDE_MAX 3
#define SECP256K1_GEJ_X_MAGNITUDE_MAX 4
#define SECP256K1_GEJ_Y_MAGNITUDE_MAX 4
#define SECP256K1_GEJ_Z_MAGNITUDE_MAX 1

/** Set a group element equal to the point with given X and Y coordinates */
static void secp256k1_ge_set_xy(secp256k1_ge *r, const secp256k1_fe *x, const secp256k1_fe *y);

Expand Down
Loading