openssh bugs - Nov 2017 - [Bug 2799] New: RSA Signatures using SHA2 provided by different ssh-agent are not properly verified

https://bugzilla.mindrot.org/show_bug.cgi?id=2799

            Bug ID: 2799
           Summary: RSA Signatures using SHA2 provided by different
                    ssh-agent are not properly verified
           Product: Portable OpenSSH
           Version: 7.6p1
          Hardware: Other
                OS: Linux
            Status: NEW
          Severity: enhancement
          Priority: P5
         Component: ssh
          Assignee: unassigned-bugs at mindrot.org
          Reporter: jjelen at redhat.com

Problem:

The SSH2 protocol has the keytype/signature algorithm written in two
places that are not checked against each other, when the signature
comes from software talking ssh-agent protocol, but ignoring the SHA2
signature flags.
>From RFC4252, Section 7 [1], the message SSH_MSG_USERAUTH_REQUEST has a
field "public key algorithm name", which is what is reported in all
the
logs as used.

RFC4253, Section 6.6 [2] talks about the format of signature, which is
again the "signature format identifier" and then the signature blob.

Steps to reproduce:

1. Apply the patch [3] to the server
2. Try to connect to this server with a signature provided by either
old ssh-agent (before openssh-7.2), gnome-keyring or pageant.

Current result:
Server debug logs contain, where hash_alg=1 is SSH_DIGEST_SHA1 in
master:

debug1: Verifying signature with ktype=ssh-rsa and hash_alg=1
debug2: userauth_pubkey: authenticated 1 pkalg rsa-sha2-512

All the other logs talk about SHA2 signature.

Expected result:
Either failure because of inconsistent signature or client adjusting
the signature algorithm and honestly logging ssh-rsa algorithm instead
the SHA2 one.

Filled based on my longer report on the mailing list yesterday [4] with
more possible options how to resolve this issue.

Feel free to ask if you would need some clarification.

[1] https://tools.ietf.org/html/rfc4252#section-7
[2] https://tools.ietf.org/html/rfc4253#section-6.6
[3] https://gist.github.com/Jakuje/b1f7161d89472c4b6a3e2024675b0b46
[4]
https://lists.mindrot.org/pipermail/openssh-unix-dev/2017-November/036443.html

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2799

NUXI <nuxi at vault24.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nuxi at vault24.org

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2799

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned-bugs at mindrot.org |djm at mindrot.org
                 CC|                            |djm at mindrot.org

--- Comment #1 from Damien Miller <djm at mindrot.org> ---
Created attachment 3090
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3090&action=edit
Check signatures received from agent

This checks that the signature that we receive back from the agent
matches what was requested of it. It's not particularly graceful, but
it's an improvement.

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2799

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2782


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2782
[Bug 2782] Tracking bug for OpenSSH 7.7 release
-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2799

--- Comment #2 from Jakub Jelen <jjelen at redhat.com> ---
Created attachment 3092
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3092&action=edit
Check signature algorithm while verifying RSA signatures

Thank you for having a look into that. This is certainly an improvement
and client is doing what it is expected to do now.

I believe similar check should also come to the rsa signature
verification, which currently uses only the insides of signature, which
is wrong in case of other algorithm is negotiated in upper level (as in
authentication). Something as I just added as an attachment should do
the job.


After building your patch, I am getting missing symbols:

./libssh.a(authfd.o): In function `ssh_agent_sign':
/home/jjelen/devel/openssh-portable/authfd.c:406: undefined reference
to `freezero'
/home/jjelen/devel/openssh-portable/authfd.c:395: undefined reference
to `freezero'
collect2: error: ld returned 1 exit status
make: *** [Makefile:165: ssh] Error 1

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2799

Jakub Jelen <jjelen at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3092|0                           |1
        is obsolete|                            |

--- Comment #3 from Jakub Jelen <jjelen at redhat.com> ---
Created attachment 3100
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3100&action=edit
Check signature algorithm while verifying RSA signatures

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2799

--- Comment #4 from Damien Miller <djm at mindrot.org> ---
I don't think the "Check signature algorithm while verifying RSA
signatures" patch is correct: key types and signature types are allowed
to be different, and the patch doesn't actually supply the signature
type in many cases where we could (esp. KEX).

I'll have a look at this now.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2799

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3100|0                           |1
        is obsolete|                            |

--- Comment #5 from Damien Miller <djm at mindrot.org> ---
Created attachment 3104
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3104&action=edit
check signature type at verification time

this checks signature types for RSA non-certificate keys, including in
KEX.

I'm not sure how best to deal with certificate types. E.g. a
ssh-rsa-cert-v01 at openssh.com key could yield any signature type. I
guess I could add explicit ssh-rsa-shaXXX-cert-v01 at openssh.com types.

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2799

--- Comment #6 from Jakub Jelen <jjelen at redhat.com> ---
Thank you for the updating the patch on the rough edges. Yes, I did not
think about certificates. It would certainly make sense to have
certificates that are also enforcing SHA2 these days. I don't think,
there is other way than defining new ones, such as
ssh-rsa-shaXXX-cert-v01 at openssh.com

The changes look good to me.

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2799

--- Comment #7 from Jakub Jelen <jjelen at redhat.com> ---
FYI, the Bitwise SSH server checks the signatures provided by the agent
correctly and here is one of the demonstrations where it results in
hard-to-debug issue for users using the following setup:

  gnome-keyring -> OpenSSH client -> Bitwise SSH server

https://unix.stackexchange.com/a/415574/121504

Is there any progress or plans?

I already fixed gnome-keyring to provide correct signatures and it
should be available in next release:

https://bugzilla.gnome.org/show_bug.cgi?id=790910

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2799

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3090|0                           |1
        is obsolete|                            |
   Attachment #3104|0                           |1
        is obsolete|                            |

--- Comment #8 from Damien Miller <djm at mindrot.org> ---
Created attachment 3135
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3135&action=edit
Stricter RSA key type checking

This diff does a few things that aren't easily separable into
individual diffs.

1. Makes ssh retry to the sign_and_send_pubkey() operation when
ssh-agent returns a signature with an incorrect type. This ensures that
the pktype in the USERAUTH_REQUEST matches that of the signature.

2. Makes PubkeyAcceptedKeyTypes and HostbasedAcceptedKeyTypes match the
pktype in USERAUTH_REQUEST rather than the type of the embedded key.
This allows these options to be effectively used to ban ssh-rsa but
leave rsa-sha2-* enabled.

3. Add new RSA certificate types that that can be used in the above
options and on the wire to require the use of RSA/SHA2 signatures.

4. More strictly check the pkalg field from USERAUTH_REQUEST packets
against the type in the signature.

5. Because current OpenSSH is lax wrt RSA signature type correctness in
the presence of agents that don't support the new signature types, add
a compat flag to relax some of the new strictness.

Unfortunately, this isn't likely to make the 7.7 release :(

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2799

--- Comment #9 from Jakub Jelen <jjelen at redhat.com> ---
Thank you for having a look into that and working on this patch. All
the features you mention would be very desirable.

FYI, the gnome-keyring developer dropped its ssh-agent implementation
and instead wrap standard ssh-agent [1] to enhance the interface with
their functionality.

I also tried to contact the PuTTY/Pageant developers about this issue,
but without any success. Are there any other specific agents, that are
causing problems with SHA2 signatures?

Some comments to the patch:

+               /*
+                * PKCS#11 tokens may not support all signature
algorithms,
+                * so check what we get back.
+                */

I don't think this should be a big problem. The PKCS#11 module gets
just a hash that it is supposed to sign with RSA PKCS#1.5 mechanism.
The hashing is done already by the ssh and you have complete control of
this. The only thing that happens sometimes is that the tokens use some
logic to make sure the passed value is a hash and not arbitrary data
(assuming based on the length?). I saw this behavior with YubiHSM. I
believe this is the only case when it might fail (if token does not
know SHA2 sizes?) and where the usage of other hash might help.

Otherwise the patch looks reasonable from my read-through. 

[1] https://bugzilla.gnome.org/show_bug.cgi?id=775981

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2799

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2852


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2852
[Bug 2852] Tracking bug for OpenSSH 7.8 release
-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2799

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|2782                        |

--- Comment #10 from Damien Miller <djm at mindrot.org> ---
Move to OpenSSH 7.8 tracking bug


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2782
[Bug 2782] Tracking bug for OpenSSH 7.7 release
-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2799

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #11 from Damien Miller <djm at mindrot.org> ---
This was fixed by the following commits and will be in OpenSSH 7.8:

commit 314908f451e6b2d4ccf6212ad246fa4619c721d3
Author: djm at openbsd.org <djm at 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 at openbsd.org <djm at 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 at openbsd.org <djm at 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 at openbsd.org <djm at 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 at mindrot.org>
Date:   Tue Jul 3 23:27:11 2018 +1000

    depend

commit b4d4eda633af433d20232cbf7e855ceac8b83fe5
Author: djm at openbsd.org <djm at 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 at openbsd.org <djm at 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 at openbsd.org <djm at 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 at openbsd.org <djm at 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 at
openssh.com"
and
    "rsa-sha2-512-cert-v01 at openssh.com" to force use of RSA-SHA2
signatures
    with certificate keys.

    feedback and ok markus@

    OpenBSD-Commit-ID: c6e9f6d45eed8962ad502d315d7eaef32c419dde

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2799

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #12 from Damien Miller <djm at mindrot.org> ---
closing resolved bugs as of 8.6p1 release

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.