Support client certificates

[stephanritscher]

This change adds support for configurations where the webserver requests a client certificate, e.g. via nginx configuration options ssl_client_certificate and ssl_verify_client.
The client certificate handling is done via InteractiveKeyManager which prompts for a client certificate from a file or the devices keystore.

This change is NOT about client certificate authentication to the nextcloud server instance. The regular authentication mechanisms will be used as soon as the communication on TLS level is established.

The change addresses ticket #603, while not being a generic solution IMO.

[nextcloud-android-bot]

master-IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/6889-IT-master-23-14

[nextcloud-android-bot]

stable-IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/6889-IT-stable-23-16

[AlvaroBrey]

Library PR for reference: nextcloud/android-library#1005

Hello @stephanritscher and thank you for your interest! I don't disagree with this feature in itself, but I'm not sure I am okay with the approach taken. Specifically, my concerns are:

• Relying on a third-party library, which is generally not a great idea for security stuff, and gives us less freedom when maintaining, and less control over UX. We've had some bad experiences with this already (e.g. the fido/u2f library)
• On the android-library side the InteractiveKeyManager is also added, which if I understand correctly will hold its own state, which is why there is no code to pass the certificate to the network client on the app side
• Following the previous one: we also have no control over the state of the certificates.

My initial assesment is in a nutshell, that I don't think this is a bad idea, but this is not a way of implementing it that I would approve.

cc @tobiasKaminsky @AndyScherzinger

[AndyScherzinger]

Not an expert on the topic but I think the talk Android app has support for client-certificates so I think it is worth checking how it is implemented there and I would always root for a single approach used throughout the different clients for maintenance / knowledge reasons.

[stephanritscher]

Hi @AlvaroBrey,

thanks for your comments - which I can understand in a way, but I have a slightly different way of looking at this.

My design intention for InteractiveKeyManager was a library which is easy to integrate into any app not supporting client certificates (I previously did this for dav5x which now supports client certificates as well as Munin-for-Android and monitclient) and easy to maintain the integration as patch. This boils down to maintaining certificate state in the library to reduce the API interface and to having a separate UI (which is not conform with the rest of the app). So the main points of your reasoning were decided on deliberately, but from different perspective (which I still consider valid).

My general assumption was to consider my work as private but publish it in case others find it useful (and rebasing/rebuiling the APK once or twice a year didn't hurt me). The PR the was requested of @Torqu3Wr3nch (which shows I wasn't quite wrong that this could be relevant).

Now basically you and @AndyScherzinger request a redevelopment of the feature, which is fine, and maybe I would judge the same in your position. I anticipated parts of it so I hesitated to create the PR until it was requested by a third party. Unfortunately I moved to Syncthing for (private) file sharing and Radicale3 as CardDAV/CalDAV server so I won't invest major efforts here.

Feel free to reuse code of mine, and yes, I agree with the discussion in #603 that this is an important feature (and I don't understand that only one of the nextcloud apps seems to support it).

I wish you all the best,
Stephan

[AlvaroBrey]

Hey @stephanritscher, the general consensus internally is aligned with my original message, so I'll close this. Many thanks for your interest and goodwill, however. Cheers!