Opened 2 years ago
Closed 19 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 )
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 2 years ago by
Description: | modified (diff) |
---|
comment:2 Changed 2 years ago by
Keywords: | review added |
---|
comment:3 Changed 2 years ago by
Cc: | Wim Lewis added |
---|---|
Keywords: | review removed |
Owner: | set to Jérôme Poisson |
comment:4 Changed 21 months ago by
Keywords: | XMPP TLS certificate removed |
---|
Keywords are used for workflow.
comment:5 Changed 21 months ago by
Owner: | changed from Jérôme Poisson to ralphm |
---|
comment:6 Changed 21 months ago by
Status: | new → assigned |
---|
comment:7 Changed 21 months ago by
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 21 months ago by
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 20 months ago by
Author: | → ralphm |
---|---|
Description: | modified (diff) |
Keywords: | review added |
Owner: | ralphm deleted |
Status: | assigned → new |
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 20 months ago by
Keywords: | review removed |
---|---|
Owner: | set to ralphm |
Thanks, Ralph!
Reviewed & approved at https://github.com/twisted/twisted/pull/1147#pullrequestreview-242396041 .
(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)