Bug 3322 - Verify both SHA1 and SHA256 SSHFP records when both are present
Summary: Verify both SHA1 and SHA256 SSHFP records when both are present
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: ssh (show other bugs)
Version: 8.6p1
Hardware: Other Linux
: P5 enhancement
Assignee: Assigned to nobody
URL:
Keywords:
Depends on:
Blocks: V_8_7
  Show dependency treegraph
 
Reported: 2021-06-19 02:12 AEST by Dmitry Belyavskiy
Modified: 2022-02-25 13:57 AEDT (History)
2 users (show)

See Also:


Attachments
Simplify verify_host_key_dns() and verify all fingerprints (4.41 KB, patch)
2021-07-16 22:52 AEST, Darren Tucker
djm: ok+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Belyavskiy 2021-06-19 02:12:18 AEST
OpenSSH uses SHA1 as a default digest for SSHFP records for RSA/DSA algorithms.

RFC 6594 permits using much more secure SHA256 algorithm with SSHFP records. SHA256 is already default digest for Ed25519 and ECDSA SSHFP records.

The straightforward PR:

https://github.com/openssh/openssh-portable/pull/259
Comment 1 Darren Tucker 2021-07-16 22:16:50 AEST
I commented on the pull request too, but I don't think your change actually does anything.   While iterating the rrset, the existing code uses the digest type from the DNS record:

        if (hostkey_digest_type != dnskey_digest_type) {
                hostkey_digest_type = dnskey_digest_type;
                free(hostkey_digest);

                /* Initialize host key parameters */
                if (!dns_read_key(&hostkey_algorithm,
                    &hostkey_digest_type, &hostkey_digest,
                    &hostkey_digest_len, hostkey)) {

If we add a couple of debug calls to the current code you can see it verifies both fingerprint types (this machine has SHA1 and SHA256 RSA fingerprints):

$ ./ssh -vvv -o verifyhostkeydns=ask -o hostkeyalgorithms=rsa-sha2-256 fw 2>&1 | grep -i dns
debug3: verify_host_key_dns
debug1: found 4 insecure fingerprints in DNS
debug3: verify_host_key_dns: checking SSHFP type 4 fptype 1
debug3: verify_host_key_dns: checking SSHFP type 1 fptype 1
debug1: verify_host_key_dns: matched SSHFP type 1 fptype 1
debug3: verify_host_key_dns: checking SSHFP type 3 fptype 2
debug3: verify_host_key_dns: checking SSHFP type 1 fptype 2
debug1: verify_host_key_dns: matched SSHFP type 1 fptype 2

It'll return success if either validate, though, which is probably not ideal.  It should probably ensure that all fingerprints match.
Comment 2 Darren Tucker 2021-07-16 22:52:09 AEST
Created attachment 3539 [details]
Simplify verify_host_key_dns() and verify all fingerprints

I think this is what it should do: verify all fingerprint types present in DNS.  If any fail to verify the overall check fails.
Comment 3 Dmitry Belyavskiy 2021-07-16 22:57:26 AEST
Yes, it's a proper solution for the verification. I'm more disturbed about creating the new records - I got a (possible wrong) impression that the default value is used on creation.
Comment 4 Darren Tucker 2021-07-16 23:28:18 AEST
> I got a (possible wrong) impression that the default value is used on creation.

Creation of the SSHFP records?  It iterates over the available digest types in export_dns_rr():

  for (dtype = SSHFP_HASH_SHA1; dtype < SSHFP_HASH_MAX; dtype++) {
      rdata_digest_type = dtype;
      if (dns_read_key(&rdata_pubkey_algorithm, &rdata_digest_type,
           &rdata_digest, &rdata_digest_len, key)) {

$ ./ssh-keygen -r fw
fw IN SSHFP 1 1 [...]
fw IN SSHFP 1 2 [...]
fw IN SSHFP 2 1 [...]
fw IN SSHFP 2 2 [...]
fw IN SSHFP 3 1 [...]
fw IN SSHFP 3 2 [...]
fw IN SSHFP 4 1 [...]
fw IN SSHFP 4 2 [...]
Comment 5 Darren Tucker 2021-07-19 14:20:50 AEST
The patch has been committed and
Comment 6 Darren Tucker 2021-07-19 14:21:19 AEST
... will be in the next major release.  Thanks for the report.
Comment 7 Damien Miller 2022-02-25 13:57:57 AEDT
closing bugs resolved before openssh-8.9