mumble-voip / mumble

Mumble is an open-source, low-latency, high quality voice chat software.
https://www.mumble.info
Other
6.41k stars 1.12k forks source link

Skype's click-to-call feature creates an enormous amount of certificates in the Root CA store #1271

Closed mkrautz closed 10 years ago

mkrautz commented 10 years ago

Skype's click-to-call feature (which is optional, and can be disabled at install time) is causing trouble for quite a few Mumble users by flooding their Root CA store with an enormous amount of certificates

This can cause Mumble to be unable to connect to any servers, simply giving "Remote host closed the connection" as the reason. It seems like the TLS handshake stalls, and eventually times out, while Mumble is trying to shuffle all the certs in the Root CA store around in order to perform verification.

See the following for more info:

http://mumble.sourceforge.net/1.2.6_Known_Issues#Cannot_connect_to_any_Mumble_servers.2C_.22remote_host_closed_connection.22

Special thanks to Mumble.com/GameVox.com for troubleshooting and proposing a fix for this issue!

mkrautz commented 10 years ago

Proposed fix: http://i.imgur.com/HEWTMKV.png

mkrautz commented 10 years ago

Targetting this for 1.2.7.

Will land in master as soon as a patch is agreed upon.

mkrautz commented 10 years ago

I think the first step in fixing this should be to create a reproducer for this issue by inserting 'fake' Root CA certs into a user's Root CA store. (Or if that doesn't work, I supose we'll have to do it in the LOCAL MACHINE store.)

the-hive commented 10 years ago

Slightly revised suggested code:

    // remove Skype certs and limit to 500 max
    {
        QSslConfiguration sslCfg = QSslConfiguration::defaultConfiguration();
        QList<QSslCertificate> ca_list = sslCfg.caCertificates();
        ca_list += QSslCertificate::fromData("CaCertificates");
        QList<QSslCertificate> resultList;
        foreach ( QSslCertificate cert, ca_list )
        {
            if ( !cert.subjectInfo(QSslCertificate::Organization).contains( QLatin1String("Skype"), Qt::CaseInsensitive ) )
                resultList.append( cert );
            if ( resultList.size() == 500 )
                break;
        }
        sslCfg.setCaCertificates( resultList );
        QSslConfiguration::setDefaultConfiguration( sslCfg );
        qWarning( "CA certs: Using %d of %d", resultList.size(), ca_list.size() );
    }

I would suggest this be put in main.cpp before any SSL connections are created. (Including any use of QNetworkAccessManager.)

(ps. I'm a GameVox dev.)

mkrautz commented 10 years ago

The proposed fix does not work for me.

In Mumble, when the client enters SeverHandler::run() and constructs its QSslSocket, the default QSslConfiguration on it is no longer the that was set as the default in the snippet above.

No obvious places in Mumble that'd do that, though.

mkrautz commented 10 years ago

Oh, and here's the reproducer (.bat):

call "c:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\vcvarsall.bat"
for /l %i in (0,1,5000000000000000) do makecert -sr localmachine -ss ROOT -sk ROOTKEYS -r -n "CN=localhost,O=Skype" -sky exchange -sp "Microsoft RSA SChannel Cryptographic Provider" -sy 12 -len 4096
mkrautz commented 10 years ago

Will dig deeper tomorrow.

mkrautz commented 10 years ago

We still have src/SSL.cpp. Obviously, that's where this happens, and the filtering should be happening. :-)

mkrautz commented 10 years ago

Fix committed as 7141a05c0e355806c0db74a9286a69bf51e698f3 in master; aef3509196f31747fee94eb9a39b793c3e9fe0f6 in 1.2.x.

mkrautz commented 10 years ago

Fixed in 1.2.7!