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

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

Closed
mkrautz opened this issue May 29, 2014 · 10 comments
Assignees
Labels
Milestone

Comments

@mkrautz
Copy link
Member

@mkrautz mkrautz commented May 29, 2014

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 mkrautz added this to the 1.2.7 milestone May 29, 2014
@mkrautz mkrautz added mumble labels May 29, 2014
@mkrautz mkrautz self-assigned this May 29, 2014
@mkrautz

This comment has been minimized.

Copy link
Member Author

@mkrautz mkrautz commented May 29, 2014

@mkrautz

This comment has been minimized.

Copy link
Member Author

@mkrautz mkrautz commented May 29, 2014

Targetting this for 1.2.7.

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

@mkrautz

This comment has been minimized.

Copy link
Member Author

@mkrautz mkrautz commented May 29, 2014

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

This comment has been minimized.

Copy link

@the-hive the-hive commented May 29, 2014

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

This comment has been minimized.

Copy link
Member Author

@mkrautz mkrautz commented Jun 11, 2014

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

This comment has been minimized.

Copy link
Member Author

@mkrautz mkrautz commented Jun 11, 2014

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

This comment has been minimized.

Copy link
Member Author

@mkrautz mkrautz commented Jun 11, 2014

Will dig deeper tomorrow.

@mkrautz

This comment has been minimized.

Copy link
Member Author

@mkrautz mkrautz commented Jun 12, 2014

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

@mkrautz

This comment has been minimized.

Copy link
Member Author

@mkrautz mkrautz commented Jun 12, 2014

Fix committed as 7141a05 in master; aef3509 in 1.2.x.

@mkrautz

This comment has been minimized.

Copy link
Member Author

@mkrautz mkrautz commented Jun 14, 2014

Fixed in 1.2.7!

@mkrautz mkrautz closed this Jun 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.