Opened 3 years ago
Closed 3 years ago
#9561 defect closed fixed (fixed)
XMPP's xmlstream is not checking TLS certificate
| Reported by: | Jérôme Poisson | Owned by: | Ralph Meijer |
|---|---|---|---|
| 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 3 years ago by
| Description: | modified (diff) |
|---|
comment:2 Changed 3 years ago by
| Keywords: | review added |
|---|
comment:3 Changed 3 years ago by
| Cc: | Wim Lewis added |
|---|---|
| Keywords: | review removed |
| Owner: | set to Jérôme Poisson |
comment:4 Changed 3 years ago by
| Keywords: | XMPP TLS certificate removed |
|---|
Keywords are used for workflow.
comment:5 Changed 3 years ago by
| Owner: | changed from Jérôme Poisson to Ralph Meijer |
|---|
comment:6 Changed 3 years ago by
| Status: | new → assigned |
|---|
comment:7 Changed 3 years 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 3 years 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 3 years ago by
| Author: | → ralphm |
|---|---|
| Description: | modified (diff) |
| Keywords: | review added |
| Owner: | Ralph Meijer 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 3 years ago by
| Keywords: | review removed |
|---|---|
| Owner: | set to Ralph Meijer |
Thanks, Ralph!
Reviewed & approved at https://github.com/twisted/twisted/pull/1147#pullrequestreview-242396041 .
(removing
reviewkeyword after leaving comment: https://github.com/twisted/twisted/pull/1084#issuecomment-439517473 ; re-add the keyword when it's ready for review again)