Add support for SSH interactive authentication#869
Add support for SSH interactive authentication#869exactly-one-kas wants to merge 2 commits intorust-lang:masterfrom
Conversation
Add missing documentation
ehuss
left a comment
ehuss
left a comment
There was a problem hiding this comment.
Sorry for the long delay on the review.
I just wanted to confirm, the ssh2 library does not provide any way to indicate an error in the callback? Is the intended way to handle an error is to print a message and just not fill in the responses?
| let text = malloc(response_bytes.len()); | ||
| response.text = text as *mut c_char; | ||
| response.length = response_bytes.len() as u32; |
There was a problem hiding this comment.
There seems to be an issue here. The response is never copied into the buffer created via malloc.
I think this also needs a check for a null return from malloc.
I'm only partially confident this is the correct way to handle this. From what I can tell, ssh2 has its own allocation routines that are part of the session, but I don't think the rust side exposes that, and I think it always defaults to libc malloc/free, so I don't think it is a problem. However, I'm a bit nervous about this manual memory handling with APIs that don't really document the behavior.
| let response = &mut *responses.offset(i as isize); | ||
| let response_bytes = wrapped_responses[i as usize].as_bytes(); | ||
|
|
||
| // libgit2 frees returned strings |
There was a problem hiding this comment.
This is the ssh2 library that is freeing the memory, not git2 right?
|
☔ The latest upstream changes (possibly f095112) made this pull request unmergeable. Please resolve the merge conflicts. |
3 participants
For lifetime reasons,
Crednow doesn't always carry agit_credand thereforeCred::unwrap()may now panic if called on aCredcreated usingCred::ssh_interactive(); this is the only backwards-incompatible changeWithout leaking memory or memorizing created handlers in
RemoteCallbacksit's not possible to have individual handler for each SSH interactiveCred, so I compromized and put the handler directly intoRemoteCallbacksAlso, since
libgit2only ships with dummytypedefs forLIBSSH2_USERAUTH_KBDINT_PROMPTandLIBSSH2_USERAUTH_KBDINT_PROMPT,libssh2-sysis now a direct dependency for thesshfeature