Re: Bugs when using rsa-sha2 (+patches)

[Jakub Jelen <jjelen@xxxxxxxxxx>]

On Thu, 2018-11-15 at 17:51 +0100, Tilo Eckert wrote:
> Hello Jakub
> 
> > With your proposed patch, we are basically opening us to the
> > downgrade
> > attack (the client request SHA2 signatures, the server sends the
> > SHA1
> > signature and it is properly verified and accepted by the client),
> > similarly as it worked some time ago in OpenSSH (with client
> > authentication):
> > 
> > https://bugzilla.mindrot.org/show_bug.cgi?id=2799
> 
> I see what you mean. However, having to allow sha-rsa alongside rsa-
> sha2
> isn't any better, except that the downgrade attack is more obvious.
> ;-)

I am not saying it is better. I just pointed out the way how it worked
for a long time.

> If I understand ssh_pki_signature_verify_blob() correctly, the server
> could also perform an upgrade "attack", i.e. send a SHA2 signature
> when
> only ssh-rsa is allowed.

If I see right, the server will not send the SHA2 signatures, when they
will not be allowed by the client. But indeed, if he would, the
verification would succeed. But really, there is nothing like "upgrade
attack", since the downgrade ones make use of some weaknesses of weaker
algorithms. No attacker would like to force you to use something
stronger :)
But anyway, this is indeed bug that should get addressed.

> The function ssh_pki_signature_verify_blob() is used by server
> (messages.c) and client code (packet_cb.c). So, I think one way to
> fix
> it is to remove the ssh_pki_import_signature_blob() call from
> ssh_pki_signature_verify_blob(), move it to the two callers and pass
> sig
> instead of sig_blob to the verify function. That would allow us to
> check
> the signature type in ssh_packet_newkeys() depending on the allowed
> hostkey types.

That sounds like a way to go. The ssh_pki_signature_verify_blob()
unfortunately does not have slightest idea what is the allowed list to
verify against.

We will not have the same problem in the server (messages.c), because
server does not have any configuration which would allow to disable
some authentication algorithms.

> I think there may be another unrelated security issue in
> ssh_pki_signature_verify_blob(): When compiling without ECC support
> and
> the server sends an unexpected ECDSA signature, it would be imported
> successfully, "if (key->type == SSH_KEYTYPE_ECDSA)" would be true,
> but
> the block does nothing. The function returns rc, which is still 0
> from
> the import. Maybe I missed something, but this looks to me like a
> signature verification bypass. We should return SSH_ERROR if ECC is
> unsupported.

I think you are right here. Setting the rc = SSH_ERROR in the else
branch should fix this problem, but there might be more issues like
this throughout the code.

Would you like to submit a patch for these two issues?

Thanks,
-- 
Jakub Jelen
Software Engineer
Security Technologies
Red Hat, Inc.