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

[linux keychain] new credential helper interfacing 99designs/keyring package #272

Merged
merged 2 commits into from Apr 8, 2019
Merged

[linux keychain] new credential helper interfacing 99designs/keyring package #272

merged 2 commits into from Apr 8, 2019

Conversation

Meroje
Copy link
Contributor

@Meroje Meroje commented Jan 28, 2019

Keyring integration for linux (more specifically gnome as it was using secret-service) has already been tried in v2.8.0 but reverted for v2.8.1 because of linking issues with libsecret for people not having on their systems (maybe they were using kde, so kwallet integration would have worked) #217.

I would like to try this again because saml2aws works very well for us (except for okta doing weird things with mfa) and proper linux integration would be a major usability gain for our linux users.

I propose to replace every current credential helpers with a new one deferring to https://github.com/99designs/keyring for storage. We tested and confirmed this to work on osx's keychain, secret-service and kwallet, it does change the storage format so there would be a new password prompt for current users though.
Before this can be considered ready for anything more it probably needs tweaking on the AllowedBackends setting depending on if the fallback to encrypted file storage is desirable.

Thanks for any comment you may have on this and your time for making this tool.

PS: I removed credentials.List(), this is a lazy move but it was dead code, let me know if it must stay.

fixes #266,#254

@Meroje Meroje changed the title [linux keychain] Replace credential helpers with 99designs/keyring package from aws-vault [linux keychain] Replace credential helpers with 99designs/keyring package Jan 28, 2019
@wolfeidau
Copy link
Contributor

We need to see how this affects existing saved credentials in OSX.

Last time i tried this it orphaned everything, interested in your thoughts on this?

Cheers for doing such a neat PR and I REALLY want this at the moment.

@Meroje
Copy link
Contributor Author

Meroje commented Feb 9, 2019

Yeah so this is going to use the "application password" kind of osx item (but the current helper is doing neat internet password) with a not ideal json content, I did not realize it is doing this compromise. I will look at maybe using it only for linux and keeping the current helpers instead.

@Meroje
Copy link
Contributor Author

Meroje commented Feb 9, 2019

I'll spin a desktop linux vm when at work Monday to test this again.

@Meroje Meroje changed the title [linux keychain] Replace credential helpers with 99designs/keyring package [linux keychain] new credential helper interfacing 99designs/keyring package Feb 9, 2019
@Meroje
Copy link
Contributor Author

Meroje commented Feb 11, 2019

Alright this is still working fine, just ran with libsecret on gnome/unity

parallels@ubuntu-18-04:~/go/src/github.com/versent/saml2aws$ go run cmd/saml2aws/main.go --verbose login
2019/02/11 14:25:21 [keyring] Considering backends: [kwallet secret-service pass]
2019/02/11 14:25:21 [keyring] Failed backend kwallet: The name org.kde.kwalletd was not provided by any .service files
DEBU[0000] Running                                       command=login
DEBU[0000] check if Creds Exist                          command=login
DEBU[0000] Expand                                        name=/home/parallels/.aws/credentials pkg=awsconfig
DEBU[0000] resolveSymlink                                name=/home/parallels/.aws/credentials pkg=awsconfig
DEBU[0000] ensureConfigExists                            filename=/home/parallels/.aws/credentials pkg=awsconfig
DEBU[0000] Dir created                                   dir=/home/parallels/.aws pkg=awsconfig
DEBU[0000] File created                                  filename=/home/parallels/.aws/credentials pkg=awsconfig
Using IDP Account default to access Okta ***redacted***
To use saved password just hit enter.
? Username ***redacted***
? Password 

DEBU[0013] building provider                             command=login idpAccount="account {***redacted***}"
Authenticating as ***redacted*** ...
DEBU[0013] HTTP Req                                      URL="***redacted***" http=client method=POST
DEBU[0015] HTTP Res                                      Status="200 OK" http=client
DEBU[0015] HTTP Req                                      URL="***redacted***" http=client method=GET
DEBU[0015] HTTP Res                                      Status="200 OK" http=client
DEBU[0015] auth complete                                 provider=okta
Selected role: arn:aws:iam::***redacted***
Requesting AWS credentials using SAML assertion
DEBU[0018] ensureConfigExists                            filename=/home/parallels/.aws/credentials pkg=awsconfig
Logged in as: arn:aws:sts::***redacted***

Your new access key pair has been stored in the AWS configuration
Note that it will expire at 2019-02-12 02:25:39 +0100 CET
To use this credential, call the AWS CLI with the --profile option (e.g. aws --profile bastion ec2 describe-instances).

@nenad
Copy link

nenad commented Apr 5, 2019

Any updates on this?

@wolfeidau
Copy link
Contributor

Apologies meant to post a comment on this a while back.

The missing support is upstream now keybase/go-keychain#36

The reason I have held off merging this is IF 99designs updates this library to support OSX correctly then this PR will merge and support both, removing a whole load of crufty c code.

Interested on thoughts on this?

@Meroje
Copy link
Contributor Author

Meroje commented Apr 8, 2019

Last I checked there was a lot of work to properly support this secClass with all its fields in go-keychain. Declaring the type is a first step, but there's more to do.

@wolfeidau wolfeidau merged commit 9814b03 into Versent:master Apr 8, 2019
@cervantek
Copy link
Contributor

@wolfeidau looks like this got merged in the day after a release. Any chance we can get another release soon?

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.

Support for Linux KDE keychain
4 participants