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

Vulnerability: hard coded password for Android KeyStore #55

Closed
roycornelissen opened this issue Apr 11, 2014 · 9 comments

Comments

@roycornelissen
Copy link

commented Apr 11, 2014

For Android: instead of using a hard coded KeyStore password, the application should provide its own from a safe origin (the app should be responsible for coming up with a password).

Otherwise the Xamarin.Social.Accounts keystone in all apps that use the Xamarin.Auth components can be easily breached by just looking at the source code on GitHub.

I'll provide a PR in which I add the option to provide your own password. Ideally the hard coded pwd should be taken out, but I'll leave it in for backwards compatibility.

@ermau ermau added this to the v1.3 milestone Apr 21, 2014

moljac added a commit that referenced this issue Jun 27, 2016
1.3.1-alpha01 update
PR
nuget with windows platfromfs
Issue #55 Vulnerability: hard coded password for Android KeyStore
@dot-ly

This comment has been minimized.

Copy link

commented Jul 6, 2016

any fix for this??

@roycornelissen

This comment has been minimized.

Copy link
Author

commented Jul 7, 2016

@AhmedWarreth the fix is merged into the upcoming 1.3.1 alpha version (see 0cb978a) I think it'll be released soon.

@moljac

This comment has been minimized.

Copy link
Member

commented Jul 7, 2016

We have problems with our CI bots, the 1.3.1-alpha release is matter of hours.

The only thing that might be missing is UWP, it is already implemented, but cake build on CI bot is crashing. Update might help.

@dot-ly

This comment has been minimized.

Copy link

commented Jul 7, 2016

Thanks guys. So what's the work around for the hardcoded password? do i need to prompt the user for input? Opened this topic in http://stackoverflow.com/questions/38234603/xamarin-android-secure-strorage-keystore

Will be happy to post any updates once this is resolved.

@joseprl89

This comment has been minimized.

Copy link

commented Nov 9, 2016

Has this been solved or not?

I am rather confused on this because:

  • When I looked around the codebase, the security hole looked obvious and I checked issues.
  • This led me to this issue, which refers to 1.3.1-alpha.
  • The last release according to the Github releases tab is 1.2.2.
  • Had to look through the history, which pointed me to:
  • - Master branch being years old.
  • - Windows test branch being also years old.
  • - A non obvious branch contains now all the development and is somewhat up to date.

After looking into the code on the branch that looks to be the latest, I see that there's still the password, but I believe it is only used to migrate from the old password to the new one old clients. Is that the case?

Just to help people following my path in the future, could you:

  • Close the issue as resolved if it is. I believe it is since I found the version 1.3.1.1 in Nuget, which I assume is newer than 1.3.1-alpha
  • Tag the releases that are not tagged between 1.2.2 and 1.3.1.1.
  • Bring Master up to date containing the last stable release.
@moljac

This comment has been minimized.

Copy link
Member

commented Nov 9, 2016

@joseprl89

"security hole looked obvious and I checked issues"

True. Initial code was written with security bug. It was fixed in March/April and published as nuget.

"release according to the Github releases tab is 1.2.2."

One of the nuget.org problem is that there is no verification process, so anybody can submit Xamarin.Auth package. The version 1.2.2 is not listed here:
https://www.nuget.org/packages/Xamarin.Auth
Version 1.2.2 was taken out of component from component store and submitted here for those that do not need samples and docs. It is not supported.

Master branch is not up to date.

True.

Windows test branch being also years old.

True. This was experimental Windows Phone branch. In the meantime several Windows Platforms have been added, even UWP. The problem is that the whole publishing goes through CI servers and there were issues with UWP builds. Additionally merging hardware with Microsoft added some delay into the process.

A non obvious branch contains now all the development and is somewhat up to date.

True. It is obvious as windows-phone-experimental. We are still working on it. If you clone/checkout those you will see a lot of changes. For a lot of bugfixes we never got feedback.

there's still the password, but I believe it is only used to migrate from the old password to the new one old clients. Is that the case?

Exactly. Yes there are some bugs in the code and we try to maintain backward compatibility.

Close the issue as resolved if it is.

There is 2 approaches: close-n-dont-care or leave-it-open-until-someone-confirms-it-is-solved.
I prefer 2nd approach. I did not receive any notification that the fix fixes the issue. Believe me I would be more than happy to close it.

"found the version 1.3.1.1 in Nuget, which I assume is newer than 1.3.1-alpha"

According to semantic versioning used by nuget 1.3.1-* comes before 1.3.1 and thus 1.3.1.1.
Check the nuget list please. (Correct list, correct package).

Bring Master up to date containing the last stable release.

We are working on it.

@jzeferino

This comment has been minimized.

Copy link

commented Feb 25, 2017

Any update on this?

@moljac

This comment has been minimized.

Copy link
Member

commented Feb 26, 2017

If you read carefully this thread you will see there was commit with bug fix and the first nuget alpha version was 1.3.1-alpha-01.

The commit where this was changed was:
a42a569

and the nuget with that fix was packaged and pushed in this commit (mentioned in this thread):

0cb978a

The only problem is that this issue was not closed (yet).
Closing it now.

@moljac moljac closed this Feb 26, 2017

@marcosanfilippo

This comment has been minimized.

Copy link

commented Aug 28, 2017

There's the same issue for iOS?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.