Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🔑 Support for client certificates #603

Closed
mlsxlist opened this issue Jan 29, 2017 · 58 comments
Closed

🔑 Support for client certificates #603

mlsxlist opened this issue Jan 29, 2017 · 58 comments
Assignees
Labels
connectivity DNS, TLS, proxies, network connection, etc. related matter enhancement feature: authentication Authentication or accounts related security 🍀 2024-Spring

Comments

@mlsxlist
Copy link

Actual behaviour

In order to secure Nextcloud on TLS level, it would be good if the app could support client certificates. If the client certificate is not sent on handshake, the server prevents access to Nextcloud logon page. This would provide a second line of defense.

Expected behaviour

Nextcloud app should support client certificates as other apps like caldav sync and carddav sync already do.

@ghost
Copy link

ghost commented Jan 30, 2017

Hello mlsxlist,

if this is of any use to the case, there is an open issue about this over at the owncloud github site:

owncloud/android#163

Best wishes
riesnerd

@tobiasKaminsky
Copy link
Member

I would vote this "medium" to get this done when we (@mario or me) have time.
Everything that enhances the security of the app/nextcloud should have priority.
cc @mario ?

@ghost
Copy link

ghost commented Feb 1, 2017

Okay thanks! In some way this also enhances security. ;)

@mario
Copy link
Contributor

mario commented Feb 1, 2017

@tobiasKaminsky let me think about it, and I'll come back to you - we need to focus on fixing current issues first to make the app (more) usable than it is now - thought definitely this is something we want to do in the future.

@seanlynch
Copy link

I would also very much like to see this, because it protects from potential issues with the login page as well as weak passwords and password guessing attacks.

@lucacarangelo
Copy link

Any news on this ? This is a very important enhancement !

@AndreasMettlen
Copy link

@mario let me know if you still need a test setup with client certificates enabled. In the meantime I will try to look into this (I haven't worked on android apps yet, but I can find my way around in php/java/python so I will give it a try)

@mario
Copy link
Contributor

mario commented Feb 25, 2018

@AndreasMettlen yes I do. Send me the required cert + url, server and pass to mario@nextcloud.com :)

@AndreasMettlen
Copy link

@mario Did you have a chance yet to look into this ?

@AndreasMettlen
Copy link

@mario Did the second certificate and the talk app help you in making progress on this issue ?
Anything I can do to support ?

@mario
Copy link
Contributor

mario commented Apr 24, 2018

@AndreasMettlen I implemented the initial support for client cert in Talk app. Now I need to validate it works which is why I asked for the second one. I'll try to do that on Friday.

If it works, I can see how easy/hard it is to put it into the Files (this) app.

@mbrescia
Copy link

mbrescia commented May 15, 2018

Hi,
I really look forward for this enhancement.
In the meantime I've tried to patch the code on Android and it kinda works... even if it's ugly as hell 😈
I've used the Android KeyChain features to allow the user to select a PKCS12 certificate and then stored the cert alias into the app preferences:

AuthenticatorActivity

private void checkOcServer() {
        if (mHostUrlInput != null) {
            uri = mHostUrlInput.getText().toString().trim();
            mOkButton.setEnabled(false);
            showRefreshButton(false);
        } else {
            uri = mServerInfo.mBaseUrl;
        }

        mServerIsValid = false;
        mServerIsChecked = false;
        mServerInfo = new GetServerInfoOperation.ServerInfo();

        if (uri.length() != 0) {
            if (mHostUrlInput != null) {
                uri = AuthenticatorUrlUtils.stripIndexPhpOrAppsFiles(uri);
                mHostUrlInput.setText(uri);
            }

            // Handle internationalized domain names
            try {
                uri = DisplayUtils.convertIdn(uri, true);
            } catch (IllegalArgumentException ex) {
                // Let Owncloud library check the error of the malformed URI
                Log_OC.e(TAG, "Error converting internationalized domain name " + uri, ex);
            }

            if (mHostUrlInput != null) {
                mServerStatusText = getResources().getString(R.string.auth_testing_connection);
                mServerStatusIcon = R.drawable.progress_small;
                showServerStatus();
            }

            if (NetworkUtils.alias == null) {
                KeyChain.choosePrivateKeyAlias(this, (KeyChainAliasCallback) this, null, null, uri, -1, null);
            } else {
                startServerInfoIntent();
            }

        } else {
            mServerStatusText = "";
            mServerStatusIcon = 0;
            if (!webViewLoginMethod) {
                showServerStatus();
            }
        }
    }
