Skip to content

Commit 38c0e8a

Browse files
ThomasK33ThomasK33
authored
fix(agent/agentssh): ensure RSA key generation always produces valid keys (coder#16694)
Modify the RSA key generation algorithm to check that GCD(e, p-1) = 1 and GCD(e, q-1) = 1 when selecting prime numbers, ensuring that e and φ(n) are coprime. This prevents ModInverse from returning nil, which would cause private key generation to fail and result in a panic when `Precompute` is called. Change-Id: I0a453e1e1f8c638e40e7a4b87a6d0d7299e1cb5d Signed-off-by: Thomas Kosiewski <tk@coder.com>
1 parent 172e523 commit 38c0e8a

File tree

3 files changed

+139
-72
lines changed

3 files changed

+139
-72
lines changed

‎agent/agentrsa/key.go

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
package agentrsa
2+
3+
import (
4+
"crypto/rsa"
5+
"math/big"
6+
"math/rand"
7+
)
8+
9+
// GenerateDeterministicKey generates an RSA private key deterministically based on the provided seed.
10+
// This function uses a deterministic random source to generate the primes p and q, ensuring that the
11+
// same seed will always produce the same private key. The generated key is 2048 bits in size.
12+
//
13+
// Reference: https://pkg.go.dev/crypto/rsa#GenerateKey
14+
func GenerateDeterministicKey(seed int64) *rsa.PrivateKey {
15+
// Since the standard lib purposefully does not generate
16+
// deterministic rsa keys, we need to do it ourselves.
17+
18+
// Create deterministic random source
19+
// nolint: gosec
20+
deterministicRand := rand.New(rand.NewSource(seed))
21+
22+
// Use fixed values for p and q based on the seed
23+
p := big.NewInt(0)
24+
q := big.NewInt(0)
25+
e := big.NewInt(65537) // Standard RSA public exponent
26+
27+
for {
28+
// Generate deterministic primes using the seeded random
29+
// Each prime should be ~1024 bits to get a 2048-bit key
30+
for {
31+
p.SetBit(p, 1024, 1) // Ensure it's large enough
32+
for i := range 1024 {
33+
if deterministicRand.Int63()%2 == 1 {
34+
p.SetBit(p, i, 1)
35+
} else {
36+
p.SetBit(p, i, 0)
37+
}
38+
}
39+
p1 := new(big.Int).Sub(p, big.NewInt(1))
40+
if p.ProbablyPrime(20) && new(big.Int).GCD(nil, nil, e, p1).Cmp(big.NewInt(1)) == 0 {
41+
break
42+
}
43+
}
44+
45+
for {
46+
q.SetBit(q, 1024, 1) // Ensure it's large enough
47+
for i := range 1024 {
48+
if deterministicRand.Int63()%2 == 1 {
49+
q.SetBit(q, i, 1)
50+
} else {
51+
q.SetBit(q, i, 0)
52+
}
53+
}
54+
q1 := new(big.Int).Sub(q, big.NewInt(1))
55+
if q.ProbablyPrime(20) && p.Cmp(q) != 0 && new(big.Int).GCD(nil, nil, e, q1).Cmp(big.NewInt(1)) == 0 {
56+
break
57+
}
58+
}
59+
60+
// Calculate phi = (p-1) * (q-1)
61+
p1 := new(big.Int).Sub(p, big.NewInt(1))
62+
q1 := new(big.Int).Sub(q, big.NewInt(1))
63+
phi := new(big.Int).Mul(p1, q1)
64+
65+
// Calculate private exponent d
66+
d := new(big.Int).ModInverse(e, phi)
67+
if d != nil {
68+
// Calculate n = p * q
69+
n := new(big.Int).Mul(p, q)
70+
71+
// Create the private key
72+
privateKey := &rsa.PrivateKey{
73+
PublicKey: rsa.PublicKey{
74+
N: n,
75+
E: int(e.Int64()),
76+
},
77+
D: d,
78+
Primes: []*big.Int{p, q},
79+
}
80+
81+
// Compute precomputed values
82+
privateKey.Precompute()
83+
84+
return privateKey
85+
}
86+
}
87+
}

‎agent/agentrsa/key_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
package agentrsa_test
2+
3+
import (
4+
"crypto/rsa"
5+
"math/rand/v2"
6+
"testing"
7+
8+
"github.com/stretchr/testify/assert"
9+
10+
"github.com/coder/coder/v2/agent/agentrsa"
11+
)
12+
13+
func TestGenerateDeterministicKey(t *testing.T) {
14+
t.Parallel()
15+
16+
key1 := agentrsa.GenerateDeterministicKey(1234)
17+
key2 := agentrsa.GenerateDeterministicKey(1234)
18+
19+
assert.Equal(t, key1, key2)
20+
assert.EqualExportedValues(t, key1, key2)
21+
}
22+
23+
var result *rsa.PrivateKey
24+
25+
func BenchmarkGenerateDeterministicKey(b *testing.B) {
26+
var r *rsa.PrivateKey
27+
28+
for range b.N {
29+
// always record the result of DeterministicPrivateKey to prevent
30+
// the compiler eliminating the function call.
31+
r = agentrsa.GenerateDeterministicKey(rand.Int64())
32+
}
33+
34+
// always store the result to a package level variable
35+
// so the compiler cannot eliminate the Benchmark itself.
36+
result = r
37+
}
38+
39+
func FuzzGenerateDeterministicKey(f *testing.F) {
40+
testcases := []int64{0, 1234, 1010101010}
41+
for _, tc := range testcases {
42+
f.Add(tc) // Use f.Add to provide a seed corpus
43+
}
44+
f.Fuzz(func(t *testing.T, seed int64) {
45+
key1 := agentrsa.GenerateDeterministicKey(seed)
46+
key2 := agentrsa.GenerateDeterministicKey(seed)
47+
assert.Equal(t, key1, key2)
48+
assert.EqualExportedValues(t, key1, key2)
49+
})
50+
}

‎agent/agentssh/agentssh.go

Lines changed: 2 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,9 @@ package agentssh
33
import (
44
"bufio"
55
"context"
6-
"crypto/rsa"
76
"errors"
87
"fmt"
98
"io"
10-
"math/big"
11-
"math/rand"
129
"net"
1310
"os"
1411
"os/exec"
@@ -33,6 +30,7 @@ import (
3330
"cdr.dev/slog"
3431

3532
"github.com/coder/coder/v2/agent/agentexec"
33+
"github.com/coder/coder/v2/agent/agentrsa"
3634
"github.com/coder/coder/v2/agent/usershell"
3735
"github.com/coder/coder/v2/codersdk"
3836
"github.com/coder/coder/v2/pty"
@@ -1092,75 +1090,7 @@ func CoderSigner(seed int64) (gossh.Signer, error) {
10921090
// Clients should ignore the host key when connecting.
10931091
// The agent needs to authenticate with coderd to SSH,
10941092
// so SSH authentication doesn't improve security.
1095-
1096-
// Since the standard lib purposefully does not generate
1097-
// deterministic rsa keys, we need to do it ourselves.
1098-
coderHostKey := func() *rsa.PrivateKey {
1099-
// Create deterministic random source
1100-
// nolint: gosec
1101-
deterministicRand := rand.New(rand.NewSource(seed))
1102-
1103-
// Use fixed values for p and q based on the seed
1104-
p := big.NewInt(0)
1105-
q := big.NewInt(0)
1106-
e := big.NewInt(65537) // Standard RSA public exponent
1107-
1108-
// Generate deterministic primes using the seeded random
1109-
// Each prime should be ~1024 bits to get a 2048-bit key
1110-
for {
1111-
p.SetBit(p, 1024, 1) // Ensure it's large enough
1112-
for i := 0; i < 1024; i++ {
1113-
if deterministicRand.Int63()%2 == 1 {
1114-
p.SetBit(p, i, 1)
1115-
} else {
1116-
p.SetBit(p, i, 0)
1117-
}
1118-
}
1119-
if p.ProbablyPrime(20) {
1120-
break
1121-
}
1122-
}
1123-
1124-
for {
1125-
q.SetBit(q, 1024, 1) // Ensure it's large enough
1126-
for i := 0; i < 1024; i++ {
1127-
if deterministicRand.Int63()%2 == 1 {
1128-
q.SetBit(q, i, 1)
1129-
} else {
1130-
q.SetBit(q, i, 0)
1131-
}
1132-
}
1133-
if q.ProbablyPrime(20) && p.Cmp(q) != 0 {
1134-
break
1135-
}
1136-
}
1137-
1138-
// Calculate n = p * q
1139-
n := new(big.Int).Mul(p, q)
1140-
1141-
// Calculate phi = (p-1) * (q-1)
1142-
p1 := new(big.Int).Sub(p, big.NewInt(1))
1143-
q1 := new(big.Int).Sub(q, big.NewInt(1))
1144-
phi := new(big.Int).Mul(p1, q1)
1145-
1146-
// Calculate private exponent d
1147-
d := new(big.Int).ModInverse(e, phi)
1148-
1149-
// Create the private key
1150-
privateKey := &rsa.PrivateKey{
1151-
PublicKey: rsa.PublicKey{
1152-
N: n,
1153-
E: int(e.Int64()),
1154-
},
1155-
D: d,
1156-
Primes: []*big.Int{p, q},
1157-
}
1158-
1159-
// Compute precomputed values
1160-
privateKey.Precompute()
1161-
1162-
return privateKey
1163-
}()
1093+
coderHostKey := agentrsa.GenerateDeterministicKey(seed)
11641094

11651095
coderSigner, err := gossh.NewSignerFromKey(coderHostKey)
11661096
return coderSigner, err

0 commit comments

Comments
 (0)