Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#2835 closed defect (fixed)

Vidalia ‘Find Bridges Now’ button doesn't work

Reported by: rransom Owned by: chiiph
Priority: High Milestone:
Component: Archived/Vidalia Version:
Severity: Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

The ‘Find Bridges Now’ button in Vidalia 0.2.10 does not work: it always produces the error message “Unable to download bridges: HTTP request failed”. Found by Lorenz Kirchner using the experimental FF4 TBB for MacOS, confirmed by me using Vidalia 0.2.10 compiled from the FreeBSD 8.2 port. Lorenz was able to access bridges.tpo using his browser. We checked the certificate's SHA1 fingerprint, and he was not being MITMed.

Child Tickets

Change History (17)

comment:1 Changed 8 years ago by arma

Perhaps the bridges.tp.o https cert changed, and Vidalia now doesn't like it because it has the old one hard-coded to check?

comment:2 Changed 8 years ago by chiiph

Nope, it's a Qt problem. I already have a branch that fixes it, but it needs a little bit more work. I was planning on putting it for review today.

comment:3 Changed 8 years ago by edmanm

What Qt problem? It sure looks like a certificate problem to me. Like arma said, bridges.torproject.org seems to have changed certificates (again). Now it's using the DigiCert *.torproject.org certificate, whereas Vidalia still just has the Equifax CA cert used to verify the previous bridges.torproject.org cert.

$ src/vidalia/Vidalia.app/Contents/MacOS/Vidalia --loglevel warn
Apr 17 11:16:12.793 [warn] 3 SSL error(s) when requesting bridge information (id 1):
Apr 17 11:16:12.793 [warn]   SSL Error: The issuer certificate of a locally looked up certificate could not be found
Apr 17 11:16:12.793 [warn]   SSL Error: The root CA certificate is not trusted for this purpose
Apr 17 11:16:12.794 [warn]   SSL Error: The signature of the certificate is invalid
Apr 17 11:16:12.794 [warn] Bridge request failed (id 1): HTTP request failed

comment:4 Changed 8 years ago by chiiph

Yes, but QHttp is deprecated, so the idea is to handle the CACert issue and port the class to QNetwork*

comment:5 Changed 8 years ago by edmanm

Sure, but that's a separate issue that actually has nothing to do with the error reported in this ticket.

comment:6 Changed 8 years ago by chiiph

Status: newneeds_review

Here's a fix for this.

Basically it ports the class to use the new QtNetwork classes, and checks the CACert SHA-1 digest against the known one (hardcoded), if everything's ok, it ignores the errors reported about it not knowing the cert.

Here's the commit diff:
https://gitweb.torproject.org/chiiph/vidalia.git/commitdiff/cbfe5e908026729818d91d2fbb17a92469574abd

It's branch bug2835_getbridges

comment:7 Changed 8 years ago by edmanm

Why would you hardcode the hash, rather than including the proper CA certificate? See Vidalia::loadDefaultCaCertificates().

comment:8 Changed 8 years ago by chiiph

Right, I didn't know about that method.

Anyway, just adding the cert isn't working. I can see that the cert's being added, everything seems right in it, but the SSL Error keeps showing up. Same issue happened when I tried to add the certificate to the list that the QNetworkReply->QSslConfiguration has.

The same error happens if I just add the cert to the QHttp version of this.

comment:9 Changed 8 years ago by jrklein

I was able to reproduce this issue and saw errors similar to those posted in comment 3.

While you are improving the SSL code, would it be possible to update the ca certificate file that is loaded by loadDefaultCaCertificates() so that the file contains a comprehensive list of certificate authorities?

This type of file can be extracted from the ca-certificates package (Debian, Ubuntu). The file is located at "/etc/ssl/certs/ca-certificates.crt" on those systems. Let me know if you'd like help with this or if this should be submitted as a separate ticket.

A comprehensive file would still need to be updated on occasion, but should prevent sudden breakage if they keep moving between SSL providers.

comment:10 in reply to:  8 ; Changed 8 years ago by edmanm

Replying to chiiph:

Right, I didn't know about that method.

Anyway, just adding the cert isn't working. I can see that the cert's being added, everything seems right in it, but the SSL Error keeps showing up. Same issue happened when I tried to add the certificate to the list that the QNetworkReply->QSslConfiguration has.

