Bug 2737 - function identity_sign() assume private key's pub part as same as the .pub key.
Summary: function identity_sign() assume private key's pub part as same as the .pub key.
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: ssh (show other bugs)
Version: 7.5p1
Hardware: Other Other
: P5 enhancement
Assignee: Damien Miller
URL:
Keywords:
: 2661 2776 (view as bug list)
Depends on:
Blocks: V_7_6
  Show dependency treegraph
 
Reported: 2017-07-04 16:48 AEST by hujunjie
Modified: 2021-04-23 15:11 AEST (History)
4 users (show)

See Also:


Attachments
report mismatched private keys (625 bytes, patch)
2017-07-07 13:44 AEST, Damien Miller
no flags Details | Diff
with better error code (853 bytes, patch)
2017-07-28 13:42 AEST, Damien Miller
dtucker: ok+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description hujunjie 2017-07-04 16:48:56 AEST
1: we use 'ssh-keygen' create two  rsa pub/pri key pair with empty passphrase.
and names it to:
<id_rsa1, id_rsa1.pub>
<id_rsa2, id_rsa2.pub>

2: add two pub key to localhost sshd:
cat id_rsa1.pub >> ~/.ssh/authorized_keys
cat id_rsa2.pub >> ~/.ssh/authorized_keys

3: copy the unmatch pub/pri key to ~/.ssh directory
cp id_rsa1 ~/.ssh/id_rsa
cp id_rsa2.pub ~/.ssh/id_rsa.pub

4: login to localhost without agent.
SSH_AUTH_SOCK= ssh 127.0.0.1 -vvv
can see ssh prompt user input password because of
method pubkey failed. the reasion is identity_sign() used
the id_rsa2.pub as pubkey, and signed it by id_rsa1 private key. that
sshd verify signature failed.

but, if you remove ~/.ssh/id_rsa.pub,
ssh client will used full ~/.ssh/id_rsa (extract pubkey,privatekey) through
userauth_pubkey()
 -->sign_and_send_pubkey()
    -->identity_sign()
and login success.

i think ssh designed to login use pubkey as possible we you can.
and if id_rsa unmatch id_rsa.pub, ssh should trust id_rsa and drop the
rsa.pub file,  try do login again as id_rsa.pub not exist.
Comment 1 Damien Miller 2017-07-05 21:13:55 AEST
I thought we checked that the public key matches the private, but I'll take another look.
Comment 2 hujunjie 2017-07-06 13:06:28 AEST
i saw a comment 
'If the key is an certificate, try to find a matching private key....'
and key ssh-rsa is not certificate in keytypes.
so it skip private key match check in function sign_and_send_pubkey()
and maybe some reason i don't know to do this skip..

final the result is in ssh client's point of view, i have correct 
id_rsa keyfile but can't auto login, that is confusing.
Comment 3 Damien Miller 2017-07-07 13:44:57 AEST
Created attachment 3010 [details]
report mismatched private keys

I think this should catch the case of mismatched private and public keys.
Comment 4 hujunjie 2017-07-07 17:20:49 AEST
thanks, i apply this patch and it will report error if mismatched but not try private again,  and add some code to fix it and works.

a/sshconnect2.c

ssherr.h
---------------
    #define SSH_ERR_CONN_CORRUPT               -54
    #define SSH_ERR_PROTOCOL_ERROR          -55
+  #define SSH_ERR_KEY_RETRY_PRIVATE       -56

    /* Translate a numeric error code to a human-readable error string */
   const char *ssh_err(int n);


ssherr.c
---------------
        case SSH_ERR_PROTOCOL_ERROR:
            return "Protocol error";
+       case SSH_ERR_KEY_RETRY_PRIVATE:
+           return "Key retry private";
        default:
            return "unknown error";

ssconnect2.c
---------------
 	/* load the private key from the file */
 	if ((prv = load_identity_file(id)) == NULL)
 		return SSH_ERR_KEY_NOT_FOUND;
+	if (id->key != NULL && !sshkey_equal_public(prv, id->key)) {
+		error("%s: private key %s contents do not match public, try again with private key",
+		   __func__, id->filename);
+		return SSH_ERR_KEY_RETRY_PRIVATE;
+	}
 	ret = sshkey_sign(prv, sigp, lenp, data, datalen,
 	    key_sign_encode(prv), compat);
 	sshkey_free(prv);
...
     ret = identity_sign(id, &signature, &slen,
         buffer_ptr(&b), buffer_len(&b), datafellows);
     if (ret != 0) {
         if (ret != SSH_ERR_KEY_NOT_FOUND)
             error("%s: signing failed: %s", __func__, ssh_err(ret));
+        if (ret == SSH_ERR_KEY_RETRY_PRIVATE) {
+           id->tried = 0;
+            key_free(id->key);
+            id->key = NULL;
+            TAILQ_REMOVE(&authctxt->keys, id, next);
+            TAILQ_INSERT_HEAD(&authctxt->keys, id, next);
         }
         free(blob);
         buffer_free(&b);
Comment 5 Damien Miller 2017-07-09 17:42:10 AEST
I'm reluctant to add more code here. I think it's sufficient to inform the user and let them fix the configuration error if they desire.

(also, please use the attachments feature for patches in future.)
Comment 6 Damien Miller 2017-07-28 13:42:59 AEST
Created attachment 3021 [details]
with better error code

This improves the error code returned when the keys don't match.
Comment 7 Damien Miller 2017-08-11 14:47:06 AEST
Patch is applied, this will be in openssh-7.6. Thanks!
Comment 8 Damien Miller 2017-09-13 12:26:01 AEST
*** Bug 2776 has been marked as a duplicate of this bug. ***
Comment 9 Damien Miller 2017-11-03 14:12:02 AEDT
*** Bug 2661 has been marked as a duplicate of this bug. ***
Comment 10 Damien Miller 2021-04-23 15:11:01 AEST
closing resolved bugs as of 8.6p1 release