Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sshconnect2.c: only re-order keys when resetting keys (instead of destroying the list and failing to rebuild it) #57

Closed
wants to merge 3 commits into from

Conversation

Feandil
Copy link
Contributor

@Feandil Feandil commented Nov 23, 2016

sshconnect2.c whipes all keys and close the agent fd between each authentication before trying to rebuild the list (but failing because building the list modifies its sources) and reconnecting to the agent.
However, during an authentication:

  • Almost all access to the keys/key list do not modify them
  • The only modification to them is in userauth_pubkey, which:
    • Re-order the key, increasing the tries count (up to 2)
    • Set the isprivate flag
    • All other functions called do not modify the keys

This PR proposes a solution by:

  • Modifying userauth_pubkey so that it cleans up isprivate when it cleans up the key
  • Adding a pubkey_reset function that:
    • Reverts the re-ordering (this could even be skipped as the order does not seem to carry much importance...)
    • Resets the tries to 0

This PR also include some function argument constification, to make it simpler to see that these function do not modify their arguments

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

None of these functions actually modify the keys they are acting on
sshconnect2.c whipes all keys and close the agent fd between each
authentication before rebuilding them and reconnecting to the agent.
However, during an authentication:
- Almost all access to the keys/key list do not modify them
- The only modification to them is in userauth_pubkey, which:
 * Re-order the key, increasing the 'tries' count (up to 2)
 * Set the 'isprivate' flag
 * All other functions called do not modify the keys

This commit:
- Modify userauth_pubkey so that it cleans up isprivate when it cleans
 up the key
- Add a pubkey_reset that:
 + Revert the re-ordering
 + Reset the 'tries' to 0

The re-ordering could be even be skipped as the order does not carry
much importance...
@djmdjm
Copy link
Contributor

djmdjm commented Feb 10, 2018

AFAIK this was committed in b9844a4

@djmdjm djmdjm closed this Feb 10, 2018
restyled-io bot pushed a commit to johnsonjh/j-hpn-ssh that referenced this pull request Feb 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants