Avoid crashing when overriding key selection on the command-line
Marco Ricci commited on 2024-07-28 17:12:05
Zeige 4 geänderte Dateien mit 71 Einfügungen und 9 Löschungen.
A control-flow error resulted in there being a code path where both `phrase` and `key` arguments were passed to the `derivepassphrase.Vault` constructor, which does not take a `key` argument. So, unconditionally discard the `key` argument from the dict of arguments to pass to the `derivepassphrase.Vault` constructor. This also allows streamlining the preceding phrase and key handling logic somewhat. For the tests, add further aliases for the second and third dummy keys, because the comprehensive regression test for this bug needs three different sources of key selection.
- src/derivepassphrase/cli.py ecb1659..c039e46
- tests/__init__.py 5cece68..a13178c
- tests/test_derivepassphrase_cli.py 069efef..3712c41
- towncrier.d/5.fixed.md 0000000..0632378
| ... | ... |
@@ -1068,18 +1068,12 @@ def derivepassphrase( |
| 1068 | 1068 |
# derivepassphrase.Vault.phrase_from_key if a key is |
| 1069 | 1069 |
# given. Finally, if nothing is set, error out. |
| 1070 | 1070 |
if use_key or use_phrase: |
| 1071 |
- if use_key: |
|
| 1072 |
- kwargs['phrase'] = key_to_phrase(key) |
|
| 1073 |
- else: |
|
| 1074 |
- kwargs['phrase'] = phrase |
|
| 1075 |
- kwargs.pop('key', '')
|
|
| 1071 |
+ kwargs['phrase'] = key_to_phrase(key) if use_key else phrase |
|
| 1076 | 1072 |
elif kwargs.get('phrase') and kwargs.get('key'):
|
| 1077 | 1073 |
if any('key' in m for m in settings.maps[:2]):
|
| 1078 |
- kwargs['phrase'] = key_to_phrase(kwargs.pop('key'))
|
|
| 1079 |
- else: |
|
| 1080 |
- kwargs.pop('key')
|
|
| 1074 |
+ kwargs['phrase'] = key_to_phrase(kwargs['key']) |
|
| 1081 | 1075 |
elif kwargs.get('key'):
|
| 1082 |
- kwargs['phrase'] = key_to_phrase(kwargs.pop('key'))
|
|
| 1076 |
+ kwargs['phrase'] = key_to_phrase(kwargs['key']) |
|
| 1083 | 1077 |
elif kwargs.get('phrase'):
|
| 1084 | 1078 |
pass |
| 1085 | 1079 |
else: |
| ... | ... |
@@ -1088,6 +1082,7 @@ def derivepassphrase( |
| 1088 | 1082 |
'or in configuration' |
| 1089 | 1083 |
) |
| 1090 | 1084 |
raise click.UsageError(msg) |
| 1085 |
+ kwargs.pop('key', '')
|
|
| 1091 | 1086 |
vault = dpp.Vault(**kwargs) |
| 1092 | 1087 |
result = vault.generate(service) |
| 1093 | 1088 |
click.echo(result.decode('ASCII'))
|
| ... | ... |
@@ -327,6 +327,8 @@ DUMMY_KEY1 = SUPPORTED_KEYS['ed25519']['public_key_data'] |
| 327 | 327 |
DUMMY_KEY1_B64 = base64.standard_b64encode(DUMMY_KEY1).decode('ASCII')
|
| 328 | 328 |
DUMMY_KEY2 = SUPPORTED_KEYS['rsa']['public_key_data'] |
| 329 | 329 |
DUMMY_KEY2_B64 = base64.standard_b64encode(DUMMY_KEY2).decode('ASCII')
|
| 330 |
+DUMMY_KEY3 = SUPPORTED_KEYS['ed448']['public_key_data'] |
|
| 331 |
+DUMMY_KEY3_B64 = base64.standard_b64encode(DUMMY_KEY3).decode('ASCII')
|
|
| 330 | 332 |
DUMMY_CONFIG_SETTINGS = {
|
| 331 | 333 |
'length': 10, |
| 332 | 334 |
'upper': 1, |
| ... | ... |
@@ -36,6 +36,9 @@ DUMMY_PHRASE_FROM_KEY1 = tests.DUMMY_PHRASE_FROM_KEY1 |
| 36 | 36 |
DUMMY_KEY1 = tests.DUMMY_KEY1 |
| 37 | 37 |
DUMMY_KEY1_B64 = tests.DUMMY_KEY1_B64 |
| 38 | 38 |
DUMMY_KEY2 = tests.DUMMY_KEY2 |
| 39 |
+DUMMY_KEY2_B64 = tests.DUMMY_KEY2_B64 |
|
| 40 |
+DUMMY_KEY3 = tests.DUMMY_KEY3 |
|
| 41 |
+DUMMY_KEY3_B64 = tests.DUMMY_KEY3_B64 |
|
| 39 | 42 |
|
| 40 | 43 |
|
| 41 | 44 |
class IncompatibleConfiguration(NamedTuple): |
| ... | ... |
@@ -358,6 +361,67 @@ class TestCLI: |
| 358 | 361 |
last_line.rstrip(b'\n') == DUMMY_RESULT_KEY1 |
| 359 | 362 |
), 'program generated unexpected result (wrong settings?)' |
| 360 | 363 |
|
| 364 |
+ @tests.skip_if_no_agent |
|
| 365 |
+ @pytest.mark.parametrize( |
|
| 366 |
+ 'config', |
|
| 367 |
+ [ |
|
| 368 |
+ pytest.param( |
|
| 369 |
+ {
|
|
| 370 |
+ 'global': {'key': DUMMY_KEY1_B64},
|
|
| 371 |
+ 'services': {DUMMY_SERVICE: {}},
|
|
| 372 |
+ }, |
|
| 373 |
+ id='global_config', |
|
| 374 |
+ ), |
|
| 375 |
+ pytest.param( |
|
| 376 |
+ {'services': {DUMMY_SERVICE: {'key': DUMMY_KEY2_B64}}},
|
|
| 377 |
+ id='service_config', |
|
| 378 |
+ ), |
|
| 379 |
+ pytest.param( |
|
| 380 |
+ {
|
|
| 381 |
+ 'global': {'key': DUMMY_KEY1_B64},
|
|
| 382 |
+ 'services': {DUMMY_SERVICE: {'key': DUMMY_KEY2_B64}},
|
|
| 383 |
+ }, |
|
| 384 |
+ id='full_config', |
|
| 385 |
+ ), |
|
| 386 |
+ ], |
|
| 387 |
+ ) |
|
| 388 |
+ @pytest.mark.parametrize('key_index', [1, 2, 3], ids=lambda i: f'index{i}')
|
|
| 389 |
+ def test_204c_key_override_on_command_line( |
|
| 390 |
+ self, |
|
| 391 |
+ monkeypatch: Any, |
|
| 392 |
+ config: dict[str, Any], |
|
| 393 |
+ key_index: int, |
|
| 394 |
+ ) -> None: |
|
| 395 |
+ def sign( |
|
| 396 |
+ _, key: bytes | bytearray, message: bytes | bytearray |
|
| 397 |
+ ) -> bytes: |
|
| 398 |
+ del message # Unused. |
|
| 399 |
+ for value in tests.SUPPORTED_KEYS.values(): |
|
| 400 |
+ if value['public_key_data'] == key: |
|
| 401 |
+ assert value['expected_signature'] is not None |
|
| 402 |
+ return value['expected_signature'] |
|
| 403 |
+ raise AssertionError |
|
| 404 |
+ |
|
| 405 |
+ monkeypatch.setattr( |
|
| 406 |
+ ssh_agent_client.SSHAgentClient, 'list_keys', tests.list_keys |
|
| 407 |
+ ) |
|
| 408 |
+ monkeypatch.setattr(ssh_agent_client.SSHAgentClient, 'sign', sign) |
|
| 409 |
+ runner = click.testing.CliRunner(mix_stderr=False) |
|
| 410 |
+ with tests.isolated_config( |
|
| 411 |
+ monkeypatch=monkeypatch, runner=runner, config=config |
|
| 412 |
+ ): |
|
| 413 |
+ result = runner.invoke( |
|
| 414 |
+ cli.derivepassphrase, |
|
| 415 |
+ ['-k', DUMMY_SERVICE], |
|
| 416 |
+ input=f'{key_index}\n'.encode('ASCII'),
|
|
| 417 |
+ ) |
|
| 418 |
+ assert result.stderr_bytes is not None |
|
| 419 |
+ assert ( |
|
| 420 |
+ b'Error:' not in result.stderr_bytes |
|
| 421 |
+ ), 'program exited with failure' |
|
| 422 |
+ assert result.stdout_bytes, 'program output expected' |
|
| 423 |
+ assert result.exit_code == 0, 'program exited with failure' |
|
| 424 |
+ |
|
| 361 | 425 |
def test_205_service_phrase_if_key_in_global_config( |
| 362 | 426 |
self, |
| 363 | 427 |
monkeypatch: Any, |