public void alias(final String alias) {
        System.out.println("THREAD: " + Thread.currentThread().getName());
        System.out.println("Selected alias: " + alias);
        try {
            SharedPreferences sp = PreferenceManager.getDefaultSharedPreferences(this.getApplicationContext());
            SharedPreferences.Editor editor = sp.edit();
            editor.putString("TLS_ALIAS", alias);
            editor.commit();

            startServerInfoIntent();
        } catch (Exception e) {
            e.printStackTrace();
        }
    }

Then it uses the cert to initialize the SslSocketFactory:

NetworkUtils (nextcloud-android-library)

public static AdvancedSslSocketFactory getAdvancedSslSocketFactory(Context context) 
    		throws GeneralSecurityException, IOException {

        if (mAdvancedSslSocketFactory  == null) {

            if(chain == null || pk == null || alias == null) {
                // try to recover from preferences
                SharedPreferences sp = PreferenceManager.getDefaultSharedPreferences(context);
                alias = sp.getString("TLS_ALIAS", "");
                try {
                    chain = KeyChain.getCertificateChain(context, alias);
                    pk = KeyChain.getPrivateKey(context, alias);
                } catch (KeyChainException e) {
                    e.printStackTrace();
                } catch (InterruptedException e) {
                    e.printStackTrace();
                } catch (NullPointerException e) {
                    e.printStackTrace();
                }
            }

            if(chain != null) {
                KeyStore trustStore = KeyStore.getInstance(KeyStore.getDefaultType());

                X509ExtendedKeyManager keyManager = new X509ExtendedKeyManager() {

                    @Override
                    public String chooseClientAlias(String[] strings, Principal[] principals, Socket socket) {
                        return alias;
                    }

                    @Override
                    public String chooseServerAlias(String s, Principal[] principals, Socket socket) {
                        return alias;
                    }

                    @Override
                    public X509Certificate[] getCertificateChain(String s) {
                        return chain;
                    }

                    @Override
                    public String[] getClientAliases(String s, Principal[] principals) {
                        return new String[]{alias};
                    }

                    @Override
                    public String[] getServerAliases(String s, Principal[] principals) {
                        return new String[]{alias};
                    }

                    @Override
                    public PrivateKey getPrivateKey(String s) {
                        return pk;
                    }
                };

                TrustManagerFactory trustFactory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
                trustFactory.init(trustStore);
                TrustManager[] trustManagers = trustFactory.getTrustManagers();

                X509TrustManager[] tm = new X509TrustManager[] { new AdvancedX509TrustManager(trustStore) {

                    public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException {
                    }

                    public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException {
                    }

                    public X509Certificate[] getAcceptedIssuers() {
                        return chain;
                    }

                    public boolean isClientTrusted(X509Certificate[] arg0) {
                        return true;
                    }

                    public boolean isServerTrusted(X509Certificate[] arg0) {
                        return true;
                    }

                } };

                SSLContext sslContext = SSLContext.getInstance("TLS");
                sslContext.init(new KeyManager[] {keyManager}, tm, null);
                SSLContext.setDefault(sslContext);

                mHostnameVerifier = new BrowserCompatHostnameVerifier();
                mAdvancedSslSocketFactory = new AdvancedSslSocketFactory(sslContext, new AdvancedX509TrustManager(trustStore), mHostnameVerifier);

            } else {
                // can't recover the chain
                ...
            }
        }
        return mAdvancedSslSocketFactory;
    }

That's a big paste-up of various articles and posts that I've read to try to solve the problem... unfortunately I can't remember all of them:
owncloud/android#163
kbremner

Hope it helps.
Best wishes

@mario
Copy link
Contributor

mario commented May 15, 2018

@mbrescia feel free to start a patch, and I can help? In the mean time, maybe you can try Nextcloud Talk v1.2.0beta? Same for you @AndreasMettlen ^_^

@ClCfe
Copy link

ClCfe commented Jun 7, 2018

Hello @mario
i just updated to 2.0.0 from play store
I ve got this error as soon as I enter the server address
attempt to invoke virtual method java.lang.String com.nextcloud.talk.models.database.UserEntity.getClientCertificate() on a null object reference

I have android 8.1.0

@mario mario closed this as completed Jun 7, 2018
@mario
Copy link
Contributor

