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

Prevent the application from allowing weak cipher suites #232

Closed
giomfo opened this issue Oct 6, 2021 · 7 comments
Closed

Prevent the application from allowing weak cipher suites #232

giomfo opened this issue Oct 6, 2021 · 7 comments
Assignees
Labels

Comments

@giomfo
Copy link
Contributor

giomfo commented Oct 6, 2021

After investigation, we have to check with the element-android platform team:

  • how to remove the "ec_point_formats" extension support on the FOSS version of element-android

  • how apply the same kind of configuration used to set up the homeserver connection (HomeServerConnectionConfig) to any others connections inside the application. In case of Tchap, on the login screen, a request is triggered to discover the right homeserver to use according to the user's email address (/info endpoint of the id server)

  • during this task can you please remove the weak cipher suites allowed for Android < 20 (if any)

if (enableCompatibilityMode) {
                    // Adopt some preceding cipher suites for Android < 20 to be able to negotiate
                    // a TLS session.
                    addAcceptedTlsCipherSuite(CipherSuite.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA);
                    addAcceptedTlsCipherSuite(CipherSuite.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA);
                }
@giomfo giomfo added the android label Oct 6, 2021
@giomfo giomfo changed the title Prevent the application from allowing weak cipher suites for all connections Prevent the application from allowing weak cipher suites Oct 6, 2021
@yostyle
Copy link
Contributor

yostyle commented Oct 6, 2021

Element has to check where these cipher suites are added (to remove them). I highlight they are not used during the second connection. The cipher suites allowed during the second connection (to the homeserver) are:

                addAcceptedTlsCipherSuite(CipherSuite.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256);
                addAcceptedTlsCipherSuite(CipherSuite.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256);
                addAcceptedTlsCipherSuite(CipherSuite.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256);
                addAcceptedTlsCipherSuite(CipherSuite.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256);
                addAcceptedTlsCipherSuite(CipherSuite.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384);
                addAcceptedTlsCipherSuite(CipherSuite.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384);
                addAcceptedTlsCipherSuite(CipherSuite.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256);
                addAcceptedTlsCipherSuite(CipherSuite.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256);

                if (enableCompatibilityMode) {
                    // Adopt some preceding cipher suites for Android < 20 to be able to negotiate
                    // a TLS session.
                    addAcceptedTlsCipherSuite(CipherSuite.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA);
                    addAcceptedTlsCipherSuite(CipherSuite.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA);
                }

Note: TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA and TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA are not used in case of Tchap, because the application is limited to Android 5.0 Lollipop (API 21) and higher

This code is not used in Tchap v2 and EA.
Now the cipher suites are setted by CertUtil.newConnectionSpecs

    fun newConnectionSpecs(hsConfig: HomeServerConnectionConfig): List<ConnectionSpec> {
        val builder = ConnectionSpec.Builder(ConnectionSpec.MODERN_TLS)
        val tlsVersions = hsConfig.tlsVersions
        if (null != tlsVersions && tlsVersions.isNotEmpty()) {
            builder.tlsVersions(*tlsVersions.toTypedArray())
        }

        val tlsCipherSuites = hsConfig.tlsCipherSuites
        if (!tlsCipherSuites.isNullOrEmpty()) {
            builder.cipherSuites(*tlsCipherSuites.toTypedArray())
        }

        @Suppress("DEPRECATION")
        builder.supportsTlsExtensions(hsConfig.shouldAcceptTlsExtensions)
        val list = ArrayList<ConnectionSpec>()
        list.add(builder.build())
        // TODO: we should display a warning if user enter an http url
        if (hsConfig.allowHttpExtension || hsConfig.homeServerUriBase.toString().startsWith("http://")) {
            list.add(ConnectionSpec.CLEARTEXT)
        }
        return list
    }

by default hsConfig.tlsCipherSuites is empty except when the app import credentials from legacy config during a migration from Riot to Element.
Otherwise cipher suites and tls version are initialized by ConnectionSpec.Builder(ConnectionSpec.MODERN_TLS) :

    @JvmField
    val MODERN_TLS = Builder(true)
        .cipherSuites(*APPROVED_CIPHER_SUITES)
        .tlsVersions(TlsVersion.TLS_1_3, TlsVersion.TLS_1_2)
        .supportsTlsExtensions(true)
        .build()

Approved cipher suites are in the following list :

    private val APPROVED_CIPHER_SUITES = arrayOf(
        // TLSv1.3.
        CipherSuite.TLS_AES_128_GCM_SHA256,
        CipherSuite.TLS_AES_256_GCM_SHA384,
        CipherSuite.TLS_CHACHA20_POLY1305_SHA256,

        // TLSv1.0, TLSv1.1, TLSv1.2.
        CipherSuite.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
        CipherSuite.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
        CipherSuite.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
        CipherSuite.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
        CipherSuite.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,
        CipherSuite.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256,

        // Note that the following cipher suites are all on HTTP/2's bad cipher suites list. We'll
        // continue to include them until better suites are commonly available.
        CipherSuite.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,
        CipherSuite.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,
        CipherSuite.TLS_RSA_WITH_AES_128_GCM_SHA256,
        CipherSuite.TLS_RSA_WITH_AES_256_GCM_SHA384,
        CipherSuite.TLS_RSA_WITH_AES_128_CBC_SHA,
        CipherSuite.TLS_RSA_WITH_AES_256_CBC_SHA,
        CipherSuite.TLS_RSA_WITH_3DES_EDE_CBC_SHA)

We should use RESTRICTED_CIPHER_SUITES :

    // Most secure but generally supported list.
    private val RESTRICTED_CIPHER_SUITES = arrayOf(
        // TLSv1.3.
        CipherSuite.TLS_AES_128_GCM_SHA256,
        CipherSuite.TLS_AES_256_GCM_SHA384,
        CipherSuite.TLS_CHACHA20_POLY1305_SHA256,

        // TLSv1.0, TLSv1.1, TLSv1.2.
        CipherSuite.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
        CipherSuite.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
        CipherSuite.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
        CipherSuite.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
        CipherSuite.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,
        CipherSuite.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256)

@yostyle
Copy link
Contributor

yostyle commented Oct 11, 2021

@giomfo
Copy link
Contributor Author

giomfo commented Oct 11, 2021

@yostyle can you please force TLS v1.3 (see #505) - to be tested in Prod first

not going to be possible: it's not even supported on the client-controlled frontend proxies
https://www.ssllabs.com/ssltest/analyze.html?d=matrix.agent.elysee.tchap.gouv.fr&hideResults=on

@giomfo
Copy link
Contributor Author

giomfo commented Nov 12, 2021

This has been fixed in Element Android (and SDK) 1.3.6
It will be fixed in Tchap-Android after the next Rebase against version v1.3.7 (#274)
@yostyle can you please check and close this issue when the rebase will be done

@giomfo giomfo assigned Florian14 and unassigned yostyle Jan 10, 2022
@giomfo
Copy link
Contributor Author

giomfo commented Jan 10, 2022

assigned to @Florian14 to check the connexion used to trigger the request to discover the right homeserver on the login screen (/info endpoint of the id server)

@Florian14
Copy link
Contributor

Tested successfully on the identity/v1/info unauthenticated endpoint. The supported client cipher suite matches with the RESTRICTED_CIPHER_SUITES shared by @yostyle. This is the first request sent by the app after it launch by trying to log into Btchap.

image

@giomfo
Copy link
Contributor Author

giomfo commented Jan 13, 2022

thx @Florian14

@giomfo giomfo closed this as completed Jan 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants