derivepassphrase.git

Fix, and test for, the consistency of the SSH test keys
Parent ac6134a
Marco Ricci

Marco Ricci commited on 2025-07-26 11:45:19
Zeige 2 geänderte Dateien mit 237 Einfügungen und 3 Löschungen. 

A typo existed in the ECDSA NIST P-521 raw public key data, making it
incompatible with the copy of the public key included in the raw private
key blob. This caused UNIX Pageant (and likely any other SSH agent
supporting deterministic DSA signatures, if they exist) to reject
unloading and signing with that key, despite actually supporting the key
format. So fix the typo, and add tests and machinery to ensure
rudimentary consistency of all test key data.
tests/__init__.py
Zeige Datei @ f567393
... ... 
@@ -68,13 +68,13 @@ class SSHTestKey(NamedTuple):
68 68
69 69 
     """
70 70
71 
-    public_key: bytes | str
71 
+    public_key: bytes
72 72 
     """"""
73 73 
     public_key_data: bytes
74 74 
     """"""
75 75 
     private_key: bytes
76 76 
     """"""
77 
-    private_key_blob: bytes | None = None
77 
+    private_key_blob: bytes
78 78 
     """"""
79 79 
     expected_signature: bytes | None = None
80 80 
     """"""
... ... 
@@ -1126,7 +1126,7 @@ Rlc3Qga2V5IHdpdGhvdXQgcGFzc3BocmFzZQ==
1126 1126 
 """,