mario commented Jun 7, 2018

@ClCfe can you file a bug here, preferably with stacktrace?

https://github.com/nextcloud/talk-android

@proton2b
Copy link

proton2b commented Jul 5, 2018

@mbrescia
is there any chance that you upload your whole example, because i tried your workaround but without success.
best regards

@mbrescia
Copy link

@proton2b
Sure, I'll start a branch as soon as I can

@proton2b
Copy link

@mbrescia
ok great, thanks in advance!

@carlos-sarmiento

This comment has been minimized.

@proton2b

This comment has been minimized.

@mirko

This comment has been minimized.

@steffenfritz

This comment has been minimized.

@ben423423n32j14e

This comment has been minimized.

@TJB-99

This comment has been minimized.

@bestrocker221
Copy link

Any news on the topic? Still not implemented but would be awesome to have!

@jancborchardt jancborchardt moved this to 🧭 Planning evaluation / ideas in 🖍 Design team Dec 21, 2023
@tobiasKaminsky tobiasKaminsky moved this from 🧭 Planning evaluation (don't pick) to 📄 To do (max 2 entries / member) in 🤖 🍏 Mobile clients team Dec 21, 2023
@github-project-automation github-project-automation bot moved this to 🧭 Planning evaluation (don't pick) in 🤖 🍏 Mobile clients team Dec 21, 2023
@tobiasKaminsky tobiasKaminsky self-assigned this Jan 15, 2024
@tobiasKaminsky tobiasKaminsky moved this from 📄 To do (max 2 entries / member) to 🏗️ In progress in 🤖 🍏 Mobile clients team Jan 15, 2024
@tobiasKaminsky tobiasKaminsky moved this from 🏗️ In progress to 📄 To do (max 2 entries / member) in 🤖 🍏 Mobile clients team Jan 17, 2024
@AndyScherzinger AndyScherzinger changed the title Support for client certificates 🔑 Support for client certificates Jan 18, 2024
@tobiasKaminsky tobiasKaminsky moved this from 📄 To do (max 2 entries / member) to 🏗️ In progress in 🤖 🍏 Mobile clients team Jan 23, 2024
@tobiasKaminsky
Copy link
Member

Done with
#12408
nextcloud/android-library#1308

Huge thanks to @Elv1zz who did 99,95% of the work 🎉

@bestrocker221
Copy link

Any plan on adding it to release?

@AndyScherzinger AndyScherzinger added this to the Nextcloud App 3.29.0 milestone Mar 17, 2024
@AndyScherzinger
Copy link
Member

It's planned to ship it with the next feature release 3.29 - scheduled for 24th April

@kaikli
Copy link

kaikli commented Sep 27, 2024

Maybe I'm just blind, but how can I specify a client certificate when signing in with the android app? Is this a hidden feature that is only enabled when a client certificate is required or how does it work?

I'm using 3.30.0 and how I understand it the feature should be added with 3.29.0.

When I add a new account, I only see two options to enter the url of the server, manually or scan the configuration with a qr code. After that a browser is opened to sign into nextcloud.

@peshovec
Copy link

peshovec commented Sep 27, 2024 via email

@Elv1zz
Copy link
Contributor

Elv1zz commented Sep 27, 2024

Maybe I'm just blind, but how can I specify a client certificate when signing in with the android app? Is this a hidden feature that is only enabled when a client certificate is required or how does it work?

When I add a new account, I only see two options to enter the url of the server, manually or scan the configuration with a qr code. After that a browser is opened to sign into nextcloud.

Yes, exactly. There is no direct way to configure the nextcloud connection, so especially no explicit selection of the certificate before trying to establish the connection. The certificate selection dialog appears when the server asks for a certificate. That's the way nexctloud connections are configured (so far). I personally prefer an explicit configuration, like e.g. DAVx uses it.
The (proxy) server has to request the client certificate for the HTTPS connection -- it can either be required or optional on the server side. However, at the moment a mixed operation of clients with and without client certificates does not seem to work.

@superamnesia
Copy link

The android app works as expected but iOS one doesn’t work. Have you already plan a release?
And for the desktop apps?

@tobiasKaminsky
Copy link
Member

Please ask in the corresponding repositories for status.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
connectivity DNS, TLS, proxies, network connection, etc. related matter enhancement feature: authentication Authentication or accounts related security 🍀 2024-Spring
Projects
Development

No branches or pull requests