Android X (P) #2402

hannesa2 · 2018-12-19T07:10:09Z

I was forced to use Android P as well, so it's the move to P as well

About Android P, I'm gathering some changes from here that might affect the ownCloud Android app when targeting Android P:

Network TLS enabled by default: if your app targets Android 9 or higher, the isCleartextTrafficPermitted() method returns false by default. If your app needs to enable cleartext for specific domains, you must explicitly set cleartextTrafficPermitted to true for those domains in your app's Network Security Configuration. Reproducible and fix in Android X (P) #2402 (comment)
Foreground services: apps that target Android P or higher and use foreground services must request the FOREGROUND_SERVICE permission. E.g.: https://github.com/owncloud/android/blob/master/owncloudApp/src/main/java/com/owncloud/android/files/services/FileUploader.java#L306

CC @jesmrec @hannesa2

BUGS & IMPROVEMENTS

(1) Camera uploads Android X (P) #2402 (comment)

hannesa2 · 2018-12-19T07:19:30Z

Ok, there are some -alpha02 included, either

we wait till they are non alpha (just number beauty) or
we go with -alpha02 (my company uses it in production without any issues , 5.000.000+ installations) or
we uses older stable versions, not sure if it work for all libs

davigonz · 2019-01-22T15:54:14Z

Thanks a lot for this contribution @hannesa2 , we'd rather not use alpha versions, would it be possible to keep using stable versions just in the alpha ones? By the way, there's some conflicts with master.

hannesa2 · 2019-01-23T06:17:55Z

I reverted to stable libs.
But I'm not convinced, because it's just number cosmetic. After 2.0.0 a 2.0.1 will come too. On the other side, using old ones means to waiver some improvement in beta

owncloudApp/src/main/java/com/owncloud/android/ui/adapter/LogListAdapter.java

owncloudApp/build.gradle

davigonz · 2019-01-23T16:56:40Z

@hannesa2 apart from the requested changes above, I have compiled the code, install the app and I get the next error when checking the server in login view, which appears as Unknown error in the app, any idea?

2019-01-23 17:53:27.454 25087-25119/com.owncloud.android.debug E/GetRemoteStatusOperation: Connection check at http://192.168.0.173:10000: Unrecovered transport exception
    java.net.UnknownServiceException: CLEARTEXT communication to 192.168.0.173 not permitted by network security policy
        at okhttp3.internal.connection.RealConnection.connect(RealConnection.java:147)
        at okhttp3.internal.connection.StreamAllocation.findConnection(StreamAllocation.java:257)
        at okhttp3.internal.connection.StreamAllocation.findHealthyConnection(StreamAllocation.java:135)
        at okhttp3.internal.connection.StreamAllocation.newStream(StreamAllocation.java:114)
        at okhttp3.internal.connection.ConnectInterceptor.intercept(ConnectInterceptor.java:42)

Maybe we need a security config file now, can you try with the next code?

In res/xml/network_security_config.xml:

<?xml version="1.0" encoding="utf-8"?>
<network-security-config>
    <base-config cleartextTrafficPermitted="true" />
</network-security-config>

In AndroidManifest.xml

android:networkSecurityConfig="@xml/network_security_config"

hannesa2 · 2019-01-23T18:23:43Z

Indeed this cleartextTrafficPermitted is an issue, I didn't measured it, because I use https.
On the other hand, who uses http needs no privacy, this person can upload files to Google photos too

hannesa2 · 2019-01-23T18:30:17Z

Removing this http communication lock is a move in one direction.
But a move in the other direction could be an introduce of (dynamic) cert pinning which prevents from a man-in-the-middle attacks -just as an idea-

michaelstingl · 2019-01-23T19:21:54Z

But a move in the other direction could be an introduce of (dynamic) cert pinning which prevents from a man-in-the-middle attacks -just as an idea-

👍

@hannesa2 you mean like in #934 ? Or better open another issue to discuss details?

hannesa2 · 2019-01-23T21:33:33Z

Exactly #934 ! (but I it's almost 3 years just an idea)
I will not promise anything, but maybe I can deliver something, when I generalize similar somewhere else

And one more additional step could be a 2 phase authentication, like Google does when a new IP/device tries to connect with a mail notification/confirmation. But with this, the backend needs some work. But to be honest, this needs too much live/spare time from my side.

When it comes to my focus I can open a ticket for sure, but currently my main interest is the non working (already closed) upload #2394
Btw you can open a ticket too, if you see a sense to have an additional ticket

hannesa2 · 2019-01-23T21:51:53Z

What should be done here ?
Allow http and be more un-secure or follow the Goggle suggestion and only allow https and do nothing ?
@davigonz @jesmrec @michaelstingl

davigonz · 2019-01-24T08:40:18Z

@hannesa2 @michaelstingl @jesmrec

What should be done here ?
Allow http and be more un-secure or follow the Goggle suggestion and only allow https and do nothing ?

I would not get rid of http IMHO, is insecure for sure but ownCloud is a self-hosted solution and therefore, each end user/company should be the responsible for the security of his ownCloud instance.

Disallowing http will likely trigger complains since we are making decisions that do not belong to us.

But a move in the other direction could be an introduce of (dynamic) cert pinning which prevents from a man-in-the-middle attacks -just as an idea-

About this, I think that we are already checking the SSL certificate against a stored one, as is suggested in #934. Have a look at https://github.com/owncloud/android-library/blob/940f0b9bb042e3c318e4cbeff71782e0d8733f93/owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/http/HttpClient.java#L78.

davigonz · 2019-01-29T16:57:39Z

@hannesa2 can you please apply the changes about http and foreground services so we can push this to be included in 2.10.0? Thanks

hannesa2 · 2019-01-30T05:37:07Z

http :
I followed https://developer.android.com/training/articles/security-config and applied the default configuration for apps targeting Android 6.0 (API level 23) and lower

hannesa2 · 2019-01-30T06:19:48Z

Foreground Service:

I added the missing ones, but to be honest I did only code changes and didn't test it.

In following files, in my point of view, there are too much conditions to run into startForeground() but you already have it on master, so I didn't changed it.
This would be ok
Build.VERSION.SDK_INT >= Build.VERSION_CODES.O but the other ones ?

FileDownloader.java
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O && (isAvailableOfflineFile || retryDownload))

