liferay / liferay-mobile-sdk

Other
47 stars 56 forks source link

Add support for digest authentication for iOS #23

Closed brunofarache closed 5 years ago

brunofarache commented 8 years ago

Hi @baxtheman! Better to discuss the issue here.

During the development of our internal apps, we always used basic auth or OAuth, that's the main reason why a digest Authenticator implementation was never added to iOS.

The only time we had to to use digest was for WebDAV downloads, which by default requires digest and not basic.

In Android, we require callers to use the digest authenticator implementation:

https://github.com/brunofarache/liferay-mobile-sdk/blob/master/android/src/main/java/com/liferay/mobile/android/http/file/DownloadUtil.java#L37-L41

In iOS, however, by passing a NSURLCredential, the iOS SDK decides between basic or digest for us, as described here.

So we do this:

https://github.com/brunofarache/liferay-mobile-sdk/blob/master/ios/Source/Http/File/LRDownloadDelegate.m#L84-L88

I will try to add digest support for non download requests by adding a NSURLCredential here:

https://github.com/brunofarache/liferay-mobile-sdk/blob/master/ios/Source/Http/LRHttpUtil.m#L194-L206

If it works, I will change the LRAuthentication protocol to not only receive the request object but also receive NSURLSession to have the chance to set the correct NSURLCredential,

baxtheman commented 8 years ago

Hi Bruno, yes this place is better than twitter, sorry!

Our mobile teams (android, ios) is searching to a better authentication than BASIC one and we starting to test Digest.

We notice that for iOs Digest is missing, but testing Digest to call /api/jsonws services by Android platform we experience it's impossible to login.

The digest auth implementation in https://github.com/brunofarache/liferay-mobile-sdk/blob/master/android/src/main/java/com/liferay/mobile/android/auth/basic/DigestAuthentication.java seems to be not compliant with the com.liferay.portal.security.auth.DigestAuthenticationAuthVerifier in the portal core.

Finally, in CE portal enviroment (No OAuth availability), the only running authentication method is BASIC.

I think it's will be nice to have a fully implemented Digest auth in mobile SDK, stating to fix the android one.

Thank you for your attention, Daniele

brunofarache commented 8 years ago

Interesting, @baxtheman.

DigestAuthentication.java does work while downloading files from WebDAV, if not, the Sync app wouldn't work at all.

I will test Digest with /api/jsonws. Silly/lazy question: how do I enable digest for /jsonws, is it in portal.properties?

brunofarache commented 8 years ago

Another question, are you going to use SSL? Because if so, Basic is as safer as Digest, I think.

yuchi commented 8 years ago

I support Bruno on this. There's no value in using Digest if you're under strict and enforced SSL.

baxtheman commented 8 years ago

Hi Pier, welcome!

I agree SSL is the best solution to fix the security matter, but it's not always applicable.

I was testing the Digest way to have a better security than Basic, without load a extra effort for the project (https layer).

brunofarache commented 8 years ago

Correct me if I'm wrong please. Digest is just a way of avoiding to send credentials in plain text, if SSL isn't enabled, all other request data will be in plain text. Which is kind of pointless to me, since credentials usually are not the only sensitive data that need to be protected.

Back to the SDK :P. In the portal, how do I enable digest for jsonws?

baxtheman commented 8 years ago

Correct Bruno, in digest auth the password is sent using MD5 hashing and it's not readable in clear text sniffing http traffic.

To enable digest for jsonws use a portal-ext.properties adding these lines:

#
# DigestAuthenticationAuthVerifier
#
auth.verifier.DigestAuthenticationAuthVerifier.hosts.allowed=
auth.verifier.DigestAuthenticationAuthVerifier.urls.includes=/api/*,/xmlrpc/*

It should be enough.

yuchi commented 8 years ago

But if I can sniff the initial handshake then I have most of the information to perform a (cheap) brute force attack on the hash. And given the fact that DIGEST is a standardized protocol I think there are a lot of “utilities” out there.

Portal supports DIGEST, so the SDK must support it too, but I wouldn’t consider it more secure than Basic, just hardened.

baxtheman commented 8 years ago

Basic auth without https means no security at all. Digest it's a first step forward to have a bit hardest security.

Any others way (without https) are welcome :-)

brunofarache commented 8 years ago

Digest being supported by portal alone doesn't justify the Mobile SDK must support it too.

The portal supports tons of other authentication mechanisms, most of them are not mobile friendly, it would be hard for the SDK to support all them. But yeah, digest seems to be an easy one, I will try it out.

We need to make it easier for devs to provide their own authentication implementation, it seems that providing just the request object is not enough (here and here). For iOS, for example, I will change the protocol to receive NSURLSession as well, so the developer can have the chance to set a NSURLCredential.

baxtheman commented 8 years ago

Thx, I stay tuned.

yuchi commented 8 years ago

We need to make it easier for devs to provide their own authentication implementation

I'm sorry, that's what I intimately meant.

javiergamarra commented 5 years ago

Closing, please reopen if necessary :)