-
Notifications
You must be signed in to change notification settings - Fork 362
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
Please sign, fetch or otherwise securely transfer update su binaries #52
Comments
Using https instead of http would also address this issue. |
Oh, and by the way, out-of-the-box HTTPS won't fix this either. You don't want to trust CAs. |
@thejh Why is that? The CA store lives in /system, which is read only. You would need root to change the CA store. So, the system would already need to be compromised for su to be replaced. However, if this is a philosophical argument about how CAs are inherently scummy organizations, that's another story :) |
I think the larger issue here is that md5sums are used instead of something more collision-resistant such as whirlpool or sha512, not whether or not HTTPS is used, as HTTPS won't save you from the machine being compromised on the other end, while using a more resilient hash function would at least give you a fighting chance. |
@nenolod The hash only guarantees file integrity, not security. This is because the binaries and the hashes for the binaries are stored on the server. Furthermore, the su manifest json format allows you to specify the full url to the su binary. Currently, one only needs to compromise the manifest server to completely compromise everything. Signing the binary would also guarantee security. http://downloads.androidsu.com/superuser/su/manifestv2.json: [
{
"min-apk-version": 47,
"version-code": 18,
"version": "3.2",
"armeabi": {
"binary": "http://downloads.androidsu.com/superuser/su/su-3.2-armv5",
"binary-md5sum": "3f4fb4ecc5ff247d805f67715952e5de"
},
"x86" : {
"binary": "http://downloads.androidsu.com/superuser/su/su-3.2-x86",
"binary-md5sum": "122068847ad1dcb2a8f072776e3da20a"
}
},
{
"min-apk-version": 42,
"version-code": 17,
"version": "3.1.1",
"armeabi": {
"binary": "http://downloads.androidsu.com/superuser/su/su-3.1.1-armv5",
"binary-md5sum": "054c9a22d8900d50ce6172fd56bbf414"
},
"x86" : {
"binary": "http://downloads.androidsu.com/superuser/su/su-3.1.1-x86",
"binary-md5sum": "33ea8cd6e7c7a23eaa9ad97bb115ea55"
}
}
] |
@koush File integrity is a component of ensuring the entire system is secure. While I agree that su shouldn't be upgradable by the app (and it certainly was not in the good old days), if the manifest server is secured, and strong hashing is used, then this shouldn't be a huge problem. HTTPS won't guarantee that the manifest server is any more or any less secure. Further, signing in and of itself only guarantees file integrity as well, even if additional semantic meaning is implied by the presence of a signature... the signature is only meaningful if you trust the person who signed it's key has not been compromised. I guess what I am trying to say is, we probably shouldn't allow su to be upgraded in the first place. Signing and stronger hash functions aside, this is kind of a stupid feature. And really, what is the difference in all of these versions? The whole point is that it asks if you want to give it root or not, so this is an app that should by this point be not necessary to upgrade... less than 100 lines of code really, and that's including IPC back to the Java-side app... |
@koush Well, my point is that just too many people can act as a CA. For example, every german university has a CA certificate that all major browsers will accept. It'd be a lot more secure to use a specific signing key by the program author. @nenolod IMO, it might still be necessary to update su - there's a lot you can do wrong in 100 lines of code. |
IMHO the update code should be disabled until someone has written a 'secure' version. |
On Thu, Aug 16, 2012 at 02:56:48AM -0700, David wrote:
+1 |
I temporarily disabled the binary updater until I can find a better way to do things. |
@ChainsDD awesome! |
Add Traditional Chinese translation
It seems that at the present time the updater service found in su.apk fetches new su binaries over http and checks that the md5sum of the downloaded binary matches that defined in the json descriptor. [0]
I would like to suggest that future upgrades hashsums are distributed through su.apk via updating su.apk in the android market (and or the binary as well) which can be used to verify a su binary is
legit
.[0] https://github.com/ChainsDD/Superuser/blob/master/src/com/noshufou/android/su/service/UpdaterService.java
The text was updated successfully, but these errors were encountered: