Bug 2746 - RFE: Allow to disable SHA1 signatures for RSA
Summary: RFE: Allow to disable SHA1 signatures for RSA
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: ssh (show other bugs)
Version: 7.5p1
Hardware: Other Linux
: P5 enhancement
Assignee: Damien Miller
URL:
Keywords:
Depends on:
Blocks: V_7_8 V_8_0
  Show dependency treegraph
 
Reported: 2017-07-21 22:34 AEST by Jakub Jelen
Modified: 2021-04-23 15:10 AEST (History)
3 users (show)

See Also:


Attachments
allow to disable sha1 also in the server configuration (4.54 KB, patch)
2018-11-13 01:31 AEDT, Jakub Jelen
no flags Details | Diff
Test only base key type in monitor (1.72 KB, patch)
2018-11-13 12:42 AEDT, Damien Miller
dtucker: ok+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Jelen 2017-07-21 22:34:42 AEST
Based on the discussion in the bug #2680, it looks like it is already time to consider support disabling SHA1 for RSA signatures.

Additionally, the extension is negotiating ssh-dss algorithms, which officially deprecated. It does not matter for OpenSSH (which is ignoring all the non-extension algorithms), but it can confuse other peers.
Comment 1 Damien Miller 2018-05-25 13:32:16 AEST
There's a fix for this included in the patch at https://bugzilla.mindrot.org/show_bug.cgi?id=2799
Comment 2 Damien Miller 2018-07-04 23:57:01 AEST
This was fixed by the following commits and will be in OpenSSH 7.8:

commit 314908f451e6b2d4ccf6212ad246fa4619c721d3
Author: djm@openbsd.org <djm@openbsd.org>
Date:   Wed Jul 4 13:51:45 2018 +0000

    upstream: deal with API rename: match_filter_list() =>
    
    match_filter_blacklist()
    
    OpenBSD-Regress-ID: 2da342be913efeb51806351af906fab01ba4367f

commit 89f54cdf6b9cf1cf5528fd33897f1443913ddfb4
Author: djm@openbsd.org <djm@openbsd.org>
Date:   Wed Jul 4 13:51:12 2018 +0000

    upstream: exercise new expansion behaviour of
    
    PubkeyAcceptedKeyTypes and, by proxy, test kex_assemble_names()
    
    ok markus@
    
    OpenBSD-Regress-ID: 292978902e14d5729aa87e492dd166c842f72736

commit 312d2f2861a2598ed08587cb6c45c0e98a85408f
Author: djm@openbsd.org <djm@openbsd.org>
Date:   Wed Jul 4 13:49:31 2018 +0000

    upstream: repair PubkeyAcceptedKeyTypes (and friends) after RSA
    
    signature work - returns ability to add/remove/specify algorithms by
    wildcard.
    
    Algorithm lists are now fully expanded when the server/client configs
    are finalised, so errors are reported early and the config dumps
    (e.g. "ssh -G ...") now list the actual algorithms selected.
    
    Clarify that, while wildcards are accepted in algorithm lists, they
    aren't full pattern-lists that support negation.
    
    (lots of) feedback, ok markus@
    
    OpenBSD-Commit-ID: a8894c5c81f399a002f02ff4fe6b4fa46b1f3207

commit 303af5803bd74bf05d375c04e1a83b40c30b2be5
Author: djm@openbsd.org <djm@openbsd.org>
Date:   Tue Jul 3 11:43:49 2018 +0000

    upstream: some magic for RSA-SHA2 checks
    
    OpenBSD-Regress-ID: e5a9b11368ff6d86e7b25ad10ebe43359b471cd4

commit 7d68e262944c1fff1574600fe0e5e92ec8b398f5
Author: Damien Miller <djm@mindrot.org>
Date:   Tue Jul 3 23:27:11 2018 +1000

    depend

commit b4d4eda633af433d20232cbf7e855ceac8b83fe5
Author: djm@openbsd.org <djm@openbsd.org>
Date:   Tue Jul 3 13:20:25 2018 +0000

    upstream: some finesse to fix RSA-SHA2 certificate authentication
    
    for certs hosted in ssh-agent
    
    OpenBSD-Commit-ID: e5fd5edd726137dda2d020e1cdebc464110a010f

commit d78b75df4a57e0f92295f24298e5f2930e71c172
Author: djm@openbsd.org <djm@openbsd.org>
Date:   Tue Jul 3 13:07:58 2018 +0000

    upstream: check correct variable; unbreak agent keys
    
    OpenBSD-Commit-ID: c36981fdf1f3ce04966d3310826a3e1e6233d93e

commit 2f30300c5e15929d0e34013f38d73e857f445e12
Author: djm@openbsd.org <djm@openbsd.org>
Date:   Tue Jul 3 11:42:12 2018 +0000

    upstream: crank version number to 7.8; needed for new compat flag
    
    for prior version; part of RSA-SHA2 strictification, ok markus@
    
    OpenBSD-Commit-ID: 84a11fc0efd2674c050712336b5093f5d408e32b

