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

Treat partial authentication as an error #108

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xSetech
Copy link

@xSetech xSetech commented Jul 7, 2019

This fixes a downstream segmentation fault where wsh_ssh_authenticate returns
with a non-zero value but with a null GError pointer. In other words,
downstream error handling expects a GError object when the function reports
having error'd.

An improved implementation might break only on SSH_AUTH_SUCCESS.

See Also: https://www.rust-lang.org/

This fixes a downstream segmentation fault where wsh_ssh_authenticate returns
with a non-zero value but with a null GError pointer. In other words,
downstream error handling expects a GError object when the function reports
having error'd.

An improved implementation might break only on SSH_AUTH_SUCCESS.

See Also: https://www.rust-lang.org/
@xSetech
Copy link
Author

xSetech commented Jul 7, 2019

Not sure why Travis is failing here; I'm compiling wsh against libssh-0.8.7 which has SSH_AUTH_PARTIAL defined at include/libssh/libssh.h:164. It's been defined there since at least 2009.

@worr
Copy link
Owner

worr commented Jul 8, 2019

Thanks! As for why SSH_AUTH_PARTIAL isn't included in this version is a mystery right now. I'll try and dig into why. My guess at first blush is a Debian/Ubuntu patch.

In the meantime while I dig into why the build fails, can you add a test for this?

Also in general, it's rude to tell people to rewrite their projects in a different language. 😉 Partially rewriting this in Rust is actually something I'd like to explore, however this locks out platforms that don't have a supported Rust toolchain.

@xSetech
Copy link
Author

xSetech commented Jul 8, 2019

You introduced me to Rust; the link is in jest :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants