Project

General

Profile

Bug #3583

Android App - No clear error when failing to import VPN profile with invalid UUID

Added by Jason Congdon about 2 months ago. Updated 26 days ago.

Status:
Closed
Priority:
Normal
Category:
android
Target version:
Start date:
Due date:
Estimated time:
Affected version:
5.9.0
Resolution:
Fixed

Description

Android OS: 9
Strongswan APK: 2.3.0

`testProfile.sswan` created according to https://wiki.strongswan.org/projects/strongswan/wiki/AndroidVPNClientProfiles

When trying to import a VPN profile when in the app the error message `Failed to import VPN profile` is displayed.
When selecting the same file via a file browser, the app briefly opens, the error message `Failed to import VPN profile: File not found` is displayed and the app closes.

I can create the profile in the app by manually entering the server address and selecting a pre-installed User certificate and leaving the the CA certificate option set to `Select automatically`.
But entering the same server details and `base64` of the User certificate for the `local.p12` into a profile file has not proven fruitful.
My `testProfile.sswan` contains the following:

{
    "uuid": "54b1e56c62544cc2937b8e20fb2d3e96",
    "name": "TestProfileCertificate",
    "type": "ikev2-cert",
    "remote": {
        "addr": "azuregateway..." 
    },
    "local": {
        "p12": "MIIN..." 
    }
}

Associated revisions

Revision ec317c29 (diff)
Added by Tobias Brunner 26 days ago

android: Throw an exception if UUID can't get parsed

The parser is quite picky and e.g. doesn't accept UUIDs without dashes.
Even without a specific error, this at least points the users into the
right direction.

Fixes #3583.

History

#1 Updated by Tobias Brunner about 2 months ago

  • Status changed from New to Feedback

When trying to import a VPN profile when in the app the error message `Failed to import VPN profile` is displayed.
When selecting the same file via a file browser, the app briefly opens, the error message `Failed to import VPN profile: File not found` is displayed and the app closes.

This exception occurs when the app tries to open the file/content URL provided by the system. Maybe some limitation on your system (e.g. some security settings).

#2 Updated by Jason Congdon about 2 months ago

I just checked permissions for the app. There was a listing for Storage, which was not enabled. At no time was I prompted for Storage access. I enabled the permission and am getting the same errors messages.

#3 Updated by Tobias Brunner about 2 months ago

There was a listing for Storage, which was not enabled. At no time was I prompted for Storage access.

Neither should it be required (at least in terms of Android's own permissions). That's the whole point of using the SAF, which is particularly the case when opening a file from the app as that uses the file browser provided by that framework.

I enabled the permission and am getting the same errors messages.

Perhaps the app that actually stores the file needs to explicitly allow access to it. No idea.

#4 Updated by Jason Congdon about 2 months ago

There is no app storing the file... it is located in "SDCard" space

#5 Updated by Tobias Brunner about 2 months ago

There is no app storing the file... it is located in "SDCard" space

Hm, the app has the android.permission.READ_EXTERNAL_STORAGE permission in its manifest, which might be required for this (it was on in some scenarios on old Android systems). I think on newer Android systems this permission has to be requested explicitly from the user at runtime, but I've never implemented that because it seemed unnecessary on those systems (at least I wasn't able to produce a situation where it was). It might still be required on your device.

#6 Updated by Jason Congdon about 2 months ago

Ok. Well, I've enabled it. And it still is not working.

#7 Updated by Tobias Brunner about 2 months ago

Ok. Well, I've enbaled it. And it still is not working.

Sorry, no idea.

#8 Updated by Jason Congdon about 2 months ago

I assume this is something that Andreas or Martin would know more about?

#9 Updated by Tobias Brunner about 2 months ago

I assume this is something that Andreas or Martin would know more about?

Why would you think that?

#10 Updated by Jason Congdon about 2 months ago

Tobias Brunner wrote:

Sorry, no idea.

'Cause that statement felt like a ¯\_(ツ)_/¯ - a little dismissive
I was thinking maybe you weren't the one that worked on the Android side of the application.
So I figured I'd get the source and see if I can figure out why it's failing for me.
Do so, I see your name in the repo as Author, so maybe you are - IDK.
I didn't mean anything by it, just trying to figure out how to get my issue resolved is all. :)

#11 Updated by Tobias Brunner about 2 months ago

'Cause that statement felt like a ¯\_(ツ)_/¯ - a little dismissive

Which is how I felt :)

I didn't mean anything by it, just trying to figure out how to get my issue resolved is all. :)

No problem, just wondered. When a profile is imported from the app, it basically does what the best practices recommend regarding opening non-media files (see source:src/frontends/android/app/src/main/java/org/strongswan/android/ui/VpnProfileImportActivity.java#L215). Not sure if intent.addCategory(Intent.CATEGORY_OPENABLE); is just a filter or actually causes content providers to behave differently. Maybe you simply wouldn't see some of the files/folders that can't be opened. (We don't use openFileDescriptor() but openInputStream(), which is not explicitly mentioned in the docs for CATEGORY_OPENABLE but I guess opening an FD is implied for an input stream.) I could release a test version of the app with that attribute set next week if you want to try it out (send me the email address of your Play account and afterwards you can opt-in to the test track via the link on AndroidVPNClient to install it).

#12 Updated by Jason Congdon about 2 months ago

So, it took some work, but I was able to get the source into Android Studio and compile it.
Using Logcat I was able to see that it did not like the UUID. The UUID generated I have srips out the `-` for some reason (I can't remember why it was set up that way)
I then updated the UUID with the dashes and it imports correctly.
It was user error. Though, it would have been an added bonus if that error was handled and displayed to the user. :)

#13 Updated by Tobias Brunner about 2 months ago

I then updated the UUID with the dashes and it imports correctly.
It was user error. Though, it would have been an added bonus if that error was handled and displayed to the user. :)

Interesting. You should not have received a "File not found" error if that was the case (I guess that could have been something else when you were opening the file from an external app and not via file picker from within our app). Some syntax errors in profile files (e.g. invalid proposals or traffic selectors) are explicitly handled, the import dialog then shows which field was incorrectly formatted. But as you saw, for invalid UUIDs that's currently not the case and you'll just get a generic error. I'll add one for the next release of the app.

It's a bit unfortunate that Java's UUID class can't parse these without dashes, but at least it's documented. The API doc explicitly says it parses the format toString() produces, which is the one defined in RFC 4122 (also mentioned on AndroidVPNClientProfiles) and that includes dashes. Additionally, we could maybe also add some dashes if they are missing or parse a dashless UUID directly as two unsigned integers and use the UUID constructor.

#14 Updated by Tobias Brunner 26 days ago

  • Tracker changed from Issue to Bug
  • Subject changed from Android App - Failed to import VPN profile to Android App - No clear error when failing to import VPN profile with invalid UUID
  • Status changed from Feedback to Closed
  • Assignee set to Tobias Brunner
  • Target version set to 5.9.1
  • Resolution set to Fixed

Also available in: Atom PDF