commit 4ba0d54794814ec0de1ec87987d0c3b89379b436
Author: djm@openbsd.org <djm@openbsd.org>
Date:   Tue Jul 3 11:39:54 2018 +0000

    upstream: Improve strictness and control over RSA-SHA2 signature
    
    In ssh, when an agent fails to return a RSA-SHA2 signature when
    requested and falls back to RSA-SHA1 instead, retry the signature to
    ensure that the public key algorithm sent in the SSH_MSG_USERAUTH
    matches the one in the signature itself.
    
    In sshd, strictly enforce that the public key algorithm sent in the
    SSH_MSG_USERAUTH message matches what appears in the signature.
    
    Make the sshd_config PubkeyAcceptedKeyTypes and
    HostbasedAcceptedKeyTypes options control accepted signature algorithms
    (previously they selected supported key types). This allows these
    options to ban RSA-SHA1 in favour of RSA-SHA2.
    
    Add new signature algorithms "rsa-sha2-256-cert-v01@openssh.com" and
    "rsa-sha2-512-cert-v01@openssh.com" to force use of RSA-SHA2 signatures
    with certificate keys.
    
    feedback and ok markus@
    
    OpenBSD-Commit-ID: c6e9f6d45eed8962ad502d315d7eaef32c419dde
Comment 3 Jakub Jelen 2018-11-13 00:54:50 AEDT
I do not think this has been completely resolved with OpenSSH 7.8 nor 7.9. Currently, specifying PubkeyAcceptedKeyTypes=rsa-sha2-256,rsa-sha2-512 in sshd_config fails the RSA authentication (actually already the key check without signature) without any helpful error message:

debug1: userauth-request for user root service ssh-connection method publickey [preauth]
debug1: attempt 1 failures 0 [preauth]
debug2: input_userauth_request: try method publickey [preauth]
debug1: userauth_pubkey: test pkalg rsa-sha2-256 pkblob RSA SHA256:33ZvPdZiDGSXPKTytIAkeLQqsfe9pWbcKjo+73WIQMY [preauth]
debug3: mm_key_allowed entering [preauth]
debug3: mm_request_send entering: type 22 [preauth]
debug3: mm_key_allowed: waiting for MONITOR_ANS_KEYALLOWED [preauth]
debug3: mm_request_receive_expect entering: type 23 [preauth]
debug3: mm_request_receive entering [preauth]
debug3: mm_request_receive entering
debug3: monitor_read: checking request 22
debug3: mm_answer_keyallowed entering
debug3: mm_answer_keyallowed: key_from_blob: 0x560e771b2c00
debug3: mm_answer_keyallowed: publickey authentication test: RSA key is not allowed
Failed publickey for root from ::1 port 52866 ssh2: RSA SHA256:33ZvPdZiDGSXPKTytIAkeLQqsfe9pWbcKjo+73WIQMY
debug3: mm_request_send entering: type 23
debug2: userauth_pubkey: authenticated 0 pkalg rsa-sha2-256 [preauth]

This is because the mm_answer_keyallowed() still checks against the raw list provided in the options.pubkey_key_types:

                         if (match_pattern_list(sshkey_ssh_name(key),
                             options.pubkey_key_types, 0) != 1)

rather than using something similar that is correctly used in the client (sshconnnec2.c) in 4ba0d5479 (key_type_allowed_by_config), taking the SHA2 extension into the account.

I am sorry that I did not manage to test that earlier.
Comment 4 Jakub Jelen 2018-11-13 01:31:07 AEDT
Created attachment 3202 [details]
allow to disable sha1 also in the server configuration

Attaching my fast-baked patch, which I verified that solves this problem for me.
Comment 5 Jakub Jelen 2018-11-13 01:35:13 AEDT
The header file should indeed list sshkey_type_allowed_by_config() instead of key_type_allowed_by_config()
Comment 6 Damien Miller 2018-11-13 12:42:08 AEDT
Created attachment 3203 [details]
Test only base key type in monitor

I think this version is a little simpler
Comment 7 Darren Tucker 2018-11-13 15:19:15 AEDT
Comment on attachment 3203 [details]
Test only base key type in monitor

not sure it's something I'd want to see at debug level 1, but otherwise ok.
Comment 8 Jakub Jelen 2018-11-13 19:41:13 AEDT
Thank you for the reworked patch. I think the debug level 1 is fine here since it is the only note why the public key test was rejected in this case. I would be for mentioning also the PubkeyAcceptedKeyTypes option (maybe rather than listing the whole list?) so it can simply point the administrator to the configuration issue.
Comment 9 Tomas Mraz 2018-11-15 23:32:01 AEDT
There is a typo mismastches -> mismatches
Comment 10 Damien Miller 2018-11-16 13:43:59 AEDT
committed

(In reply to Tomas Mraz from comment #9)
> There is a typo mismastches -> mismatches

fixed

(In reply to Darren Tucker from comment #7)
> not sure it's something I'd want to see at debug level 1, but
> otherwise ok.

I've made it an error()

Think this is a candidate for V_7_9?
Comment 11 Damien Miller 2018-11-16 13:49:06 AEDT
I've committed this to master and will cherry-pick it to the V_7_9 branch.
Comment 12 Damien Miller 2021-04-23 15:10:42 AEST
closing resolved bugs as of 8.6p1 release
Comment 13 Damien Miller 2021-04-23 15:10:43 AEST
closing resolved bugs as of 8.6p1 release