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

Authentication API #908

Closed
ashh87 opened this Issue Sep 30, 2014 · 28 comments

Comments

Projects
None yet
10 participants
@ashh87
Contributor

ashh87 commented Sep 30, 2014

As a long term goal, I think it would be interesting to see if the API can be extended to allow public key authentication (eg ssh) using appropriate subkeys in a keyring. I've been using ES's sftp capabilities and had to import my private key to the app specifically, so I think it would be cool if apps like this could use the API to handle public key authentication without having to implement key handling themselves.

I think this should be possible, but I've not investigated the ssh protocol in depth to discover exactly what is required. I think it would increase the usefulness of OK though, as I suspect there are many apps which could potentially use this.

@Valodim

This comment has been minimized.

Show comment
Hide comment
@Valodim

Valodim Sep 30, 2014

Member

Hey, good to see you're still alive!

If we are content with providing the capability in a new API, that's actually fairly easy to implement - we pretty much just have to provide a raw "sign this challenge" method. There are existing APIs for this purpose, but they aren't exactly generic from what I remember. Might be worth investigating.

connectbot/connectbot#13

Member

Valodim commented Sep 30, 2014

Hey, good to see you're still alive!

If we are content with providing the capability in a new API, that's actually fairly easy to implement - we pretty much just have to provide a raw "sign this challenge" method. There are existing APIs for this purpose, but they aren't exactly generic from what I remember. Might be worth investigating.

connectbot/connectbot#13

@ashh87

This comment has been minimized.

Show comment
Hide comment
@ashh87

ashh87 Sep 30, 2014

Contributor

Still alive, still have a thesis to finish :)

That's good to hear, although I think different versions of the protocol require different things of the key - v1 requires decryption and v2 requires signing (??) - so should the API ask for the protocol version number and figure out the correct thing to do, or should it act as a simple interface, and have the app using the api request the correct action. Connectbot looks interesting, perhaps this problem was already considered there :)

Contributor

ashh87 commented Sep 30, 2014

Still alive, still have a thesis to finish :)

That's good to hear, although I think different versions of the protocol require different things of the key - v1 requires decryption and v2 requires signing (??) - so should the API ask for the protocol version number and figure out the correct thing to do, or should it act as a simple interface, and have the app using the api request the correct action. Connectbot looks interesting, perhaps this problem was already considered there :)

@dschuermann dschuermann changed the title from Authentication Keys to Authentication API Oct 1, 2014

@phryneas

This comment has been minimized.

Show comment
Hide comment
@phryneas

phryneas Mar 18, 2015

there is definitely demand for such a feature (see zeapo/Android-Password-Store#71 ) - is this still on the list?

phryneas commented Mar 18, 2015

there is definitely demand for such a feature (see zeapo/Android-Password-Store#71 ) - is this still on the list?

@dschuermann

This comment has been minimized.

Show comment
Hide comment
@dschuermann

dschuermann Mar 18, 2015

Member

@phryneas It's here in this issue tracker, so yes ;)
Unfortunately, there are much bigger ToDos than this enhancement, so I will not dive into this now.
We are open for pull requests :)

Member

dschuermann commented Mar 18, 2015

@phryneas It's here in this issue tracker, so yes ;)
Unfortunately, there are much bigger ToDos than this enhancement, so I will not dive into this now.
We are open for pull requests :)

@phryneas

This comment has been minimized.

Show comment
Hide comment
@phryneas

phryneas Mar 18, 2015

I'd be happy to, but I just don't have the resources as well as depth of knowledge to do so with good conscience.
For now. Maybe in a bunch of months, as a bigger side-project. But for now I'll hope someone else sees this and picks this up, either here or over at zeapo/Android-Password-Store ;)

phryneas commented Mar 18, 2015

I'd be happy to, but I just don't have the resources as well as depth of knowledge to do so with good conscience.
For now. Maybe in a bunch of months, as a bigger side-project. But for now I'll hope someone else sees this and picks this up, either here or over at zeapo/Android-Password-Store ;)

@fishsoupisgood

This comment has been minimized.

Show comment
Hide comment
@fishsoupisgood

fishsoupisgood May 29, 2016