The same error happens if I just add the cert to the QHttp version of this.

Interesting. I suppose either 1) you're adding the certificate incorrectly or adding the wrong certificate, 2) there's a bug in Qt's OpenSSL support, or 3) the certificate is invalid. I think we can be pretty sure number 3 is not the case, so it's probably one of the first two. :) I'll see if I can reproduce that this weekend.

Replying to jrklein:

While you are improving the SSL code, would it be possible to update the ca certificate file that is loaded by loadDefaultCaCertificates() so that the file contains a comprehensive list of certificate authorities?

Gah. Don't do that. The fact that there are no additional built-in CA certificates is intentional. See the first line in Vidalia::loadDefaultCaCertificates().

comment:11 Changed 8 years ago by jrklein

edmanm: Thanks for the clarification. That makes sense.

comment:12 in reply to:  10 Changed 8 years ago by chiiph

Replying to edmanm:

Replying to chiiph:

Right, I didn't know about that method.

Anyway, just adding the cert isn't working. I can see that the cert's being added, everything seems right in it, but the SSL Error keeps showing up. Same issue happened when I tried to add the certificate to the list that the QNetworkReply->QSslConfiguration has.

The same error happens if I just add the cert to the QHttp version of this.

Interesting. I suppose either 1) you're adding the certificate incorrectly or adding the wrong certificate, 2) there's a bug in Qt's OpenSSL support, or 3) the certificate is invalid. I think we can be pretty sure number 3 is not the case, so it's probably one of the first two. :) I'll see if I can reproduce that this weekend.

1) No, I'm sure I'm adding the cert right.
2) Possible, while checking things "by hand" I noticed I wasn't able to check the serial number.
3) The cert is the one I exported from Firefox, may be that's what's wrong.

comment:13 Changed 8 years ago by chiiph

It seems adding Tor's cert as a valid one isn't the way of doing this. I've just added DigiCert's CA certificates, and now it works.

See my branch chiiph/bug2835_getbridges:
https://gitweb.torproject.org/chiiph/vidalia.git/shortlog/refs/heads/bug2835_getbridges

This is much better now.

comment:14 Changed 8 years ago by edmanm

Ah, yeah, I was going to take a look at this and then forgot. Sorry. But, yes, you want to add the right CA certificate, not Tor's certificate. That's why I said "including the proper CA certificate" above, named the function loadDefaultCaCertificates(), use QSslSocket::addDefaultCaCertificates(), etc. :-)

In any case, that patch sure doesn't look right to me. You're adding the certificates to the default list of CA certificates like this:

+  if (! QSslSocket::addDefaultCaCertificates(":/pki/DigiCertCA.crt"))
+    vWarn("Failed to add the DigiCert Global CA certificate to the default CA "
+          "certificate database.");
+  if (! QSslSocket::addDefaultCaCertificates(":/pki/DigiCertCA2.crt"))
+    vWarn("Failed to add the DigiCert Assured CA certificate to the default CA "
+          "certificate database.");
+  if (! QSslSocket::addDefaultCaCertificates(":/pki/DigiCertCA3.crt"))
+    vWarn("Failed to add the DigiCert High Assurance CA certificate to the default CA "
+          "certificate database.");

But you're adding them to the .qrc like this:

+    <file>DigiCertCA.crt</file>
+    <file>DigiCertAssuredCA.crt</file>
+    <file>DigiCertHighAssuranceCA.crt</file>

The second two DigiCert CA certificates you're trying to add in Vidalia::loadDefaultCaCertificates() won't get added since you're specifying the wrong filenames. The first one will, though, so if that's all that's needed then why are you trying to add the other two CA certificates?

comment:15 Changed 8 years ago by chiiph

Ugh, yeah, I mixed up the commits. I added them with those names when I was testing.

It doesn't work if you just add the first one, you need to have all three of them it seems.

comment:16 Changed 8 years ago by chiiph

Resolution: fixed
Status: needs_reviewclosed

Merged to both master and alpha.

comment:17 Changed 8 years ago by chiiph

This will be on 0.3.0-alpha, and later on 0.2.13.

Note: See TracTickets for help on using tickets.