Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non-obvious connection error when tls is enabled on DSN without port #717

Closed
artyom opened this issue Nov 29, 2017 · 2 comments
Closed

Non-obvious connection error when tls is enabled on DSN without port #717

artyom opened this issue Nov 29, 2017 · 2 comments

Comments

@artyom
Copy link

artyom commented Nov 29, 2017

Issue description

I believe the fix for issue #668 introduced a regression: now if DSN specifies host without a port along with a tls=true argument, connection fails with non-obvious message from crypto/tls package:

tls: either ServerName or InsecureSkipVerify must be specified in the tls.Config

Whereas it should either succeed provided that certificate is valid or fail with the proper error: x509: certificate signed by unknown authority.

Here's the relevant code:

mysql/dsn.go

Lines 524 to 527 in cd4cb90

host, _, err := net.SplitHostPort(cfg.Addr)
if err == nil {
cfg.tls.ServerName = host
}

As cfg.Addr holds only hostname without port, net.SplitHostPort returns non-nil error and cfg.tls.ServerName left blank leading to error tls: either ServerName or InsecureSkipVerify must be specified in the tls.Config returned from crypto/tls code:

https://github.com/golang/go/blob/21672b36eb0ad3bf1e2220b247900b2c63664464/src/crypto/tls/handshake_client.go#L32-L35

Example code

package main

import (
	"database/sql"
	"flag"
	"fmt"
	"os"

	_ "github.com/go-sql-driver/mysql"
)

func main() {
	dsn := "user:password@tcp(hostname)/?tls=true"
	flag.StringVar(&dsn, "dsn", dsn, "https://github.com/go-sql-driver/mysql#dsn-data-source-name")
	flag.Parse()
	if err := run(dsn); err != nil {
		fmt.Fprintln(os.Stderr, err)
		os.Exit(1)
	}
}

func run(dsn string) error {
	db, err := sql.Open("mysql", dsn)
	if err != nil {
		return err
	}
	defer db.Close()
	return db.Ping()
}

Run this adjusting -dsn flag value as necessary — set host without port as address.

Error log

tls: either ServerName or InsecureSkipVerify must be specified in the tls.Config 

Configuration

Driver version (or git SHA):

949221881fa8e37a73d073dd3e33d857909d909e

Go version: run go version in your console

go version devel +9a13f8e11c Tue Nov 28 06:47:50 2017 +0000 darwin/amd64

Server version: E.g. MySQL 5.6, MariaDB 10.0.20

5.6.10 MySQL Community Server (GPL)

Server OS: E.g. Debian 8.1 (Jessie), Windows 10

Debian GNU/Linux 8.8 (jessie)
@methane
Copy link
Member

methane commented Nov 30, 2017

I believe the fix for issue #668 introduced a regression: now if DSN specifies host without a port along with a tls=true argument, connection fails with non-obvious message from crypto/tls package:

#668 is not wrong. See #564, it is broken at first.

@methane
Copy link
Member

methane commented Nov 30, 2017

Before #668, hostname without port is not accepted.
After #668, port can be omitted. But cfg.Addr has port always after parsing DSN.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants