zalando / go-keyring

Cross-platform keyring interface for Go
MIT License
881 stars 86 forks source link

[MacOS] Insecure keychain usage #110

Open rumiljonov opened 2 months ago

rumiljonov commented 2 months ago

Using /usr/bin/security to interact with the keychain undermines its security mechanisms and makes it as secure as storing data on the file system with global read permissions. By default, Keychain entries grant password-less access to the app that created them. This means anyone able to execute code on the machine can call /usr/bin/security and access credentials without a password prompt.

I think this behaviour should be changed or a disclaimer should be added to the README, as it might currently give users a false sense of security.

I've experimented with the go-keychain (https://github.com/keybase/go-keychain) library, which uses API calls to communicate with the keychain, resulting in proper permissions being set. It could be a good candidate for replacing the /usr/bin/security logic. However, I've encountered one issue: Apple seems to rely on the checksum of the binary when granting permissions, so when the Go binary is rebuilt, it loses access to old secrets in the keychain.

szuecs commented 2 months ago

Using /usr/bin/security to interact with the keychain undermines its security mechanisms and makes it as secure as storing data on the file system with global read permissions. By default, Keychain entries grant password-less access to the app that created them. This means anyone able to execute code on the machine can call /usr/bin/security and access credentials without a password prompt.

I think it's not fully correct statement. If you can execute arbitrary command as a different user you have no access to other users default keychain, so you have to be that user.

Not listed but another weakness: Shelling out has also the disadvantage of having the password in the process listing, so anyone that can do ps aux (global/other-users process listing) can read the password to open the keychain.

I think this behaviour should be changed or a disclaimer should be added to the README, as it might currently give users a false sense of security.

I've experimented with the go-keychain (https://github.com/keybase/go-keychain) library, which uses API calls to communicate with the keychain, resulting in proper permissions being set. It could be a good candidate for replacing the /usr/bin/security logic. However, I've encountered one issue: Apple seems to rely on the checksum of the binary when granting permissions, so when the Go binary is rebuilt, it loses access to old secrets in the keychain.

There was at least one trial to change the code to do API calls instead but it was also broken in some details. Please check the GH issues and PRs to cross link from here. If you can fix the problems mentioned in the other trials we are open to review and accept the code change. Keep in mind this lib is used by a lot of dependencies. To break these is not an option and would likely lead to a revert.

Thanks for your time checking the issue.

rumiljonov commented 2 months ago

I think it's not fully correct statement. If you can execute arbitrary command as a different user you have no access to other users default keychain, so you have to be that user.

Oh, good point that I've missed, thanks for correcting me