Opened 13 years ago

Closed 4 years ago

Last modified 3 years ago

#756 closed Defect (Fixed)

Unsecure SSL connection

Reported by: anonymous Owned by: timothy
Component: Chat Core (IRC) Version: 2.3 (Mac)
Severity: Blocker Keywords:
Cc: colloquy@…

Description (last modified by rinoa)

I find SSL connection handling in Colloquy unsecure. Server's SSL certificate is not checked for validity and as such the connection could be compromised by man in the middle attack.

Colloquy should prompt the user about invalid certificate or at least there should be an option to turn such check on.

I find this critical as it prevents using Colloquy in certain security driven scenarios.

Change History (17)

comment:1 Changed 12 years ago by rinoa

  • Description modified (diff)
  • Version changed from 2.0 (2D16) to Latest 2.1

comment:2 Changed 9 years ago by meineerde

  • Cc colloquy@… added
  • Severity changed from Critical to Blocker
  • Version changed from 2.1 (Mac) to 2.3 (Mac)

Just to bump this up a bit. It is still unresolved in a current 2.3 Colloquy on Snow Leopard. It still happily accepts the self-signed certificate from my bouncer.

This make SSL encryption effectively useless, as anyone capable of sniffing line-traffic could just as easily become a man-in-the-middle and use his own spoofed certificate.

In [3798] (created because of #1212) you disabled certificate checking altogether which makes SSL superfluous. A more sensible workflow would be:

  • check the certificate
  • if it fails
    • ask the user what to do (with hints of what was wrong with the cert)
    • if he ignores the error, ignore only this specific certificate for this specific server
    • else deny access with a usable error message (examples from any browser)
  • only if the certificate is valid or specifically ignored then continue.

Checks should at least include:

  • basic validity (checksums)
  • correct hostname in one of the various fields
  • valid certificate chain from known trust roots.

comment:3 Changed 9 years ago by meineerde

  • milestone set to Colloquy 2.4

This is severe. Please fix this ASAP!

comment:4 Changed 6 years ago by anonymous

I cannot believe that this issue has been been fixed since 2006(!). As already pointed out, Colloquy's SSL option, in its current state, is effectively useless.

As a quick and effective fix, please:

1) Add a toggle which enables checking the certificate with the operating system's database. This should be enabled by default.
2) If the toggle is disabled, display the certificate's fingerprint to the user. Store it after the user confirms the certificate, and display the message again only if the stored fingerprint does not match the fingerprint of the certificate presented by the server.

Colloquy advertises to provide "Solid support for secure connections over SSL." - right now, this is unfortunately not the case.

comment:5 Changed 6 years ago by anonymous

Ping! This bug is severe and makes me sad.
I don't want the government snooping on my conversations!

comment:6 Changed 5 years ago by ma.anselmi@…

I concur. It is a absurd to claim to support SSL and then disable cert verification---deliberately, even! A fix for this is LONG overdue.

comment:7 Changed 5 years ago by ben.maurer@…

Facebook is putting a $300 bounty on this bug via Bounty Source:

comment:8 Changed 5 years ago by anonymous

So as it turns out, SSL did not really work in iOS all the time anyway! Great, no harm done! Oh, no, wait, Colloquy still accepts rogue certificates.

comment:9 Changed 5 years ago by jordy.dickinson@…

I have a solution for this. The diff is here:

comment:10 Changed 5 years ago by zach

I'm sorry, but, I don't think we can accept that patch as a solution; it does not work on iOS, it does not prompt the user when an SSL issue happens and changes the default behavior around without telling people.

An ideal fix would work on Mac and iOS, and, show a panel to the user when an invalid SSL certificate comes up (with certificate information) for people to decide what to do, rather than silently start to fail to connect.

comment:11 Changed 5 years ago by jordy.dickinson@…

Thanks for the feedback. I'll continue to work on this.

comment:12 Changed 5 years ago by jordy.dickinson@…

With regard to "changing the default behavior" I assume you mean defaulting to certificate verification? Should the default then be not verifying, or verify with prompt?

comment:13 Changed 5 years ago by zach

The default should be to match Safari's behavior. e.g.: When there is a problem, want to show a prompt to people, asking what to do — preferably, while showing x509 details. In this prompt, there would be a way to set the policy to always allow, always reject or to always ask (default), as well as to allow or deny the certificate's usage for the current session.

With your current patch, it changes things from connecting without warning on an invalid ssl certificate (bad) to not connecting without warning on an invalid ssl certificate (even worse).

It should also, I would think, be smart enough to handle disconnects and realize that if you're reconnecting from a dropped session, we wouldn't need to revalidate the same certificate again.

Last edited 5 years ago by zach (previous) (diff)

comment:14 Changed 5 years ago by zach

See for why this is a hard problem.

tl;dr is "iOS doesn't expose the necessary information to parse information such as 'when does this certificate expire?' or 'who signed this?' out of an x.509 certificate.

Spoke to people at Apple about this during WWDC 2014. The longer answer is that on iOS, the safest thing to do here (API-wise) would be to pull in libDER and other parts of libsecurity_keychain from Apple (last version provided is from 10.7.5, but, DER parsing hasn't really changed) to build our own SecCertificateRefP's (probably renamed to not conflict with SPI) and then build a CQCertificateTrustPanel using that. CQCertificateTrustPanel would/should mimic how Safari/Mail/Settings?'s certificate view looks and behaves when examining ssl certificates or mobileprovision profiles.

On Mac, the answer is to use the built-in SFCertificateTrustPanel.

comment:15 Changed 5 years ago by rgov

(Application security is my day job.)

With your current patch, it changes things from connecting without warning on an invalid ssl certificate (bad) to not connecting without warning on an invalid ssl certificate (even worse).

I think this is backwards. The user has explicitly requested an encrypted connection by checking the box in the connection settings. A hard failure is reasonable in the sense that the connection could not be established as specified, effectively the same as entering the wrong port.

Of course, a hard failure without being able to inspect the certificate is not ideal, but skipping certificate validation goes completely counter to the user's request and makes the connection readable by an active attacker. Similar vulnerabilities in other software have permitted surveillance of political dissidents, identity theft, etc.

It should also, I would think, be smart enough to handle disconnects and realize that if you're reconnecting from a dropped session, we wouldn't need to revalidate the same certificate again.

The performance gain from doing so is not going to be worth the possibility of introducing more validation issues in the process.

comment:16 Changed 4 years ago by zach

  • Resolution set to Fixed
  • Status changed from new to closed

I added this in r6223. We match Safari's behavior of deny/look at details/confirm on iOS. On Mac we, use the built-in SFCertificateTrustPanel to inspect certificates if we are looking at details.

The default is currently to ask all the time, and there will be options to always accept and never accept insecure certificates as well.

When these options are built, we might default to accepting any SSL certificate (matching the current shipping apps behavior) because almost no IRC network has a completely valid SSL certificate. Freenode has a wildcard for *, most bouncers are self-signed, etc.

Last edited 4 years ago by zach (previous) (diff)

comment:17 Changed 3 years ago by trev

While bouncers do often use self-signed certificates, that's not a reason to ignore connection security completely. Certificate pinning isn't a perfect solution but it is a lot better than nothing - first time the user connects to the bouncer they should get the option to add a permanent exception for this particular invalid certificate in connection with this host name. If the certificate changes later (and only if it changes) they should get a big and scary warning.

Note: See TracTickets for help on using tickets.