Opened 13 months ago

Closed 5 months ago

#9561 defect closed fixed (fixed)

XMPP's xmlstream is not checking TLS certificate

Reported by: Jérôme Poisson Owned by: ralphm
Priority: high Milestone:
Component: words Keywords:
Cc: Wim Lewis Branch:
Author: ralphm

Description (last modified by ralphm)

words.protocols.jabber.xmlstream's TLSInitiatingInitializer is not checking TLS certificate, and has not option to do so. As a result, any certificate can be used without alert.

The ssl.CertificateOptions in TLSInitiatingInitializer.onProceed method should have an option (probably activated by default) to set trustRoot = ssl.platformTrust().

I set this as a defect and high priority because it's a security issue (TLS encryption is mandatory in XMPP since 2014).

github PR: https://github.com/twisted/twisted/pull/1084 https://github.com/twisted/twisted/pull/1147

Change History (11)

comment:1 Changed 13 months ago by Jérôme Poisson

Description: modified (diff)

comment:2 Changed 13 months ago by Jérôme Poisson

Keywords: review added

comment:3 Changed 13 months ago by Wim Lewis

Cc: Wim Lewis added
Keywords: review removed
Owner: set to Jérôme Poisson

(removing review keyword after leaving comment: https://github.com/twisted/twisted/pull/1084#issuecomment-439517473 ; re-add the keyword when it's ready for review again)

comment:4 Changed 7 months ago by Glyph

Keywords: XMPP TLS certificate removed

Keywords are used for workflow.

comment:5 Changed 7 months ago by Glyph

Owner: changed from Jérôme Poisson to ralphm

comment:6 Changed 7 months ago by ralphm

Status: newassigned

comment:7 Changed 7 months ago by Jérôme Poisson

Hello, just for the record I'm sorry for the long delay, I have been overwhelmed lastly and didn't update my PR yet, but I will.

comment:8 Changed 7 months ago by Glyph

Hello, just for the record I'm sorry for the long delay, I have been overwhelmed lastly and didn't update my PR yet, but I will.

No need to apologize. We're all volunteers here :).

Ralph is probably going to take over the fix for this PR, but there are going to be a number of follow-up issues which we'd be delighted to have your help with :).

comment:9 Changed 6 months ago by ralphm

Author: ralphm
Description: modified (diff)
Keywords: review added
Owner: ralphm deleted
Status: assignednew

Created a more comprehensive PR for this, including the ability to pass custom certificate options. https://github.com/twisted/twisted/pull/1147

Please review.

comment:10 Changed 6 months ago by Glyph

Keywords: review removed
Owner: set to ralphm

comment:11 Changed 5 months ago by Ralph Meijer <ralphm@…>

Resolution: fixed
Status: newclosed

In cf8ed698:

Merge pull request #1147 from twisted/9561-xmpp-tls-verify-cert

#9561 Check remote certificates for XMPP TLS (CVE-2019-12855)

Author: ralphm
Reviewer: glyph, alex
Fixes: ticket:9561

Note: See TracTickets for help on using tickets.