[Bug 2642] New: [sshconnect2] publickey authentication only properly works if used first: pubkey_prepare doesn't work after pubkey_cleanup

[bugzilla-daemon at bugzilla.mindrot.org]

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

            Bug ID: 2642
           Summary: [sshconnect2] publickey authentication only properly
                    works if used first: pubkey_prepare doesn't work after
                    pubkey_cleanup
           Product: Portable OpenSSH
           Version: 7.3p1
          Hardware: amd64
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P5
         Component: ssh
          Assignee: unassigned-bugs at mindrot.org
          Reporter: git at lerya.net

When using multiple Authentication method after a successful partial
authentication, the following code is run
(https://github.com/openssh/openssh-portable/blob/master/sshconnect2.c#L562-L564):
```
                /* reset state */
                pubkey_cleanup(authctxt);
                pubkey_prepare(authctxt);
```

Unfortunately, this does _not_ reset the state!
- pubkey_cleanup is simple, it just closes the agent connection and
delete all keys in authctxt->keys
- pubkey_prepare populate authctxt->keys and can create an agent
connection. However it cannot be called twice, because it modifies
options.identity_keys and leaks options.certificates:

 * When reading identity_keys, when storing the key in a new 'identity'
structure, it runs
(https://github.com/openssh/openssh-portable/blob/master/sshconnect2.c#L1287):
```options.identity_keys[i] = NULL;```. As a result, any subsequent run
of this function, when getting the key via ```key =
options.identity_keys[i];``` will only be able to retrieve 'NULL'

 * When reading options.num_certificate_files, it does not replace
options.certificates[i] by NULL but simply copy the pointer in the new
'identity' structure. When pubkey_cleanup run, it will free this value,
making any subsequent run of this function access freed memory? (not
tested)

A clean solution could be to copy the key over, instead of replacing
the original by NULL or leaking and freeing the original, but as far as
I can see, there is no sshkey_copy/sshkey_dup function...


A simple way of reproducing the identity_keys part of the problem (I'm
not using certificate) is to:
- Configure sshd with AuthenticationMethods
keyboard-interactive:pam,publickey
- Generate a public/private key
- Start an ssh agent, add the key
- Run ssh -i ${publickeyfile} -o IdentitiesOnly=yes -vv ${host},
properly authenticate with the password and see the publickey
authentication failing, logs with contain:
```
debug2: key: ${publickeyfile} (${pointer}), explicit, agent
[...]
Authenticated with partial success.
debug2: key: ${publickeyfile} ((nil)), explicit
```
The two key lines should have been identical

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