I have a working proof of concept for this (it's a hack) with connectbot write-up is here: http://wordpress.panaceas.org/wp/index.php/2016/05/29/ssh-with-yubikey-neo-on-android/
patches are here http://git.panaceas.org/cgit.cgi/connectbot-yubikey/ (ignore the ones that touch the gradle stuff obviously). Apologies if the code is vile, I'm normally writing C for a living :).

fishsoupisgood commented May 29, 2016

I have a working proof of concept for this (it's a hack) with connectbot write-up is here: http://wordpress.panaceas.org/wp/index.php/2016/05/29/ssh-with-yubikey-neo-on-android/
patches are here http://git.panaceas.org/cgit.cgi/connectbot-yubikey/ (ignore the ones that touch the gradle stuff obviously). Apologies if the code is vile, I'm normally writing C for a living :).

@Valodim

This comment has been minimized.

Show comment
Hide comment
@Valodim

Valodim May 29, 2016

Member

Nice work! You mention ConnectBot development is active again in your blog post, did you send them a notice, too? We would definitely include the API call (with some modifications and also made to work with regular keys) in OpenKeychain if they wanted to integrate the feature.

Member

Valodim commented May 29, 2016

Nice work! You mention ConnectBot development is active again in your blog post, did you send them a notice, too? We would definitely include the API call (with some modifications and also made to work with regular keys) in OpenKeychain if they wanted to integrate the feature.

@hurricanehrndz

This comment has been minimized.

Show comment
Hide comment
@hurricanehrndz

hurricanehrndz Sep 19, 2016

@Valodim

Is is still part of the plan to include this feature. I would like to perhaps implement the new api calls for zeapo/Android-Password-Store.

hurricanehrndz commented Sep 19, 2016

@Valodim

Is is still part of the plan to include this feature. I would like to perhaps implement the new api calls for zeapo/Android-Password-Store.

@dschuermann

This comment has been minimized.

Show comment
Hide comment
@dschuermann

dschuermann Sep 20, 2016

Member

@hurricanehrndz due to limited resources nobody is working on this now. But we are open for pull requests.

Member

dschuermann commented Sep 20, 2016

@hurricanehrndz due to limited resources nobody is working on this now. But we are open for pull requests.

@Valodim

This comment has been minimized.

Show comment
Hide comment
@Valodim

Valodim Sep 20, 2016

Member

The commit from above contains the idea of how to do it: http://git.panaceas.org/cgit.cgi/connectbot-yubikey/open-keychain/commit/?id=5aa36b089f09346787e44f9850924faae1fda363

Although this should really live in its own operation, and needs tests to boot

Member

Valodim commented Sep 20, 2016

The commit from above contains the idea of how to do it: http://git.panaceas.org/cgit.cgi/connectbot-yubikey/open-keychain/commit/?id=5aa36b089f09346787e44f9850924faae1fda363

Although this should really live in its own operation, and needs tests to boot

@hurricanehrndz

This comment has been minimized.

Show comment
Hide comment
@hurricanehrndz

hurricanehrndz Sep 20, 2016

@dschuermann @Valodim Yes the patch mentioned and written by @fishsoupisgood should work, so as long as I fork, write tests and do a pr the chances of it being accepted are high? This is something I'm willing to do if no one else is.

hurricanehrndz commented Sep 20, 2016

@dschuermann @Valodim Yes the patch mentioned and written by @fishsoupisgood should work, so as long as I fork, write tests and do a pr the chances of it being accepted are high? This is something I'm willing to do if no one else is.

@Valodim

This comment has been minimized.

Show comment
Hide comment
@Valodim

Valodim Sep 20, 2016

Member

Sure, it's functionality we would support. We wouldn't have left this ticket open otherwise :)

Member

Valodim commented Sep 20, 2016

Sure, it's functionality we would support. We wouldn't have left this ticket open otherwise :)

@dschuermann

This comment has been minimized.

Show comment
Hide comment
@dschuermann

dschuermann Sep 20, 2016

Member

@hurricanehrndz Yes, I would love to see this! We can do code reviews and give you directions when you open a "Work in Progress" pull request. Also: https://www.openkeychain.org/pr-incentive ;)

Member

dschuermann commented Sep 20, 2016

@hurricanehrndz Yes, I would love to see this! We can do code reviews and give you directions when you open a "Work in Progress" pull request. Also: https://www.openkeychain.org/pr-incentive ;)

@hurricanehrndz

This comment has been minimized.

Show comment
Hide comment
@hurricanehrndz

hurricanehrndz Sep 20, 2016

@dschuermann

Thanks. I will see what I can do.

hurricanehrndz commented Sep 20, 2016

@dschuermann

Thanks. I will see what I can do.

@fishsoupisgood

This comment has been minimized.

Show comment
Hide comment
@fishsoupisgood

fishsoupisgood Sep 20, 2016