1127 1127 
         public_key_data=bytes.fromhex("""
1128 1128 
             00 00 00 13 65 63 64
1129 
-            73 61 2d 73 68 61 32 2d 6e 69 73 74 70 32 35 36
1129 
+            73 61 2d 73 68 61 32 2d 6e 69 73 74 70 35 32 31
1130 1130 
             00 00 00 08 6e 69 73 74 70 35 32 31
1131 1131 
             00 00 00 85 04 00 49 53 9d
1132 1132 
             c0 3c e7 9a 57 06 aa 22 ef 16 d6 1e 56 da c0 12
tests/test_000_testing_machinery.py
Zeige Datei @ f567393
... ... 
@@ -0,0 +1,234 @@
1 
+# SPDX-FileCopyrightText: 2025 Marco Ricci <software@the13thletter.info>
2 
+#
3 
+# SPDX-License-Identifier: Zlib
4 
+
5 
+from __future__ import annotations
6 
+
7 
+import base64
8 
+
9 
+import pytest
10 
+
11 
+import tests
12 
+from derivepassphrase import ssh_agent
13 
+
14 
+OPENSSH_MAGIC = b'openssh-key-v1\x00'
15 
+OPENSSH_HEADER = (
16 
+    OPENSSH_MAGIC  # magic
17 
+    + b'\x00\x00\x00\x04none'  # ciphername
18 
+    + b'\x00\x00\x00\x04none'  # kdfname
19 
+    + b'\x00\x00\x00\x00'  # kdfoptions
20 
+    + b'\x00\x00\x00\x01'  # number of keys
21 
+)
22 
+
23 
+
24 
+def as_openssh_keyfile_payload(
25 
+    public_key: bytes, private_key: bytes, checkint: int
26 
+) -> bytes:
27 
+    """Format an SSH private key in OpenSSH format.
28 
+
29 
+    Args:
30 
+        public_key:
31 
+            The unframed public key, in SSH wire format.
32 
+        private_key:
33 
+            The unframed private key, in SSH wire format, including the
34 
+            comment.
35 
+        checkint:
36 
+            The "check" integer to use.
37 
+
38 
+    Returns:
39 
+        The payload for a formatted OpenSSH private key, as a byte
40 
+        string, without the base64 encoding and the framing lines.
41 
+
42 
+    """
43 
+    # The OpenSSH private key file format is described in PROTOCOL.key
44 
+    # in their git repository; see below for links to OpenSSH 10.0p2.
45 
+    # The block size of the "none" cipher is 8 bytes; see line 108 of
46 
+    # cipher.c, with definitions from line 67 onwards.  Padding is not
47 
+    # used if the payload already is a multiple of 8 bytes long; see
48 
+    # line 2935 onwards of sshkey.c
49 
+    #
50 
+    # https://github.com/openssh/openssh-portable/raw/2593769fb291fe6c542173927698c69e9f9a08b9/PROTOCOL.key
51 
+    # https://github.com/openssh/openssh-portable/raw/2593769fb291fe6c542173927698c69e9f9a08b9/cipher.c
52 
+    # https://github.com/openssh/openssh-portable/raw/2593769fb291fe6c542173927698c69e9f9a08b9/sshkey.c
53 
+    string = ssh_agent.SSHAgentClient.string
54 
+    uint32 = ssh_agent.SSHAgentClient.uint32
55 
+    payload = bytearray(OPENSSH_HEADER)
56 
+    payload.extend(string(public_key))
57 
+    secret = bytearray()
58 
+    secret.extend(uint32(checkint))  # checkint
59 
+    secret.extend(uint32(checkint))  # checkint
60 
+    secret.extend(private_key)  # privatekey1 and comment1
61 
+    i = 1
62 
+    while len(secret) % 16 != 0:
63 
+        secret.append(i)
64 
+        i += 1
65 
+    payload.extend(string(secret))  # encrypted, padded list of private keys
66 
+    return bytes(payload)
67 
+
68 
+
69 
+def minimize_openssh_keyfile_padding(
70 
+    decoded_openssh_private_key: bytes,
71 
+) -> bytes:
72 
+    """Minimize the padding used in an OpenSSH private key file.
73 
+
74 
+    Args:
75 
+        decoded_openssh_private_key:
76 
+            The non-base64-encoded, unframed, formatted OpenSSH private
77 
+            key.
78 
+
79 
+    Returns:
80 
+        The same non-base64-encoded, unframed, formatted OpenSSH private
81 
+        key, but with minimal padding applied.
82 
+
83 
+    """
84 
+    string = ssh_agent.SSHAgentClient.string
85 
+    unstring_prefix = ssh_agent.SSHAgentClient.unstring_prefix
86 
+
87 
+    _public_key, framed_private_block = unstring_prefix(
88 
+        decoded_openssh_private_key.removeprefix(OPENSSH_HEADER)
89 
+    )
90 
+    result = bytearray(decoded_openssh_private_key).removesuffix(
91 
+        framed_private_block
92 
+    )
93 
+    private_block, trailer = unstring_prefix(framed_private_block)
94 
+    assert not trailer
95 
+
96 
+    # Skip two checkint values.
97 
+    key_type, remainder = unstring_prefix(private_block[8:])
98 
+    # We need to semi-generically skip private key payloads.  Currently,
99 
+    # all supported (test) key types exclusively store multi-precision
100 
+    # integers or strings as their private key payload (which are both
101 
+    # parsed the same way, but interpreted differently).  We can
102 
+    # therefore generically parse `k` strings/mpints (for different
103 
+    # values of `k`, depending on key type) to correctly skip the
104 
+    # private key payload, and don't have to deal with having to parse
105 
+    # and skip other types of data such as uint32s.
106 
+    #
107 
+    # (This scheme needs updating if ever a different data type needs to
108 
+    # be parsed.)
109 
+    num_mpints = {
110 
+        b'ssh-ed25519': 2,
111 
+        b'ssh-ed448': 2,
112 
+        b'ssh-rsa': 6,
113 
+        b'ssh-dss': 5,
114 
+        b'ecdsa-sha2-nistp256': 3,
115 
+        b'ecdsa-sha2-nistp384': 3,
116 
+        b'ecdsa-sha2-nistp521': 3,
117 
+    }
118 
+    for _ in range(num_mpints[key_type]):
119 
+        _, remainder = unstring_prefix(remainder)
120 
+    # Skip comment.
121 
+    _comment, remainder = unstring_prefix(remainder)
122 
+    new_private_block = bytearray(private_block).removesuffix(remainder)
123 
+    padding = bytearray(remainder)
124 
+
125 
+    expected_padding = bytearray()
126 
+    for i in range(1, len(padding) + 1):
127 
+        expected_padding.append(i & 0xFF)
128 
+    assert padding == expected_padding
129 
+    while len(padding) >= 8:
130 
+        padding[-8:] = b''
131 
+
132 
+    new_private_block.extend(padding)
133 
+    result.extend(string(new_private_block))
134 
+    return result
135 
+
136 
+
137 
+class Parametrize:
138 
+    """Common test parametrizations."""
139 
+
140 
+    TEST_KEYS = pytest.mark.parametrize(
141 
+        ['keyname', 'key'], tests.ALL_KEYS.items(), ids=tests.ALL_KEYS.keys()
142 
+    )
143 
+
144 
+
145 
+@Parametrize.TEST_KEYS
146 
+def test_100_test_keys_public_keys_are_internally_consistent(
147 
+    keyname: str,
148 
+    key: tests.SSHTestKey,
149 
+) -> None:
150 
+    """The test key public key data structures are internally consistent."""
151 
+    del keyname
152 
+    string = ssh_agent.SSHAgentClient.string
153 
+    public_key_lines = key.public_key.splitlines(keepends=False)
154 
+    assert len(public_key_lines) == 1
155 
+    line_parts = public_key_lines[0].strip(b'\r\n').split(None, 2)
156 
+    key_type_name, public_key_b64 = line_parts[:2]
157 
+    assert base64.standard_b64encode(key.public_key_data) == public_key_b64
158 
+    assert key.public_key_data.startswith(string(key_type_name))
159 
+
160 
+
161 
+@Parametrize.TEST_KEYS
162 
+def test_101_test_keys_private_keys_are_consistent_with_public_keys(
163 
+    keyname: str,
164 
+    key: tests.SSHTestKey,
165 
+) -> None:
166 
+    """The test key private key data are consistent with their public parts."""
167 
+    del keyname
168 
+    string = ssh_agent.SSHAgentClient.string
169 
+
170 
+    if key.public_key_data.startswith(string(b'ssh-rsa')):
171 
+        # RSA public keys are *not* prefixes of the corresponding private
172 
+        # key in OpenSSH format! RSA public keys consist of an exponent
173 
+        # e and a modulus n, which in the public key are in the order (e,
174 
+        # n), but in the order (n, e) in the OpenSSH private key.  We thus
175 
+        # need to parse and rearrange the components of the public key into
176 
+        # a new "mangled" public key that then *is* a prefix of the
177 
+        # respective private key.
178 
+        unstring_prefix = ssh_agent.SSHAgentClient.unstring_prefix
179 
+        key_type, numbers = unstring_prefix(key.public_key_data)
180 
+        e, encoded_n = unstring_prefix(numbers)
181 
+        n, trailer = unstring_prefix(encoded_n)
182 
+        assert not trailer
183 
+        mangled_public_key_data = string(key_type) + string(n) + string(e)
184 
+        assert (
185 
+            key.private_key_blob[: len(mangled_public_key_data)]
186 
+            == mangled_public_key_data
187 
+        )
188 
+    else:
189 
+        assert (
190 
+            key.private_key_blob[: len(key.public_key_data)]
191 
+            == key.public_key_data
192 
+        )
193 
+
194 
+
195 
+@Parametrize.TEST_KEYS
196 
+def test_102_test_keys_private_keys_are_internally_consistent(
197 
+    keyname: str,
198 
+    key: tests.SSHTestKey,
199 
+) -> None:
200 
+    """The test key private key data structures are internally consistent."""
201 
+    del keyname
202 
+    string = ssh_agent.SSHAgentClient.string
203 
+
204 
+    private_key_lines = [
205 
+        line
206 
+        for line in key.private_key.splitlines(keepends=False)
207 
+        if line and not line.startswith((b'-----BEGIN', b'-----END'))
208 
+    ]
209 
+    private_key_from_openssh = base64.standard_b64decode(
210 
+        b''.join(private_key_lines)
211 
+    )
212 
+    wrapped_public_key = string(key.public_key_data)
213 
+    assert (
214 
+        private_key_from_openssh[
215 
+            len(OPENSSH_HEADER) : len(OPENSSH_HEADER) + len(wrapped_public_key)
216 
+        ]
217 
+        == wrapped_public_key
218 
+    )
219 
+
220 
+    # Offset skips the header, the wrapped public key, and the framing
221 
+    # of the private keys section.
222 
+    offset = len(OPENSSH_HEADER) + len(wrapped_public_key) + 4
223 
+    checkint = int.from_bytes(
224 
+        private_key_from_openssh[offset : offset + 4], 'big'
225 
+    )
226 
+    assert minimize_openssh_keyfile_padding(
227 
+        private_key_from_openssh
228 
+    ) == minimize_openssh_keyfile_padding(
229 
+        as_openssh_keyfile_payload(
230 
+            public_key=key.public_key_data,
231 
+            private_key=key.private_key_blob,
232 
+            checkint=checkint,
233 
+        )
234 
+    )
0 235