FileUploader.java
if ((createdBy == CREATED_AS_CAMERA_UPLOAD_PICTURE || createdBy == CREATED_AS_CAMERA_UPLOAD_VIDEO || isAvailableOfflineFile || isRequestedFromWifiBackEvent) && Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {

davigonz · 2019-01-30T10:58:46Z

FileDownloader.java
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O && (isAvailableOfflineFile || retryDownload))

You have a comment just below that line that explains why do we need those checks. We create the service to check available offline files and retry downloads by using startForegroundService and we have to call startForeground within five seconds only for those cases.

android/owncloudApp/src/main/java/com/owncloud/android/files/services/FileDownloader.java

Lines 193 to 197 in 2b15b5b

    
                       /** 
        
                        * We have to call this within five seconds after the service is created with startForegroundService when: 
        
                        * - Checking available offline files in background 
        
                        * - Retry downloads in background, e.g. when recovering wifi connection 
        
                        */

if ((createdBy == CREATED_AS_CAMERA_UPLOAD_PICTURE || createdBy == CREATED_AS_CAMERA_UPLOAD_VIDEO || isAvailableOfflineFile || isRequestedFromWifiBackEvent) && Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {

Have a look at the comment below:

android/owncloudApp/src/main/java/com/owncloud/android/files/services/FileUploader.java

Lines 300 to 304 in 2b15b5b

    
                       /** 
        
                        * After calling startForegroundService method from {@link TransferRequester} for camera uploads or 
        
                        * available offline, we have to call this within five seconds after the service is created to avoid 
        
                        * an error 
        
                        */

hannesa2 · 2019-02-11T19:06:12Z

Can you check that @jesmrec ?

My opinion is, it's just necessary to test latest commit

davigonz · 2019-02-12T08:26:59Z

I agree, 72a4297 fixes now the e07901b.
I can squash (together with introduce 6dd0a4c and revert d84f0bf) it on request to summarize necessary changes, but in review you see summarize too

@hannesa2 the startForegroundService stuff is still in this PR, can you have a look at it? Thanks

hannesa2 · 2019-02-12T08:29:53Z

please test it first. Without it, we've seen a crash #2402 (comment)

davigonz · 2019-02-12T08:33:08Z

please test it first. Without it, we've seen a crash #2402 (comment)

As I explained in #2402 (comment), I tested without it and I'm not getting that crash but the notification error.

@jesmrec can you please test af756ba and confirm there's no crash there? If so, we can remove the startForegroundService stuff

jesmrec · 2019-02-12T11:22:55Z

With commit 4064941:

Installation: OK
URL check and authentication: OK
Camera uploads: Fails with Android X (P) #2402 (comment)

With commit 6ed4d30

Installation: OK
URL check and authentication: OK
Camera uploads: Fails. In this case, pics are not either enqueued (in the previous one, they were). I stumbled upon this trace:

2019-02-12 11:01:29.270 22838-22838/? E/AndroidRuntime: FATAL EXCEPTION: main
Process: com.owncloud.android.debug, PID: 22838
java.lang.RuntimeException: Unable to start service com.owncloud.android.files.services.FileUploader@a120a3d with Intent { cmp=com.owncloud.android.debug/com.owncloud.android.files.services.FileUploader (has extras) }: java.lang.SecurityException: Permission Denial: startForeground from pid=22838, uid=10482 requires android.permission.FOREGROUND_SERVICE
at android.app.ActivityThread.handleServiceArgs(ActivityThread.java:3687)

with commit 72a4297 (the latest one)

Installation: OK
URL check and authentication: OK
Camera uploads: Works fine!!!
Additional check: av, offline files: OK

so, good news. In any case, a deeper testing should be performed to assure all scenarios are OK, but this is out of the scope of the current one.

jesmrec · 2019-02-12T15:00:12Z

@hannesa2 i have rechecked the whole feature by dropping the commit c2db38e, and it works properly. No crashes in authentication and uploads run correctly (camera uploads and manual uploads).

Maybe we can get rid of such commit, so it does not introduce anything sensible for the described feature, and will make a cleaner history.

Sorry for the noise.

hannesa2 · 2019-02-12T19:29:42Z

ok, I removed the

introduce
revert
re-intoduce

commit, and kept all except my fix. (To be honest, because it worked for me before, I simply followed your suggestion and didn't tested it)
@jesmrec please take over

jesmrec · 2019-02-13T11:19:21Z

After a new check:

Services job-based (camera uploads and av. offline syncing) are working properly in Android 9
Same checks works fine in Android 8 and 7.

We can move forward, and new checks will be performed in the following release stage.

Approved

jesmrec · 2019-02-13T11:20:05Z

Please, update the branch for the final merge

attempt to fix upload

hannesa2 · 2019-02-13T16:32:36Z

Sounds good ! Rebase is done

This was referenced Dec 19, 2018

Android P #2283

Closed

Android X #2399

Closed

hannesa2 force-pushed the AndroidX branch from b5b97d1 to b4caf79 Compare December 19, 2018 18:58

hannesa2 force-pushed the AndroidX branch from b4caf79 to c97274d Compare December 27, 2018 12:00

davigonz assigned hannesa2 Jan 2, 2019

hannesa2 force-pushed the AndroidX branch from c97274d to 3e08316 Compare January 2, 2019 16:35

hannesa2 force-pushed the AndroidX branch 4 times, most recently from 2eea61a to 3c2c05f Compare January 15, 2019 18:35

hannesa2 force-pushed the AndroidX branch from 3c2c05f to 739e86e Compare January 17, 2019 18:57

hannesa2 force-pushed the AndroidX branch from 739e86e to 22782ac Compare January 23, 2019 06:14

davigonz suggested changes Jan 23, 2019

View reviewed changes

owncloudApp/src/main/java/com/owncloud/android/ui/adapter/LogListAdapter.java Show resolved Hide resolved

owncloudApp/build.gradle Outdated Show resolved Hide resolved

hannesa2 force-pushed the AndroidX branch from 3ff60dd to 7b27105 Compare January 23, 2019 21:49

hannesa2 mentioned this pull request Jan 30, 2019

Allow User-added-CAs in Android 7. #2000

Closed

hannesa2 force-pushed the AndroidX branch from 045a23a to 1d84340 Compare January 30, 2019 20:46

hannesa2 force-pushed the AndroidX branch from 51cc9e2 to 16e0a28 Compare February 12, 2019 19:26

hannesa2 force-pushed the AndroidX branch 3 times, most recently from a404aeb to a111f26 Compare February 13, 2019 05:56

jesmrec self-requested a review February 13, 2019 11:19

jesmrec added the Approved by qa label Feb 13, 2019

jesmrec approved these changes Feb 13, 2019

View reviewed changes

hannesa2 added 7 commits February 13, 2019 17:31

Android P

8c08b9b

AndroidX

4415952

apply reviews

ac0d0df

use network security from Android 6.0 (api23)

02eef92

remove a lint warning

2015eca

add android.permission.FOREGROUND_SERVICE

2cca667

fix forground crash

70fbd7f

attempt to fix upload

hannesa2 force-pushed the AndroidX branch from a111f26 to 70fbd7f Compare February 13, 2019 16:32

davigonz merged commit fba789e into owncloud:master Feb 13, 2019

davigonz mentioned this pull request Feb 13, 2019

Android Studio 3.3.1 #2438

Merged

hannesa2 deleted the AndroidX branch February 13, 2019 17:47

jesmrec mentioned this pull request Feb 14, 2019

2.10.0 beta v1 #2449

Merged

hannesa2 mentioned this pull request Aug 10, 2020

Fatal Exception: Not allowed to start service Intent #2911

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Android X (P) #2402

Android X (P) #2402

hannesa2 commented Dec 19, 2018 •

edited by jesmrec

Loading

hannesa2 commented Dec 19, 2018 •

edited

Loading

davigonz commented Jan 22, 2019 •

edited

Loading

hannesa2 commented Jan 23, 2019

davigonz commented Jan 23, 2019 •

edited

Loading

hannesa2 commented Jan 23, 2019

hannesa2 commented Jan 23, 2019

michaelstingl commented Jan 23, 2019

hannesa2 commented Jan 23, 2019

hannesa2 commented Jan 23, 2019 •

edited

Loading

davigonz commented Jan 24, 2019 •

edited

Loading

davigonz commented Jan 29, 2019

hannesa2 commented Jan 30, 2019

hannesa2 commented Jan 30, 2019

davigonz commented Jan 30, 2019

hannesa2 commented Feb 11, 2019

davigonz commented Feb 12, 2019 •

edited

Loading

hannesa2 commented Feb 12, 2019

davigonz commented Feb 12, 2019

jesmrec commented Feb 12, 2019

jesmrec commented Feb 12, 2019

hannesa2 commented Feb 12, 2019 •

edited

Loading

jesmrec commented Feb 13, 2019

jesmrec commented Feb 13, 2019

hannesa2 commented Feb 13, 2019

Android X (P) #2402

Android X (P) #2402

Conversation

hannesa2 commented Dec 19, 2018 • edited by jesmrec Loading

hannesa2 commented Dec 19, 2018 • edited Loading

davigonz commented Jan 22, 2019 • edited Loading

hannesa2 commented Jan 23, 2019

davigonz commented Jan 23, 2019 • edited Loading

hannesa2 commented Jan 23, 2019

hannesa2 commented Jan 23, 2019

michaelstingl commented Jan 23, 2019

hannesa2 commented Jan 23, 2019

hannesa2 commented Jan 23, 2019 • edited Loading

davigonz commented Jan 24, 2019 • edited Loading

davigonz commented Jan 29, 2019

hannesa2 commented Jan 30, 2019

hannesa2 commented Jan 30, 2019

davigonz commented Jan 30, 2019

hannesa2 commented Feb 11, 2019

davigonz commented Feb 12, 2019 • edited Loading

hannesa2 commented Feb 12, 2019

davigonz commented Feb 12, 2019

jesmrec commented Feb 12, 2019

jesmrec commented Feb 12, 2019

hannesa2 commented Feb 12, 2019 • edited Loading

jesmrec commented Feb 13, 2019

jesmrec commented Feb 13, 2019

hannesa2 commented Feb 13, 2019

hannesa2 commented Dec 19, 2018 •

edited by jesmrec

Loading

hannesa2 commented Dec 19, 2018 •

edited

Loading

davigonz commented Jan 22, 2019 •

edited

Loading

davigonz commented Jan 23, 2019 •

edited

Loading

hannesa2 commented Jan 23, 2019 •

edited

Loading

davigonz commented Jan 24, 2019 •

edited

Loading

davigonz commented Feb 12, 2019 •

edited

Loading

hannesa2 commented Feb 12, 2019 •

edited

Loading