Refactor the testing machinery tests
Marco Ricci

Marco Ricci commited on 2025-08-22 19:50:46
Zeige 1 geänderte Dateien mit 28 Einfügungen und 21 Löschungen.


(This is part 6 of a series of refactorings for the test suite.)

Assign all tests to a common class, then elide common parts in the test
names and docstrings.

Also factor out the OpenSSH cipher blocksize for the "none" cipher into
a separate constant, and use that constant in lieu of magic numbers
throughout the helper functions.  We correct one instance of where
double the block size is used, which (if memory serves correctly) was
due to a stylistic difference between OpenSSH- and Pageant-generated
keys, but both conforming to the specification.  These differences are
removed in the canonicalization step anyway, but the double-sized block
size is probably confusing to read if you don't already anticipate the
canonicalization step.
... ...
@@ -19,6 +19,7 @@ OPENSSH_HEADER = (
19 19
     + b"\x00\x00\x00\x00"  # kdfoptions
20 20
     + b"\x00\x00\x00\x01"  # number of keys
21 21
 )
22
+OPENSSH_NONE_CIPHER_BLOCKSIZE = 8
22 23
 
23 24
 
24 25
 def as_openssh_keyfile_payload(
... ...
@@ -59,7 +60,7 @@ def as_openssh_keyfile_payload(
59 60
     secret.extend(uint32(checkint))  # checkint
60 61
     secret.extend(private_key)  # privatekey1 and comment1
61 62
     i = 1
62
-    while len(secret) % 16 != 0:
63
+    while len(secret) % OPENSSH_NONE_CIPHER_BLOCKSIZE != 0:
63 64
         secret.append(i)
64 65
         i += 1
65 66
     payload.extend(string(secret))  # encrypted, padded list of private keys
... ...
@@ -126,8 +127,8 @@ def minimize_openssh_keyfile_padding(
126 127
     for i in range(1, len(padding) + 1):
127 128
         expected_padding.append(i & 0xFF)
128 129
     assert padding == expected_padding
129
-    while len(padding) >= 8:
130
-        padding[-8:] = b""
130
+    while len(padding) >= OPENSSH_NONE_CIPHER_BLOCKSIZE:
131
+        padding[-OPENSSH_NONE_CIPHER_BLOCKSIZE:] = b""
131 132
 
132 133
     new_private_block.extend(padding)
133 134
     result.extend(string(new_private_block))
... ...
@@ -144,12 +145,16 @@ class Parametrize:
144 145
     )
145 146
 
146 147
 
148
+class TestTestKeys:
149
+    """Tests testing the test keys."""
150
+
147 151
     @Parametrize.TEST_KEYS
148
-def test_test_keys_public_keys_are_internally_consistent(
152
+    def test_public_keys_are_internally_consistent(
153
+        self,
149 154
         keyname: str,
150 155
         key: data.SSHTestKey,
151 156
     ) -> None:
152
-    """The test key public key data structures are internally consistent."""
157
+        """The public key data structures are internally consistent."""
153 158
         del keyname
154 159
         string = ssh_agent.SSHAgentClient.string
155 160
         public_key_lines = key.public_key.splitlines(keepends=False)
... ...
@@ -159,24 +164,25 @@ def test_test_keys_public_keys_are_internally_consistent(
159 164
         assert base64.standard_b64encode(key.public_key_data) == public_key_b64
160 165
         assert key.public_key_data.startswith(string(key_type_name))
161 166
 
162
-
163 167
     @Parametrize.TEST_KEYS
164
-def test_test_keys_private_keys_are_consistent_with_public_keys(
168
+    def test_private_keys_are_consistent_with_public_keys(
169
+        self,
165 170
         keyname: str,
166 171
         key: data.SSHTestKey,
167 172
     ) -> None:
168
-    """The test key private key data are consistent with their public parts."""
173
+        """The private key data are consistent with their public parts."""
169 174
         del keyname
170 175
         string = ssh_agent.SSHAgentClient.string
171 176
 
172 177
         if key.public_key_data.startswith(string(b"ssh-rsa")):
173
-        # RSA public keys are *not* prefixes of the corresponding private
174
-        # key in OpenSSH format! RSA public keys consist of an exponent
175
-        # e and a modulus n, which in the public key are in the order (e,
176
-        # n), but in the order (n, e) in the OpenSSH private key.  We thus
177
-        # need to parse and rearrange the components of the public key into
178
-        # a new "mangled" public key that then *is* a prefix of the
179
-        # respective private key.
178
+            # RSA public keys are *not* prefixes of the corresponding
179
+            # private key in OpenSSH format! RSA public keys consist of
180
+            # an exponent e and a modulus n, which in the public key are
181
+            # in the order (e, n), but in the order (n, e) in the
182
+            # OpenSSH private key.  We thus need to parse and rearrange
183
+            # the components of the public key into a new "mangled"
184
+            # public key that then *is* a prefix of the respective
185
+            # private key.
180 186
             unstring_prefix = ssh_agent.SSHAgentClient.unstring_prefix
181 187
             key_type, numbers = unstring_prefix(key.public_key_data)
182 188
             e, encoded_n = unstring_prefix(numbers)
... ...
@@ -193,13 +199,13 @@ def test_test_keys_private_keys_are_consistent_with_public_keys(
193 199
                 == key.public_key_data
194 200
             )
195 201
 
196
-
197 202
     @Parametrize.TEST_KEYS
198
-def test_test_keys_private_keys_are_internally_consistent(
203
+    def test_private_keys_are_internally_consistent(
204
+        self,
199 205
         keyname: str,
200 206
         key: data.SSHTestKey,
201 207
     ) -> None:
202
-    """The test key private key data structures are internally consistent."""
208
+        """The private key data structures are internally consistent."""
203 209
         del keyname
204 210
         string = ssh_agent.SSHAgentClient.string
205 211
 
... ...
@@ -214,13 +220,14 @@ def test_test_keys_private_keys_are_internally_consistent(
214 220
         wrapped_public_key = string(key.public_key_data)
215 221
         assert (
216 222
             private_key_from_openssh[
217
-            len(OPENSSH_HEADER) : len(OPENSSH_HEADER) + len(wrapped_public_key)
223
+                len(OPENSSH_HEADER) : len(OPENSSH_HEADER)
224
+                + len(wrapped_public_key)
218 225
             ]
219 226
             == wrapped_public_key
220 227
         )
221 228
 
222
-    # Offset skips the header, the wrapped public key, and the framing
223
-    # of the private keys section.
229
+        # Offset skips the header, the wrapped public key, and the
230
+        # framing of the private keys section.
224 231
         offset = len(OPENSSH_HEADER) + len(wrapped_public_key) + 4
225 232
         checkint = int.from_bytes(
226 233
             private_key_from_openssh[offset : offset + 4], "big"
227 234