The reason I didn't persue this was that I couldn't convince myself this was the right approach.

  1. Even if the api were made more general (sign this with key x and algorythm y please) it's going to be hard to push that functionality into all the various ssh clients.

  2. There's already a sensible API for doing this on the ssh side - viz ssh-agents. And desktop gpg implements a perfectly servicable ssh agent.

I almost convinced myself the correct approach would be to implement an ssh agent in an open-keychain service, however there's one issue - the current system does make it much easier to see which app is requesting the signature.

  • J.

fishsoupisgood commented Sep 20, 2016

The reason I didn't persue this was that I couldn't convince myself this was the right approach.

  1. Even if the api were made more general (sign this with key x and algorythm y please) it's going to be hard to push that functionality into all the various ssh clients.

  2. There's already a sensible API for doing this on the ssh side - viz ssh-agents. And desktop gpg implements a perfectly servicable ssh agent.

I almost convinced myself the correct approach would be to implement an ssh agent in an open-keychain service, however there's one issue - the current system does make it much easier to see which app is requesting the signature.

  • J.
@hurricanehrndz

This comment has been minimized.

Show comment
Hide comment
@hurricanehrndz

hurricanehrndz Sep 20, 2016

@fishsoupisgood
I would totally agree with your statements, that being said I believe having some of the functionality is better than not having the functionality at all. Additionally, who is to know how and when developers will implement the ssh api, one thing is for sure it will never be implemented if it's never available. I know you believe the code you wrote is an ugly hack, but it works.

hurricanehrndz commented Sep 20, 2016

@fishsoupisgood
I would totally agree with your statements, that being said I believe having some of the functionality is better than not having the functionality at all. Additionally, who is to know how and when developers will implement the ssh api, one thing is for sure it will never be implemented if it's never available. I know you believe the code you wrote is an ugly hack, but it works.

@fishsoupisgood

This comment has been minimized.

Show comment
Hide comment
@fishsoupisgood

fishsoupisgood Sep 20, 2016

Sure, the open-keychain patches aren't too bad they just hijack the the existing structure and flow to do something different, I'm sure someone here will have views on what the correct way to do that is without spawning another pile of classes.

The connectbot patches OTOH are more complicated as the interactive patch (the rpc to open-keychain) need to come from the activity rather than the service where they originate. That needs someone to plumb though a proper noticiation from the service to the activity rather than just lobbing the class down into the service (or one could do this the proper android way with a notification although I think the UX would be horrible in that case).

I mostly did this as a proof of concept so that both pieces would have motivation to fix it, but I have ended up using it almost daily.

Either way - I'd be exceedingly happy to see someone look after this and get it all upstream.

  • J.

fishsoupisgood commented Sep 20, 2016

Sure, the open-keychain patches aren't too bad they just hijack the the existing structure and flow to do something different, I'm sure someone here will have views on what the correct way to do that is without spawning another pile of classes.

The connectbot patches OTOH are more complicated as the interactive patch (the rpc to open-keychain) need to come from the activity rather than the service where they originate. That needs someone to plumb though a proper noticiation from the service to the activity rather than just lobbing the class down into the service (or one could do this the proper android way with a notification although I think the UX would be horrible in that case).

I mostly did this as a proof of concept so that both pieces would have motivation to fix it, but I have ended up using it almost daily.

Either way - I'd be exceedingly happy to see someone look after this and get it all upstream.

  • J.
@dschuermann

This comment has been minimized.

Show comment
Hide comment
@dschuermann

dschuermann Sep 20, 2016

Member

