Skip to content

Commit 76a7570

Browse files
New pynacl version. Minor fixes on key parsing, typing and additional tests (#76)
* Stricter validation on wg secret keys. Typing hints and fixes. * Require two years more recent pynacl version. * Add password-protected minisign test keys. * Test minisign keyfiles. * Test WireGuard key strings. * Increase test coverage Co-authored-by: foonoxous <RWQe4qDs7LIsmI7Q94ctNMSfwfDW8eld24YGevEzQEKZyfJz1NPlH6uf>
1 parent ef35271 commit 76a7570

File tree

5 files changed

+64
-26
lines changed

5 files changed

+64
-26
lines changed

Diff for: covert/pubkey.py

+27-25
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import struct
33
from base64 import b64decode
44
from contextlib import suppress
5+
from typing import Optional
56
from urllib.parse import quote
67
from urllib.request import urlopen
78

@@ -11,12 +12,6 @@
1112
from covert.elliptic import egcreate, egreveal
1213

1314

14-
def derive_symkey(nonce, local, remote):
15-
assert local.sk, f"Missing secret key for {local=}"
16-
shared = sodium.crypto_scalarmult(local.sk, remote.pk)
17-
return sodium.crypto_hash_sha512(bytes(nonce) + shared)[:32]
18-
19-
2015
class Key:
2116

2217
def __init__(self, *, keystr="", comment="", sk=None, pk=None, edsk=None, edpk=None, pkhash=None):
@@ -106,7 +101,13 @@ def _validate(self):
106101
sodium.crypto_box_open(ciphertext, nonce, self.pk, self.sk)
107102

108103

109-
def read_pk_file(keystr):
104+
def derive_symkey(nonce, local: Key, remote: Key) -> bytes:
105+
assert local.sk, f"Missing secret key for {local=}"
106+
shared = sodium.crypto_scalarmult(local.sk, remote.pk)
107+
return sodium.crypto_hash_sha512(bytes(nonce) + shared)[:32]
108+
109+
110+
def read_pk_file(keystr: str) -> list[Key]:
110111
ghuser = None
111112
if keystr.startswith("github:"):
112113
ghuser = keystr[7:]
@@ -135,13 +136,13 @@ def read_pk_file(keystr):
135136
return keys
136137

137138

138-
def read_sk_any(keystr):
139+
def read_sk_any(keystr: str) -> list[Key]:
139140
with suppress(ValueError):
140-
return decode_sk(keystr)
141+
return [decode_sk(keystr)]
141142
return read_sk_file(keystr)
142143

143144

144-
def read_sk_file(keystr):
145+
def read_sk_file(keystr: str) -> list[Key]:
145146
if not os.path.isfile(keystr):
146147
raise ValueError(f"Secret key file {keystr} not found")
147148
with open(keystr, "rb") as f:
@@ -163,16 +164,16 @@ def read_sk_file(keystr):
163164
return keys
164165

165166

166-
def decode_pk(keystr):
167+
def decode_pk(keystr: str) -> Key:
167168
# Age keys use Bech32 encoding
168169
if keystr.startswith("age1"):
169170
return decode_age_pk(keystr)
170171
# Try Base64 encoded formats
171172
try:
172173
token, comment = keystr, ''
173174
if keystr.startswith('ssh-ed25519 '):
174-
t, token, *comment = keystr.split(' ', 2)
175-
comment = comment[0] if comment else 'ssh'
175+
t, token, *cmt = keystr.split(' ', 2)
176+
comment = cmt[0] if cmt else 'ssh'
176177
keybytes = b64decode(token, validate=True)
177178
ssh = keybytes.startswith(b"\x00\x00\x00\x0bssh-ed25519\x00\x00\x00 ")
178179
minisign = len(keybytes) == 42 and keybytes.startswith(b'Ed')
@@ -188,7 +189,7 @@ def decode_pk(keystr):
188189
raise ValueError(f"Unrecognized key {keystr}")
189190

190191

191-
def decode_sk(keystr):
192+
def decode_sk(keystr: str) -> Key:
192193
# Age secret keys in Bech32 encoding
193194
if keystr.lower().startswith("age-secret-key-"):
194195
return decode_age_sk(keystr)
@@ -198,48 +199,49 @@ def decode_sk(keystr):
198199
# Plain Curve25519 key (WireGuard)
199200
try:
200201
keybytes = b64decode(keystr, validate=True)
201-
if len(keybytes) == 32:
202-
return Key(sk=keybytes)
202+
# Must be a clamped scalar
203+
if len(keybytes) == 32 and keybytes[0] & 8 == 0 and keybytes[31] & 0xC0 == 0x40:
204+
return Key(keystr=keystr, sk=keybytes, comment="wg")
203205
except ValueError:
204206
pass
205-
raise ValueError(f"Unable to parse private key {keystr!r}")
207+
raise ValueError(f"Unable to parse secret key {keystr!r}")
206208

207209

208-
def decode_sk_minisign(keystr, pw=None):
210+
def decode_sk_minisign(keystr: str, pw: Optional[bytes] = None) -> Key:
209211
# None means try without password, then ask
210212
if pw is None:
211213
try:
212214
return decode_sk_minisign(keystr, b'')
213215
except ValueError:
214216
pass
215-
pw = util.encode(passphrase.ask('Minisign passkey')[0])
217+
pw = passphrase.ask('Minisign passkey')[0]
216218
return decode_sk_minisign(keystr, pw)
217219
data = b64decode(keystr)
218220
fmt, salt, ops, mem, token = struct.unpack('<6s32sQQ104s', data)
219221
if fmt != b'EdScB2' or ops != 1 << 25 or mem != 1 << 30:
220222
raise ValueError(f'Not a (supported) Minisign secret key {fmt=}')
221-
out = sodium.crypto_pwhash_scryptsalsa208sha256_ll(pw, salt, n=1 << 20, r=8, p=1, maxmem=float('inf'), dklen=104)
223+
out = sodium.crypto_pwhash_scryptsalsa208sha256_ll(pw, salt, n=1 << 20, r=8, p=1, maxmem=1 << 31, dklen=104)
222224
token = util.xor(out, token)
223225
keyid, edsk, edpk, csum = struct.unpack('8s32s32s32s', token)
224226
b2state = sodium.crypto_generichash_blake2b_init()
225227
sodium.crypto_generichash.generichash_blake2b_update(b2state, fmt[:2] + keyid + edsk + edpk)
226228
csum2 = sodium.crypto_generichash.generichash_blake2b_final(b2state)
227229
if csum != csum2:
228230
raise ValueError('Unable to decrypt Minisign secret key')
229-
return Key(edsk=edsk, edpk=edpk)
231+
return Key(edsk=edsk, edpk=edpk, comment="ms")
230232

231233

232-
def decode_age_pk(keystr):
234+
def decode_age_pk(keystr: str) -> Key:
233235
return Key(keystr=keystr, comment="age", pk=bech.decode("age", keystr.lower()))
234236

235237

236-
def encode_age_pk(key):
238+
def encode_age_pk(key: Key) -> str:
237239
return bech.encode("age", key.pk)
238240

239241

240-
def decode_age_sk(keystr):
242+
def decode_age_sk(keystr: str) -> Key:
241243
return Key(keystr=keystr, comment="age", sk=bech.decode("age-secret-key-", keystr.lower()))
242244

243245

244-
def encode_age_sk(key):
246+
def encode_age_sk(key: Key) -> str:
245247
return bech.encode("age-secret-key-", key.sk).upper()

Diff for: setup.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
"bcrypt>=3.0.0",
2323
"colorama>=0.4",
2424
"cryptography>=35",
25-
"pynacl>=1.4",
25+
"pynacl>=1.5",
2626
"tqdm>=4.62",
2727
"msgpack>=1.0",
2828
"pyperclip>=1.8",

Diff for: tests/keys/minisign_password.key

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
untrusted comment: minisign encrypted secret key
2+
RWRTY0IyhY/6g2XLCLS4D+DoqjxeOJDUR9gv+ilrmp3/B4KpIaIAAAACAAAAAAAAAEAAAAAAF0HHgdqvvXFEO3QEYm11JrGdnfjZFAWeO38bLT98FUoDxGdcgvCdeCmmj8ZiHCHeTpfablIfRrEWEvjho5yyTciuN/mr6j0YAKfd3Ew9SkUDRY/t8qvvQz1bxKHdYwZRk1RChapZ32U=

Diff for: tests/keys/minisign_password.pub

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
untrusted comment: minisign public key 8A1B111AFC8CA1A
2+
RWQaysivEbGhCD/XmdIesSX8kAROXUUSTp5M4ochKA+Ia0Iou0KgWyhr

Diff for: tests/test_pubkey.py

+32
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212
AGE_SK = "AGE-SECRET-KEY-1GFPYYSJZGFPYYSJZGFPYYSJZGFPYYSJZGFPYYSJZGFPYYSJZGFPQ4EGAEX"
1313
AGE_SK_BYTES = 32 * b"\x42"
1414

15+
# Generated with wg genkey and wg pubkey
16+
WG_SK = "kLkIpWh5MYKwUA7JdQHnmbc6dEiW0py4VRvqmYyPLHc="
17+
WG_PK = "ElMfFd2qVIROK4mRaXJouYWC2lxxMApMSe9KyAZcEBc="
18+
1519

1620
def test_age_key_decoding():
1721
pk = pubkey.decode_pk(AGE_PK)
@@ -36,6 +40,25 @@ def test_age_key_decoding_and_encoding():
3640
assert pubkey.encode_age_sk(sk) == AGE_SK
3741

3842

43+
def test_wireguard_keystr():
44+
pk = pubkey.decode_pk(WG_PK)
45+
sk = pubkey.decode_sk(WG_SK)
46+
# Key comparison is by public keys
47+
assert pk == sk
48+
assert pk.keystr == WG_PK
49+
assert sk.keystr == WG_SK
50+
assert pk.comment == 'wg'
51+
assert sk.comment == 'wg'
52+
assert repr(pk).endswith(':PK]')
53+
assert repr(sk).endswith(':SK]')
54+
55+
# Trying to decode a public key as secret key should usually fail
56+
# (works with the test key but no guarantees with others)
57+
with pytest.raises(ValueError) as exc:
58+
pubkey.decode_sk(WG_PK)
59+
assert "Unable to parse secret key" in str(exc.value)
60+
61+
3962
def test_ssh_key_decoding():
4063
pk, = pubkey.read_pk_file("tests/keys/ssh_ed25519.pub")
4164
sk, = pubkey.read_sk_file("tests/keys/ssh_ed25519")
@@ -56,6 +79,15 @@ def test_ssh_wrong_password(mocker):
5679
sk, = pubkey.read_sk_file("tests/keys/ssh_ed25519_password")
5780

5881

82+
def test_minisign_keyfiles(mocker):
83+
mocker.patch('covert.passphrase.ask', return_value=(b"password", True))
84+
sk, = pubkey.read_sk_file("tests/keys/minisign_password.key")
85+
pk, = pubkey.read_pk_file("tests/keys/minisign_password.pub")
86+
assert sk.comment == 'ms'
87+
assert pk.comment == 'ms'
88+
assert sk == pk
89+
90+
5991
def test_key_exchange():
6092
# Alice sends a message to Bob
6193
nonce = token_bytes(12)

0 commit comments

Comments
 (0)