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

hash: Make code agnostic of endianness #1093

Merged
merged 3 commits into from
Mar 28, 2022
Merged

hash: Make code agnostic of endianness #1093

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8d89b9e
hash: Make code agnostic of endianness
real-or-random Mar 25, 2022
616b43d
util: Remove endianness detection
real-or-random Mar 25, 2022
37d3692
tests: Add tests for _read_be32 and _write_be32
real-or-random Mar 26, 2022
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
2 changes: 1 addition & 1 deletion src/hash.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

typedef struct {
uint32_t s[8];
uint32_t buf[16]; /* In big endian */
unsigned char buf[64];
uint64_t bytes;
} secp256k1_sha256;

Expand Down
57 changes: 24 additions & 33 deletions src/hash_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,6 @@
(h) = t1 + t2; \
} while(0)

#if defined(SECP256K1_BIG_ENDIAN)
#define BE32(x) (x)
#elif defined(SECP256K1_LITTLE_ENDIAN)
#define BE32(p) ((((p) & 0xFF) << 24) | (((p) & 0xFF00) << 8) | (((p) & 0xFF0000) >> 8) | (((p) & 0xFF000000) >> 24))
#endif

static void secp256k1_sha256_initialize(secp256k1_sha256 *hash) {
hash->s[0] = 0x6a09e667ul;
hash->s[1] = 0xbb67ae85ul;
Expand All @@ -47,26 +41,26 @@ static void secp256k1_sha256_initialize(secp256k1_sha256 *hash) {
}

/** Perform one SHA-256 transformation, processing 16 big endian 32-bit words. */
static void secp256k1_sha256_transform(uint32_t* s, const uint32_t* chunk) {
static void secp256k1_sha256_transform(uint32_t* s, const unsigned char* buf) {
uint32_t a = s[0], b = s[1], c = s[2], d = s[3], e = s[4], f = s[5], g = s[6], h = s[7];
uint32_t w0, w1, w2, w3, w4, w5, w6, w7, w8, w9, w10, w11, w12, w13, w14, w15;

Round(a, b, c, d, e, f, g, h, 0x428a2f98, w0 = BE32(chunk[0]));
Round(h, a, b, c, d, e, f, g, 0x71374491, w1 = BE32(chunk[1]));
Round(g, h, a, b, c, d, e, f, 0xb5c0fbcf, w2 = BE32(chunk[2]));
Round(f, g, h, a, b, c, d, e, 0xe9b5dba5, w3 = BE32(chunk[3]));
Round(e, f, g, h, a, b, c, d, 0x3956c25b, w4 = BE32(chunk[4]));
Round(d, e, f, g, h, a, b, c, 0x59f111f1, w5 = BE32(chunk[5]));
Round(c, d, e, f, g, h, a, b, 0x923f82a4, w6 = BE32(chunk[6]));
Round(b, c, d, e, f, g, h, a, 0xab1c5ed5, w7 = BE32(chunk[7]));
Round(a, b, c, d, e, f, g, h, 0xd807aa98, w8 = BE32(chunk[8]));
Round(h, a, b, c, d, e, f, g, 0x12835b01, w9 = BE32(chunk[9]));
Round(g, h, a, b, c, d, e, f, 0x243185be, w10 = BE32(chunk[10]));
Round(f, g, h, a, b, c, d, e, 0x550c7dc3, w11 = BE32(chunk[11]));
Round(e, f, g, h, a, b, c, d, 0x72be5d74, w12 = BE32(chunk[12]));
Round(d, e, f, g, h, a, b, c, 0x80deb1fe, w13 = BE32(chunk[13]));
Round(c, d, e, f, g, h, a, b, 0x9bdc06a7, w14 = BE32(chunk[14]));
Round(b, c, d, e, f, g, h, a, 0xc19bf174, w15 = BE32(chunk[15]));
Round(a, b, c, d, e, f, g, h, 0x428a2f98, w0 = secp256k1_read_be32(&buf[0]));
Round(h, a, b, c, d, e, f, g, 0x71374491, w1 = secp256k1_read_be32(&buf[4]));
Round(g, h, a, b, c, d, e, f, 0xb5c0fbcf, w2 = secp256k1_read_be32(&buf[8]));
Round(f, g, h, a, b, c, d, e, 0xe9b5dba5, w3 = secp256k1_read_be32(&buf[12]));
Round(e, f, g, h, a, b, c, d, 0x3956c25b, w4 = secp256k1_read_be32(&buf[16]));
Round(d, e, f, g, h, a, b, c, 0x59f111f1, w5 = secp256k1_read_be32(&buf[20]));
Round(c, d, e, f, g, h, a, b, 0x923f82a4, w6 = secp256k1_read_be32(&buf[24]));
Round(b, c, d, e, f, g, h, a, 0xab1c5ed5, w7 = secp256k1_read_be32(&buf[28]));
Round(a, b, c, d, e, f, g, h, 0xd807aa98, w8 = secp256k1_read_be32(&buf[32]));
Round(h, a, b, c, d, e, f, g, 0x12835b01, w9 = secp256k1_read_be32(&buf[36]));
Round(g, h, a, b, c, d, e, f, 0x243185be, w10 = secp256k1_read_be32(&buf[40]));
Round(f, g, h, a, b, c, d, e, 0x550c7dc3, w11 = secp256k1_read_be32(&buf[44]));
Round(e, f, g, h, a, b, c, d, 0x72be5d74, w12 = secp256k1_read_be32(&buf[48]));
Round(d, e, f, g, h, a, b, c, 0x80deb1fe, w13 = secp256k1_read_be32(&buf[52]));
Round(c, d, e, f, g, h, a, b, 0x9bdc06a7, w14 = secp256k1_read_be32(&buf[56]));
Round(b, c, d, e, f, g, h, a, 0xc19bf174, w15 = secp256k1_read_be32(&buf[60]));

Round(a, b, c, d, e, f, g, h, 0xe49b69c1, w0 += sigma1(w14) + w9 + sigma0(w1));
Round(h, a, b, c, d, e, f, g, 0xefbe4786, w1 += sigma1(w15) + w10 + sigma0(w2));
Expand Down Expand Up @@ -136,7 +130,7 @@ static void secp256k1_sha256_write(secp256k1_sha256 *hash, const unsigned char *
while (len >= 64 - bufsize) {
/* Fill the buffer, and process it. */
size_t chunk_len = 64 - bufsize;
memcpy(((unsigned char*)hash->buf) + bufsize, data, chunk_len);
memcpy(hash->buf + bufsize, data, chunk_len);
data += chunk_len;
len -= chunk_len;
secp256k1_sha256_transform(hash->s, hash->buf);
Expand All @@ -150,20 +144,18 @@ static void secp256k1_sha256_write(secp256k1_sha256 *hash, const unsigned char *

static void secp256k1_sha256_finalize(secp256k1_sha256 *hash, unsigned char *out32) {
static const unsigned char pad[64] = {0x80};
uint32_t sizedesc[2];
uint32_t out[8];
int i = 0;
unsigned char sizedesc[8];
int i;
/* The maximum message size of SHA256 is 2^64-1 bits. */
VERIFY_CHECK(hash->bytes < ((uint64_t)1 << 61));
sizedesc[0] = BE32(hash->bytes >> 29);
sizedesc[1] = BE32(hash->bytes << 3);
secp256k1_write_be32(&sizedesc[0], hash->bytes >> 29);
secp256k1_write_be32(&sizedesc[4], hash->bytes << 3);
secp256k1_sha256_write(hash, pad, 1 + ((119 - (hash->bytes % 64)) % 64));
secp256k1_sha256_write(hash, (const unsigned char*)sizedesc, 8);
secp256k1_sha256_write(hash, sizedesc, 8);
for (i = 0; i < 8; i++) {
out[i] = BE32(hash->s[i]);
secp256k1_write_be32(&out32[4*i], hash->s[i]);
hash->s[i] = 0;
}
memcpy(out32, (const unsigned char*)out, 32);
}

/* Initializes a sha256 struct and writes the 64 byte string
Expand Down Expand Up @@ -287,7 +279,6 @@ static void secp256k1_rfc6979_hmac_sha256_finalize(secp256k1_rfc6979_hmac_sha256
rng->retry = 0;
}

#undef BE32
#undef Round
#undef sigma1
#undef sigma0
Expand Down
14 changes: 14 additions & 0 deletions src/tests.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6887,6 +6887,19 @@ void run_secp256k1_memczero_test(void) {
CHECK(secp256k1_memcmp_var(buf1, buf2, sizeof(buf1)) == 0);
}

void run_secp256k1_byteorder_tests(void) {
const uint32_t x = 0xFF03AB45;
const unsigned char x_be[4] = {0xFF, 0x03, 0xAB, 0x45};
unsigned char buf[4];
uint32_t x_;

secp256k1_write_be32(buf, x);
CHECK(secp256k1_memcmp_var(buf, x_be, sizeof(buf)) == 0);

x_ = secp256k1_read_be32(buf);
CHECK(x == x_);
}

void int_cmov_test(void) {
int r = INT_MAX;
int a = 0;
Expand Down Expand Up @@ -7161,6 +7174,7 @@ int main(int argc, char **argv) {

/* util tests */
run_secp256k1_memczero_test();
run_secp256k1_byteorder_tests();

run_cmov_tests();

Expand Down
41 changes: 16 additions & 25 deletions src/util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,31 +173,6 @@ static SECP256K1_INLINE void *checked_realloc(const secp256k1_callback* cb, void
# define SECP256K1_GNUC_EXT
#endif

/* If SECP256K1_{LITTLE,BIG}_ENDIAN is not explicitly provided, infer from various other system macros. */
#if !defined(SECP256K1_LITTLE_ENDIAN) && !defined(SECP256K1_BIG_ENDIAN)
/* Inspired by https://github.com/rofl0r/endianness.h/blob/9853923246b065a3b52d2c43835f3819a62c7199/endianness.h#L52L73 */
# if (defined(__BYTE_ORDER__) && defined(__ORDER_LITTLE_ENDIAN__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__) || \
defined(_X86_) || defined(__x86_64__) || defined(__i386__) || \
defined(__i486__) || defined(__i586__) || defined(__i686__) || \
defined(__MIPSEL) || defined(_MIPSEL) || defined(MIPSEL) || \
defined(__ARMEL__) || defined(__AARCH64EL__) || \
(defined(__LITTLE_ENDIAN__) && __LITTLE_ENDIAN__ == 1) || \
(defined(_LITTLE_ENDIAN) && _LITTLE_ENDIAN == 1) || \
defined(_M_IX86) || defined(_M_AMD64) || defined(_M_ARM) /* MSVC */
# define SECP256K1_LITTLE_ENDIAN
# endif
# if (defined(__BYTE_ORDER__) && defined(__ORDER_BIG_ENDIAN__) && __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__) || \
defined(__MIPSEB) || defined(_MIPSEB) || defined(MIPSEB) || \
defined(__MICROBLAZEEB__) || defined(__ARMEB__) || defined(__AARCH64EB__) || \
(defined(__BIG_ENDIAN__) && __BIG_ENDIAN__ == 1) || \
(defined(_BIG_ENDIAN) && _BIG_ENDIAN == 1)
# define SECP256K1_BIG_ENDIAN
# endif
#endif
#if defined(SECP256K1_LITTLE_ENDIAN) == defined(SECP256K1_BIG_ENDIAN)
# error Please make sure that either SECP256K1_LITTLE_ENDIAN or SECP256K1_BIG_ENDIAN is set, see src/util.h.
#endif

/* Zero memory if flag == 1. Flag must be 0 or 1. Constant time. */
static SECP256K1_INLINE void secp256k1_memczero(void *s, size_t len, int flag) {
unsigned char *p = (unsigned char *)s;
Expand Down Expand Up @@ -338,4 +313,20 @@ static SECP256K1_INLINE int secp256k1_ctz64_var(uint64_t x) {
#endif
}

/* Read a uint32_t in big endian */
SECP256K1_INLINE static uint32_t secp256k1_read_be32(const unsigned char* p) {
return (uint32_t)p[0] << 24 |
Copy link

Choose a reason for hiding this comment

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

can you add more parens around the cast

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can do it in a follow up PR if you insist but the code is correct given C's operator precedence.

Copy link

Choose a reason for hiding this comment

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

like this:

(((uint32_t)p[0]) << 24) |
(((uint32_t)p[1]) << 16) |
(((uint32_t)p[2]) << 8) |
((uint32_t)p[3])

is much easier to read IMO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I agree, it may be easier to review for some reviewers because they don't need to to spend time looking up the rules. But given that it has been merged (i.e., review has been done), I don't think it's worth spending a PR on this. Feel free to open a PR if you disagree.

(Sorry, I hadn't seen your comment earlier. I was spending some time testing this locally before pushing the merge. Your comment arrived after I had created the merge commit locally, but I believe before I pushed it. Then I didn't check github again before pushing the merge.)

(uint32_t)p[1] << 16 |
(uint32_t)p[2] << 8 |
(uint32_t)p[3];
}

/* Write a uint32_t in big endian */
SECP256K1_INLINE static void secp256k1_write_be32(unsigned char* p, uint32_t x) {
p[3] = x;
p[2] = x >> 8;
p[1] = x >> 16;
p[0] = x >> 24;
}

#endif /* SECP256K1_UTIL_H */