Based on the comment by the maintainer of ConnectBot Kenny Root @kruton in connectbot/connectbot#13 (comment) and connectbot/connectbot#405 (comment)
and our own experiences, I think the correct approach here would be:

  1. define a new SSH specific API as AIDL in org.openintents.ssh (see OpenPGP API at https://github.com/open-keychain/openpgp-api/tree/master/openpgp-api/src/main/aidl/org/openintents/openpgp)
  2. make it work like our current OpenPGP API by returning a PendingIntent for confirmation if an app tries to access the API (connectbot/connectbot#13 (comment)). What I think we don't need for SSH are the ParcelFileDescriptor pipes because the SSH API is for authentication only, not for encrypting large amounts of data.
  3. make a pull request here at OpenKeychain that implements the provider-side of this API.
  4. make a pull request at ConnectBot that implements the client-side of this API.
Member

dschuermann commented Sep 20, 2016

Based on the comment by the maintainer of ConnectBot Kenny Root @kruton in connectbot/connectbot#13 (comment) and connectbot/connectbot#405 (comment)
and our own experiences, I think the correct approach here would be:

  1. define a new SSH specific API as AIDL in org.openintents.ssh (see OpenPGP API at https://github.com/open-keychain/openpgp-api/tree/master/openpgp-api/src/main/aidl/org/openintents/openpgp)
  2. make it work like our current OpenPGP API by returning a PendingIntent for confirmation if an app tries to access the API (connectbot/connectbot#13 (comment)). What I think we don't need for SSH are the ParcelFileDescriptor pipes because the SSH API is for authentication only, not for encrypting large amounts of data.
  3. make a pull request here at OpenKeychain that implements the provider-side of this API.
  4. make a pull request at ConnectBot that implements the client-side of this API.
@splack

This comment has been minimized.

Show comment
Hide comment

splack commented Sep 18, 2017

@Valodim

This comment has been minimized.

Show comment
Hide comment
@Valodim

Valodim Sep 18, 2017

Member

This is being worked on, actually. :)

Member

Valodim commented Sep 18, 2017

This is being worked on, actually. :)

@dschuermann

This comment has been minimized.

Show comment
Hide comment
Member

dschuermann commented Dec 19, 2017

@Valodim

This comment has been minimized.

Show comment
Hide comment
@Valodim

Valodim Jan 8, 2018

Member

It's worth mentioning that on OpenKeychain's side, this has been merged and released since December. We are just waiting for the connectbot guys to merge our PRs here, hopefully they'll get around to it soon :)

Member

Valodim commented Jan 8, 2018

It's worth mentioning that on OpenKeychain's side, this has been merged and released since December. We are just waiting for the connectbot guys to merge our PRs here, hopefully they'll get around to it soon :)

@ngharo

This comment has been minimized.

Show comment
Hide comment
@ngharo

ngharo May 9, 2018

I just installed TermBot and successfully used the SSH authentication API for logging into my server. I think this issue could be closed and that bounty claimed?

Thanks so much and good work!

ngharo commented May 9, 2018

I just installed TermBot and successfully used the SSH authentication API for logging into my server. I think this issue could be closed and that bounty claimed?

Thanks so much and good work!

@erayd

This comment has been minimized.

Show comment
Hide comment
@erayd

erayd May 9, 2018

TermBot auth via OpenKeychain (using a YubiKey Neo) isn't working for me at all - it looks like it's working, asks for the yubikey etc., and then fails authentication, every time, without exception. Connecting to the same hosts using the yubikey works just fine from my laptop.

erayd commented May 9, 2018

TermBot auth via OpenKeychain (using a YubiKey Neo) isn't working for me at all - it looks like it's working, asks for the yubikey etc., and then fails authentication, every time, without exception. Connecting to the same hosts using the yubikey works just fine from my laptop.

@Valodim

This comment has been minimized.

Show comment
Hide comment
@Valodim

Valodim May 9, 2018

Member

Please report issues with termbot to the termbot repository. Also, make sure you have the latest version of OpenKeychain installed, we fixed a bug a couple days ago that might be the cause of this.

Member

Valodim commented May 9, 2018

Please report issues with termbot to the termbot repository. Also, make sure you have the latest version of OpenKeychain installed, we fixed a bug a couple days ago that might be the cause of this.

@erayd

This comment has been minimized.

Show comment
Hide comment
@erayd

erayd May 9, 2018

Will do, stand by.

erayd commented May 9, 2018

Will do, stand by.

@erayd

This comment has been minimized.

Show comment
Hide comment
@erayd

erayd May 9, 2018

@Valodim Looks like it was the OpenKeychain version. I was using 5.0, which did not work - however after updating to 5.0.2, it now works correctly. Thanks 👍.

Unrelated, but do you know whether there are any other SSH clients which are also implementing auth via OpenKeychain? I'd love to see JuiceSSH in particular add support for this.

erayd commented May 9, 2018

@Valodim Looks like it was the OpenKeychain version. I was using 5.0, which did not work - however after updating to 5.0.2, it now works correctly. Thanks 👍.

Unrelated, but do you know whether there are any other SSH clients which are also implementing auth via OpenKeychain? I'd love to see JuiceSSH in particular add support for this.

@hagau

This comment has been minimized.

Show comment
Hide comment
@hagau

hagau May 9, 2018

Member

I'm closing this since sshauthentication-api has been implemented in OpenKeychain some time ago and has already been used in the wild for a while.
For any bugs in the implementation, please open separate issues.

Member

hagau commented May 9, 2018

I'm closing this since sshauthentication-api has been implemented in OpenKeychain some time ago and has already been used in the wild for a while.
For any bugs in the implementation, please open separate issues.

@hagau hagau closed this May 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment