Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

serious security risk: passwords in clear text on redhat6 #1458

Closed
moscicki opened this Issue Feb 19, 2014 · 15 comments

Comments

Projects
None yet
4 participants
Contributor

moscicki commented Feb 19, 2014

Hello,

Following the discussion here owncloud#999
we can still see passwords stored in cleartext in the config files on redhat.

I think if a password cannot be stored securely in a qtkeychain then it should never be stored plain-text into any file. This protection should be built-in in the client because a similar problem may appear on other systems, depending on the configuration.

Note: on my system I have libqtkeychain0 rpm installed by the owncloud client (version: 0.20120221 vendor: owncloud).

Could this issue be fixed please asap?

kuba

Contributor

moscicki commented Feb 19, 2014

There seems to be a packaging problem with require on libqtkeychain0 so the new client versions do not get the newer version of the library (opt-libqtkeychain0).

However, I removed the old libqtkeychain0 and reinstalled 1.5.1 which pulled in opt-libqtkeychain0 -- the passwords are still saved in clear text.

So the problem persists.

The bottom line is the same: if passwords cannot be stored in cryptographically strong way into this file (let alone clear text) then they should never be stored.

kuba

Owner

dragotin commented Feb 19, 2014

@moscicki which desktop environment is in use on that platfrom, incl. version? Qtkeychain has support for the GNOME and KDE key rings, but probably none is there on the platform. If no encryption backend is available, the qtkeychain lib falls back to the plain text workaround.

So we basically have two options: We either can make a keychain backend working, or we disable the plain text backend and people will have to log in on every client start.

Contributor

moscicki commented Feb 19, 2014

I will get back to you on the KDE/GNOME versions.

However, is it possible to detect at runtime if the keychain support is there (and working) before falling back onto plain text? If yes, then I would propose that the behaviour be never to fall back on plain text and ask for password on every start. If other users in your community require plain text then this default behaviour could be configurable (I would be happy with an option in the config file which we may influence at install time, no necessarily fully fledged GUI option).

kuba

On Feb 19, 2014, at 6:58 PM, dragotin notifications@github.com wrote:

@moscicki which desktop environment is in use on that platfrom, incl. version? Qtkeychain has support for the GNOME and KDE key rings, but probably none is there on the platform. If no encryption backend is available, the qtkeychain lib falls back to the plain text workaround.

So we basically have two options: We either can make a keychain backend working, or we disable the plain text backend and people will have to log in on every client start.


Reply to this email directly or view it on GitHub.

Contributor

moscicki commented Feb 20, 2014

From our linux support:

Gnome 2.28 (seahorse) , KDE 4.3 (kdewallet)

Nontheless, I would strongly suggest to disable plain text fallback by default if there is any malfunction of the keychain.

kuba

On Feb 19, 2014, at 7:15 PM, Jakub Moscicki jakub.moscicki@gmail.com wrote:

I will get back to you on the KDE/GNOME versions.

However, is it possible to detect at runtime if the keychain support is there (and working) before falling back onto plain text? If yes, then I would propose that the behaviour be never to fall back on plain text and ask for password on every start. If other users in your community require plain text then this default behaviour could be configurable (I would be happy with an option in the config file which we may influence at install time, no necessarily fully fledged GUI option).

kuba

On Feb 19, 2014, at 6:58 PM, dragotin notifications@github.com wrote:

@moscicki which desktop environment is in use on that platfrom, incl. version? Qtkeychain has support for the GNOME and KDE key rings, but probably none is there on the platform. If no encryption backend is available, the qtkeychain lib falls back to the plain text workaround.

So we basically have two options: We either can make a keychain backend working, or we disable the plain text backend and people will have to log in on every client start.


Reply to this email directly or view it on GitHub.

Contributor

moscicki commented Feb 20, 2014

Hello,

We think we have found a possible solution (thanks to our Linux support team). Still there is a problem with the fallback to clear-text which should not be happening within the owncloud sync client.

libqtkeychain follows this logic:

static KeyringBackend detectKeyringBackend()
{
   if ( getenv( "GNOME_KEYRING_CONTROL" ) && GnomeKeyring::isSupported() )
       return Backend_GnomeKeyring;
   else
       return Backend_Kwallet;
}

The version of seahorse which we have defines GNOME_KEYRING_SOCKET but not GNOME_KEYRING_CONTROL.

What about modifying the condition of detectKeyringBackend() either to take this variable into account OR to disable GNOME_KEYRING_CONTROL check altogether?

I am not sure how it works but if GnomeKeyring::isSupported() connects to the keyring daemon to check if it can talk to it then I don't know why there is an additional check of any environment variables in the code.

In either case, let me emphasise once more: the fallback on plain text should not happen no matter what environment or system configuration. At least not without a very very clear user consent.

Owner

dragotin commented Feb 20, 2014

Yes, I agree. I completely removed the writing of the plain password an added code that removes it if it is in the config file.

Owner

dragotin commented Feb 20, 2014

I also patched the QtKeychain package to be able to detect the older keychain implementation based on your suggestion to check for GNOME_KEYRING_SOCKET, thanks for that. If you update to this new package from the isv:ownCloud:desktop repository, it uses the seahorse store on CentOS6, tested here.
The fix that does clearly not store the password in the config file any more comes with 1.5.2.

@dragotin dragotin closed this Feb 20, 2014

Contributor

moscicki commented Feb 20, 2014

Thanks for that. I think the rhel repo does not yet have the new libqtkeychain0 build...

Owner

dragotin commented Feb 20, 2014

@moscicki now the packages should be synced out to the mirrors. Note that it is repository isv:ownCloud:desktop

Contributor

moscicki commented Feb 20, 2014

I am using this repository http://download.opensuse.org/repositories/isv:/ownCloud:/desktop/RedHat_RHEL-6

I have erased owncloud-client package with yum (old build 1.5.1-8.1) and installed owncloud-client again (so build 1.5.1-8.2)

The problem persists.

Looking into the content of the repository the package opt-libqtkeychain0-0.20130805-2.1 stll has a very old create date. So probably was not updated in the repo?

On Feb 20, 2014, at 5:14 PM, dragotin notifications@github.com wrote:

@moscicki now the packages should be synced out to the mirrors. Note that it is repository isv:ownCloud:desktop


Reply to this email directly or view it on GitHub.

Collaborator

danimo commented Feb 20, 2014

@moscicki We added a patch, and the date relates to the git snapshot date from upstream.

Owner

dragotin commented Feb 20, 2014

no, the problem is that the package was not yet updated on the mirrors, strange. I will trigger a rebuild now.

Owner

dragotin commented Feb 21, 2014

Now the packages are up to date.

Contributor

moscicki commented Feb 21, 2014

OK, thank you. Works for me.

I think this broke things on Windows, the username and password have to be reconfigured after every reboot.
I haven't tested Linux yet.

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