From b04299b40c5306171366f9c17518263a4a9f364a Mon Sep 17 00:00:00 2001 From: Mickael Remond Date: Mon, 15 Jul 2019 12:18:35 +0200 Subject: [PATCH] Add support for self-signed certificates --- README.md | 26 ++++++++++++++++++++++++ _examples/xmpp_echo/xmpp_echo.go | 1 + cert_checker.go | 5 +++-- config.go | 2 ++ session.go | 35 +++++++++++++++++++++----------- stanza/starttls.go | 3 --- stream_manager.go | 2 +- 7 files changed, 56 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 2488de9..e205c83 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,32 @@ The goal is to make simple to write simple XMPP clients and components: The library is designed to have minimal dependencies. For now, the library does not depend on any other library. +## Configuration and connection + +### Allowing Insecure TLS connection during development + +It is not recommended to disable the check for domain name and certificate chain. Doing so would open your client +to man-in-the-middle attacks. + +However, in development, XMPP servers often use self-signed certificates. In that situation, it is better to add the +root CA that signed the certificate to your trusted list of root CA. It avoids changing the code and limit the risk +of shipping an insecure client to production. + +That said, if you really want to allow your client to trust any TLS certificate, you can customize Go standard +`tls.Config` and set it in Config struct. + +Here is an example code to configure a client to allow connecting to a server with self-signed certificate. Note the +`InsecureSkipVerify` option. When using this `tls.Config` option, all the checks on the certificate are skipped. + +```go +config := xmpp.Config{ + Address: "localhost:5222", + Jid: "test@localhost", + Password: "test", + TLSConfig: tls.Config{InsecureSkipVerify: true}, +} +``` + ## Supported specifications ### Clients diff --git a/_examples/xmpp_echo/xmpp_echo.go b/_examples/xmpp_echo/xmpp_echo.go index 8794a5a..38cbe62 100644 --- a/_examples/xmpp_echo/xmpp_echo.go +++ b/_examples/xmpp_echo/xmpp_echo.go @@ -20,6 +20,7 @@ func main() { Password: "test", StreamLogger: os.Stdout, Insecure: true, + // TLSConfig: tls.Config{InsecureSkipVerify: true}, } router := xmpp.NewRouter() diff --git a/cert_checker.go b/cert_checker.go index e33d9f9..c85664b 100644 --- a/cert_checker.go +++ b/cert_checker.go @@ -86,8 +86,9 @@ func (c *ServerCheck) Check() error { return fmt.Errorf("expecting starttls proceed: %s", err) } - stanza.DefaultTlsConfig.ServerName = c.domain - tlsConn := tls.Client(tcpconn, &stanza.DefaultTlsConfig) + var tlsConfig tls.Config + tlsConfig.ServerName = c.domain + tlsConn := tls.Client(tcpconn, &tlsConfig) // We convert existing connection to TLS if err = tlsConn.Handshake(); err != nil { return err diff --git a/config.go b/config.go index 3ae4a1c..a9b6b8d 100644 --- a/config.go +++ b/config.go @@ -1,6 +1,7 @@ package xmpp import ( + "crypto/tls" "io" "os" ) @@ -13,6 +14,7 @@ type Config struct { StreamLogger *os.File // Used for debugging Lang string // TODO: should default to 'en' ConnectTimeout int // Client timeout in seconds. Default to 15 + TLSConfig tls.Config // Insecure can be set to true to allow to open a session without TLS. If TLS // is supported on the server, we will still try to use it. Insecure bool diff --git a/session.go b/session.go index 4fe8bcf..57d96a6 100644 --- a/session.go +++ b/session.go @@ -35,13 +35,15 @@ func NewSession(conn net.Conn, o Config) (net.Conn, *Session, error) { // starttls var tlsConn net.Conn - tlsConn = s.startTlsIfSupported(conn, o.parsedJid.Domain) - if s.TlsEnabled { - s.reset(conn, tlsConn, o) - } + tlsConn = s.startTlsIfSupported(conn, o.parsedJid.Domain, o) if !s.TlsEnabled && !o.Insecure { - return nil, nil, NewConnError(errors.New("failed to negotiate TLS session"), true) + err := fmt.Errorf("failed to negotiate TLS session : %s", s.err) + return nil, nil, NewConnError(err, true) + } + + if s.TlsEnabled { + s.reset(conn, tlsConn, o) } // auth @@ -101,7 +103,7 @@ func (s *Session) open(domain string) (f stanza.StreamFeatures) { return } -func (s *Session) startTlsIfSupported(conn net.Conn, domain string) net.Conn { +func (s *Session) startTlsIfSupported(conn net.Conn, domain string, o Config) net.Conn { if s.err != nil { return conn } @@ -114,21 +116,30 @@ func (s *Session) startTlsIfSupported(conn net.Conn, domain string) net.Conn { s.err = errors.New("expecting starttls proceed: " + s.err.Error()) return conn } - s.TlsEnabled = true - // TODO: add option to accept all TLS certificates: insecureSkipTlsVerify (DefaultTlsConfig.InsecureSkipVerify) - stanza.DefaultTlsConfig.ServerName = domain - tlsConn := tls.Client(conn, &stanza.DefaultTlsConfig) + o.TLSConfig.ServerName = domain + tlsConn := tls.Client(conn, &o.TLSConfig) // We convert existing connection to TLS if s.err = tlsConn.Handshake(); s.err != nil { return tlsConn } - // We check that cert matches hostname - s.err = tlsConn.VerifyHostname(domain) + if !o.TLSConfig.InsecureSkipVerify { + // We check that cert matches hostname + s.err = tlsConn.VerifyHostname(domain) + } + + if s.err == nil { + s.TlsEnabled = true + } return tlsConn } + // If we do not allow cleartext connections, make it explicit that server do not support starttls + if !o.Insecure { + s.err = errors.New("XMPP server does not advertise support for starttls") + } + // starttls is not supported => we do not upgrade the connection: return conn } diff --git a/stanza/starttls.go b/stanza/starttls.go index e0a187c..b8fce94 100644 --- a/stanza/starttls.go +++ b/stanza/starttls.go @@ -1,12 +1,9 @@ package stanza import ( - "crypto/tls" "encoding/xml" ) -var DefaultTlsConfig tls.Config - // Used during stream initiation / session establishment type TLSProceed struct { XMLName xml.Name `xml:"urn:ietf:params:xml:ns:xmpp-tls proceed"` diff --git a/stream_manager.go b/stream_manager.go index a15ce40..1aaf164 100644 --- a/stream_manager.go +++ b/stream_manager.go @@ -119,7 +119,7 @@ func (sm *StreamManager) connect() error { var actualErr ConnError if xerrors.As(err, &actualErr) { if actualErr.Permanent { - return xerrors.Errorf("unrecoverable connect error %w", actualErr) + return xerrors.Errorf("unrecoverable connect error %#v", actualErr) } } backoff.wait()