schwabe / ics-openvpn

OpenVPN for Android
3.31k stars 1.2k forks source link

Alias defined in app Restriction? #1595

Open lukajandric opened 1 year ago

lukajandric commented 1 year ago

Hi,

Apologies in advance if this is not appropriate place for the request.

Is there a way to define certificate alias via App Restrictiton?

We would be adding VPN profile via MDM app. restrictions with "uuid", "name" and "ovpn" fields. Certificate and private key is also delivered and stored in the device, but user still needs to manually select the certificate alias from the store in the app. Is there any way that this can be selected automatically, maybe by additional "alias" restriction field to be set for the VPN profile?

MDM device owner (DPC) is capable of handling onChoosePrivateKeyAlias() where it can select the alias instead of user, but as-is, user again needs to call the very activity of selecting the certificate manually.

We would like to avoid any user interaction for enabling VPN. Is this possible?

Thank you and kind regards, Luka

schwabe commented 1 year ago

So you are basically looking for something like

            <restriction
                    android:key="certificate_alias"
                    android:title="@string/apprest_certalias"
                    android:defaultValue=""
                    android:description="@string/apprest_certalias_desc"
                    android:restrictionType="string"/>

So the app will use that certificate alias for the VPN configuration when importing a profile?

lukajandric commented 1 year ago

Yes, exactly! I suppose that would then get stored to that's VPNProfile.mAlias, and this would hopefully allow for VPN to work without user interaction.

schwabe commented 1 year ago

I implemented this in 3c160282b4a4d450810baa28f1a757bba82ce6bb but I currently do not have an MDM setup to actually test this change.

lukajandric commented 1 year ago

Great! It can be tested with https://play.google.com/store/apps/details?id=com.afwsamples.testdpc. I've managed to build it, we will gladly test it with our MDM setup and report here.

schwabe commented 1 year ago

I also enabled archiving of artefacts on the github actions, so this can be easier than needing to build yourself.

lukajandric commented 1 year ago

Ok, so restriction were read correctly, but our both builds were crashing.

VpnProfile remains null on new entry (as it should since it is not an update) so addCertificateAlias() fails. If I getProfileByName() before calling it and saveProfile() after calling it, it works ok.

I found one more issue. Apparently, choosePrivateKeyAlias() needs to be called with that alias before attempting the connection, otherwise it somehow fails to access the cert from Keychain. When called by pressing Select, nothing really happens, there is no selection activity popup, but description of cert changes a bit (days until expiry is shown) and then it works! So I made a workaround to call choosePrivateKeyAlias() in LaunchVPN (if alias is defined), so it would work without user needing to "selecting" it before.

Let me know if you would like pull request of the changes or something. Thanks again for the implementation!

schwabe commented 1 year ago

A pull request would be appriciated and yes I forgot about that but selecting a certificate via the choosePrivateKeyAlias() also grants permission to use that certificate. I am not sure if there is a way that a DeviceAdmin can grant this without the dialog.

lukajandric commented 1 year ago

So yes, our DPC just takes the cert preselected with choosePrivateKeyAlias() and selects it automatically by overriding onChoosePrivateKeyAlias(), so no dialog is shown and it all works automatically and seamleslly.

Small issue is that choosePrivateKeyAlias() actually needs Activity (which makes sense if user expects a dialogue, but not really when it is overrided) and I am not sure what would be best Activity in app for it. Also not sure how best to make sure it's only executed once, but I guess there is no harm in doing it on every LaunchVPN.

Also, see my fix for crash due to null new VpnProfile, it could probably be done more elegantly.

Patch file attached.

alias.patch