aws / aws/aws-lc

s2n-bignum: Curve25519 assembly assumes readable text segment

Open
#3,408 2 comments 0 reactions 1 assignee Claimed by @justsmth View on GitHub
Dominant language
Assembly
Stars
830
Forks
212
Avg merge
2d 22h
Merged PRs (30d)
61

Description

The s2n-bignum `curve25519_x25519base_alt.S` implementation places Edwards25519 tables in `.text`:
```
nm:
002a55c8 t Lcurve25519_x25519base_alt_edwards25519_0g
002a5628 t Lcurve25519_x25519base_alt_edwards25519_8g
002a5688 t Lcurve25519_x25519base_alt_edwards25519_gtable
```
OpenBSD's linker maps .text as execute-only:
```
LOAD ... flags --x
```
The assembly does RIP-relative data loads:
```
lea ...,%r10
movq (%r10),%rax
```
which causes SIGSEGV.
Environment:
```sh
# uname -a
OpenBSD openbsd-build 7.9 GENERIC.MP#4 amd64

# clang --version
OpenBSD clang version 19.1.7
Target: amd64-unknown-openbsd7.9
Thread model: posix
InstalledDir: /usr/bin

# ld --version
LLD 19.1.7 (compatible with GNU linkers)

aws/aws-lc commit 88ffcdd
```

You can reproduce this with the following C code:
```c
#include

int main(void)
{
EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new_id(EVP_PKEY_X25519, NULL);
EVP_PKEY *pkey = NULL;

EVP_PKEY_keygen_init(ctx);
EVP_PKEY_keygen(ctx, &pkey);

EVP_PKEY_free(pkey);
EVP_PKEY_CTX_free(ctx);

return 0;
}
```

Moving the constant tables:
- `Lcurve25519_x25519base_alt_edwards25519_0g`
- `Lcurve25519_x25519base_alt_edwards25519_8g`
- `Lcurve25519_x25519base_alt_edwards25519_gtable`

to `.rodata` makes the program run successfully on OpenBSD:

```diff
diff --git a/third_party/s2n-bignum/s2n-bignum-imported/x86_att/curve25519/curve25519_x25519base_alt.S b/third_party/s2n-bignum/s2n-bignum-imported/x86_att/curve25519/curve25519_x25519base_alt.S
index fd28086..69f407e 100644
--- a/third_party/s2n-bignum/s2n-bignum-imported/x86_att/curve25519/curve25519_x25519base_alt.S
+++ b/third_party/s2n-bignum/s2n-bignum-imported/x86_att/curve25519/curve25519_x25519base_alt.S
@@ -2360,6 +2360,8 @@ S2N_BN_SIZE_DIRECTIVE(curve25519_x25519base_alt)
// 2^254 * G and (2^254 + 8) * G in extended-projective coordinates
// but with z = 1 assumed and hence left out, so they are (X,Y,T) only.

+.section .rodata
+.align 8
Lcurve25519_x25519base_alt_edwards25519_0g:

.quad 0x251037f7cf4e861d
@@ -2377,6 +2379,8 @@ Lcurve25519_x25519base_alt_edwards25519_0g:
.quad 0x72e302a348492870
.quad 0x1253c19e53dbe1bc

+.section .rodata
+.align 8
Lcurve25519_x25519base_alt_edwards25519_8g:

.quad 0x331d086e0d9abcaa
@@ -2397,6 +2401,8 @@ Lcurve25519_x25519base_alt_edwards25519_8g:
// Precomputed table of multiples of generator for edwards25519
// all in precomputed extended-projective (y-x,x+y,2*d*x*y) triples.

+.section .rodata
+.align 8
Lcurve25519_x25519base_alt_edwards25519_gtable:

// 2^4 * 1 * G
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.