Merge pull request #1126 from lucas-clemente/mint-is-insecure

force clients to set tls.Config.InsecureSkipVerify when using mint
This commit is contained in:
Marten Seemann
2018-01-26 21:50:55 +11:00
committed by GitHub
4 changed files with 32 additions and 7 deletions

View File

@@ -79,7 +79,8 @@ var _ = Describe("Handshake tests", func() {
}) })
Context("Certifiate validation", func() { Context("Certifiate validation", func() {
for _, v := range []protocol.VersionNumber{protocol.Version39, protocol.VersionTLS} { // no need to run these tests with TLS. mint doesn't do certificate verification
for _, v := range protocol.SupportedVersions {
version := v version := v
Context(fmt.Sprintf("using %s", version), func() { Context(fmt.Sprintf("using %s", version), func() {

View File

@@ -16,6 +16,8 @@ import (
"github.com/lucas-clemente/quic-go/internal/wire" "github.com/lucas-clemente/quic-go/internal/wire"
) )
var errMintIsInsecure = errors.New("mint currently DOES NOT support certificate verification (see https://github.com/bifurcation/mint/issues/161 for details). Set InsecureSkipVerify to acknowledge that no certificate verification will be performed, and the connection will be vulnerable to man-in-the-middle attacks")
type mintController struct { type mintController struct {
csc *handshake.CryptoStreamConn csc *handshake.CryptoStreamConn
conn *mint.Conn conn *mint.Conn
@@ -77,6 +79,9 @@ func tlsToMintConfig(tlsConf *tls.Config, pers protocol.Perspective) (*mint.Conf
}, },
} }
if tlsConf != nil { if tlsConf != nil {
if pers == protocol.PerspectiveClient && !tlsConf.InsecureSkipVerify {
return nil, errMintIsInsecure
}
mconf.ServerName = tlsConf.ServerName mconf.ServerName = tlsConf.ServerName
mconf.Certificates = make([]*mint.Certificate, len(tlsConf.Certificates)) mconf.Certificates = make([]*mint.Certificate, len(tlsConf.Certificates))
for i, certChain := range tlsConf.Certificates { for i, certChain := range tlsConf.Certificates {

View File

@@ -43,7 +43,10 @@ var _ = Describe("Packing and unpacking Initial packets", func() {
}) })
It("sets the server name", func() { It("sets the server name", func() {
conf := &tls.Config{ServerName: "www.example.com"} conf := &tls.Config{
ServerName: "www.example.com",
InsecureSkipVerify: true,
}
mintConf, err := tlsToMintConfig(conf, protocol.PerspectiveClient) mintConf, err := tlsToMintConfig(conf, protocol.PerspectiveClient)
Expect(err).ToNot(HaveOccurred()) Expect(err).ToNot(HaveOccurred())
Expect(mintConf.ServerName).To(Equal("www.example.com")) Expect(mintConf.ServerName).To(Equal("www.example.com"))
@@ -51,25 +54,40 @@ var _ = Describe("Packing and unpacking Initial packets", func() {
It("sets the certificate chain", func() { It("sets the certificate chain", func() {
tlsConf := testdata.GetTLSConfig() tlsConf := testdata.GetTLSConfig()
tlsConf.InsecureSkipVerify = true
mintConf, err := tlsToMintConfig(tlsConf, protocol.PerspectiveClient) mintConf, err := tlsToMintConfig(tlsConf, protocol.PerspectiveClient)
Expect(err).ToNot(HaveOccurred()) Expect(err).ToNot(HaveOccurred())
Expect(mintConf.Certificates).ToNot(BeEmpty()) Expect(mintConf.Certificates).ToNot(BeEmpty())
Expect(mintConf.Certificates).To(HaveLen(len(tlsConf.Certificates))) Expect(mintConf.Certificates).To(HaveLen(len(tlsConf.Certificates)))
}) })
It("forces the application to set InsecureSkipVerify, because mint is INSECURE", func() {
conf := &tls.Config{
ServerName: "www.example.com",
InsecureSkipVerify: false,
}
_, err := tlsToMintConfig(conf, protocol.PerspectiveClient)
Expect(err).To(HaveOccurred())
Expect(err).To(MatchError(errMintIsInsecure))
})
It("requires client authentication", func() { It("requires client authentication", func() {
mintConf, err := tlsToMintConfig(nil, protocol.PerspectiveClient) conf := &tls.Config{ServerName: "localhost"} // mint forces us to set a ServerName for a server config, although this field is only used for clients
mintConf, err := tlsToMintConfig(conf, protocol.PerspectiveServer)
Expect(err).ToNot(HaveOccurred()) Expect(err).ToNot(HaveOccurred())
Expect(mintConf.RequireClientAuth).To(BeFalse()) Expect(mintConf.RequireClientAuth).To(BeFalse())
conf := &tls.Config{ClientAuth: tls.RequireAnyClientCert} conf = &tls.Config{
mintConf, err = tlsToMintConfig(conf, protocol.PerspectiveClient) ServerName: "localhost", // mint forces us to set a ServerName for a server config, although this field is only used for clients
ClientAuth: tls.RequireAnyClientCert,
}
mintConf, err = tlsToMintConfig(conf, protocol.PerspectiveServer)
Expect(err).ToNot(HaveOccurred()) Expect(err).ToNot(HaveOccurred())
Expect(mintConf.RequireClientAuth).To(BeTrue()) Expect(mintConf.RequireClientAuth).To(BeTrue())
}) })
It("rejects unsupported client auth types", func() { It("rejects unsupported client auth types", func() {
conf := &tls.Config{ClientAuth: tls.RequireAndVerifyClientCert} conf := &tls.Config{ClientAuth: tls.RequireAndVerifyClientCert}
_, err := tlsToMintConfig(conf, protocol.PerspectiveClient) _, err := tlsToMintConfig(conf, protocol.PerspectiveServer)
Expect(err).To(MatchError("mint currently only support ClientAuthType RequireAnyClientCert")) Expect(err).To(MatchError("mint currently only support ClientAuthType RequireAnyClientCert"))
}) })
}) })

View File

@@ -36,7 +36,8 @@ var _ = Describe("Stateless TLS handling", func() {
Versions: []protocol.VersionNumber{protocol.VersionTLS}, Versions: []protocol.VersionNumber{protocol.VersionTLS},
} }
var err error var err error
server, sessionChan, err = newServerTLS(conn, config, nil, testdata.GetTLSConfig()) tlsConf := testdata.GetTLSConfig()
server, sessionChan, err = newServerTLS(conn, config, nil, tlsConf)
Expect(err).ToNot(HaveOccurred()) Expect(err).ToNot(HaveOccurred())
server.newMintConn = func(bc *handshake.CryptoStreamConn, v protocol.VersionNumber) (handshake.MintTLS, <-chan handshake.TransportParameters, error) { server.newMintConn = func(bc *handshake.CryptoStreamConn, v protocol.VersionNumber) (handshake.MintTLS, <-chan handshake.TransportParameters, error) {
mintReply = bc mintReply = bc