Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable HostKeyTest to extract ECDSA and DSA keys #286

Conversation

dlenskiSB
Copy link
Contributor

Their certificate-embedded counterparts are enabled as well.

As with RSA, it is possible for DSA keys to be of variable length (not just 1024 bits), so I've added {'variable_key_len': True} to the relevant HOST_KEY_TYPES entries, although this key-value pair is otherwise unused.

Their certificate-embedded counterparts are enabled as well.

As with RSA, it *is* possible for DSA keys to be of variable length (not
just 1024 bits), so I've added `{'variable_key_len': True}` to the relevant
`HOST_KEY_TYPES` entries, although this key-value pair is otherwise unused.
@dlenskiSB
Copy link
Contributor Author

The way that HostKeyTest is written currently, it will only test for key types in its own internal allowlist (HostKeyTest.HOST_KEY_TYPES):

# For each host key type...
for host_key_type in host_key_types:
key_fail_comments = []
key_warn_comments = []
# Skip those already handled (i.e.: those in the RSA family, as testing one tests them all).
if host_key_type in parsed_host_key_types:
continue
# If this host key type is supported by the server, we test it.
if host_key_type in server_kex.key_algorithms:
out.d('Preparing to obtain ' + host_key_type + ' host key...', write_now=True)

A more universal and future-proof way to handle this would be to simply test for every key type that the server claims to support (for host_key_type in server_kex.key_algorithms:), rather than maintaining this allowlist. The special cases (currently to prevent unnecessary re-querying for repeated RSA keys, could be extended to ECDSA as well) could still be kept in the loop:

# Set the hostkey size for all RSA key types since 'ssh-rsa', 'rsa-sha2-256', etc. are all using the same host key. Note, however, that this may change in the future.
if cert is False and host_key_type in HostKeyTest.RSA_FAMILY:
for rsa_type in HostKeyTest.RSA_FAMILY:
server_kex.set_host_key(rsa_type, raw_hostkey_bytes, hostkey_modulus_size, ca_key_type, ca_modulus_size)

@jtesta would you be receptive to such a change?

@jtesta jtesta merged commit a4b78b7 into jtesta:master Sep 25, 2024
5 checks passed
jtesta added a commit that referenced this pull request Sep 25, 2024
@jtesta
Copy link
Owner

jtesta commented Sep 25, 2024

As with RSA, it is possible for DSA keys to be of variable length (not just 1024 bits)

The DSA algorithm can indeed use variable moduli, though the DSS standard is defined at exactly 1024 bits only. So using, say, a 3072-bit modulus would be possible, but wouldn't be considered DSS anymore. That said, I've seen a non-standard algorithm out in the wild called dsa2048...

A more universal and future-proof way to handle this would be to [...] @jtesta would you be receptive to such a change?

Sure.

Thanks for this PR!

@perkelix
Copy link

Btw, aren't DSA keys currently discouraged?

@dlenskiSB
Copy link
Contributor Author

@perkelix wrote:

Btw, aren't DSA keys currently discouraged?

Yes, indeed. Which is exactly why a tool for finding and cataloging them should be able to extract them.

@dlenskiSB dlenskiSB deleted the enable_HostKeyTest_to_extract_ECDSA_and_DSA_keys branch October 2, 2024 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants