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.

Sign up for GitHub

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

pgwire/hba: parse connection type as bit field, not string #43843

Merged
merged 1 commit into from
Jan 9, 2020
Merged

pgwire/hba: parse connection type as bit field, not string #43843

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 52 additions & 2 deletions pkg/sql/pgwire/hba/hba.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ type Conf struct {

// Entry is a single line of a configuration.
type Entry struct {
Type String
// ConnType is the connection type to match.
ConnType ConnType
// Database is the list of databases to match. An empty list means
// "match any database".
Database []String
Expand All @@ -51,6 +52,38 @@ type Entry struct {
OptionQuotes []bool
}

// ConnType represents the type of connection matched by a rule.
type ConnType int

const (
// ConnLocal matches unix socket connections.
ConnLocal ConnType = 1 << iota
// ConnHostNoSSL matches TCP connections without SSL/TLS.
ConnHostNoSSL
// ConnHostSSL matches TCP connections with SSL/TLS.
ConnHostSSL

// ConnHostAny matches TCP connections with or without SSL/TLS.
ConnHostAny = ConnHostNoSSL | ConnHostSSL
)

// String implements the fmt.Formatter interface.
func (t ConnType) String() string {
switch t {
case ConnLocal:
return "local"
case ConnHostNoSSL:
return "hostnossl"
case ConnHostSSL:
return "hostssl"
case ConnHostAny:
return "host"
default:
panic("unimplemented")
}
}

// String implements the fmt.Formatter interface.
func (c Conf) String() string {
if len(c.Entries) == 0 {
return "# (empty configuration)\n"
Expand All @@ -68,7 +101,7 @@ func (c Conf) String() string {
row := []string{"# TYPE", "DATABASE", "USER", "ADDRESS", "METHOD", "OPTIONS"}
table.Append(row)
for _, e := range c.Entries {
row[0] = e.Type.String()
row[0] = e.ConnType.String()
row[1] = e.DatabaseString()
row[2] = e.UserString()
row[3] = e.AddressString()
Expand Down Expand Up @@ -114,6 +147,23 @@ func (h Entry) GetOptions(name string) []string {
return val
}

// ConnTypeMatches returns true iff the provided actual client connection
// type matches the connection type specified in the rule.
func (h Entry) ConnTypeMatches(clientConn ConnType) bool {
switch clientConn {
case ConnLocal:
return h.ConnType == ConnLocal
case ConnHostSSL:
// A SSL connection matches both "hostssl" and "host".
return h.ConnType&ConnHostSSL != 0
case ConnHostNoSSL:
// A non-SSL connection matches both "hostnossl" and "host".
return h.ConnType&ConnHostNoSSL != 0
default:
panic("unimplemented")
}
}

// UserMatches returns true iff the provided username matches the an
// entry in the User list or if the user list is empty (the entry
// matches all).
Expand Down
26 changes: 26 additions & 0 deletions pkg/sql/pgwire/hba/hba_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,32 @@ func TestParseAndNormalizeAuthConfig(t *testing.T) {
})
}

func TestMatchConnType(t *testing.T) {
testCases := []struct {
conf, conn ConnType
match bool
}{
{ConnLocal, ConnHostSSL, false},
{ConnLocal, ConnHostNoSSL, false},
{ConnLocal, ConnLocal, true},
{ConnHostAny, ConnLocal, false},
{ConnHostAny, ConnHostSSL, true},
{ConnHostAny, ConnHostNoSSL, true},
{ConnHostSSL, ConnLocal, false},
{ConnHostSSL, ConnHostSSL, true},
{ConnHostSSL, ConnHostNoSSL, false},
{ConnHostNoSSL, ConnLocal, false},
{ConnHostNoSSL, ConnHostSSL, false},
{ConnHostNoSSL, ConnHostNoSSL, true},
}
for _, tc := range testCases {
entry := Entry{ConnType: tc.conf}
if m := entry.ConnTypeMatches(tc.conn); m != tc.match {
t.Errorf("%s vs %s: expected %v, got %v", tc.conf, tc.conn, tc.match, m)
}
}
}

// TODO(mjibson): these are untested outside ccl +gss builds.
var _ = Entry.GetOption
var _ = Entry.GetOptions
26 changes: 20 additions & 6 deletions pkg/sql/pgwire/hba/parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ func Parse(input string) (*Conf, error) {
// parseHbaLine parses one line of HBA configuration.
//
// Inspired from pg's src/backend/libpq/hba.c, parse_hba_line().
func parseHbaLine(line [][]String) (Entry, error) {
var entry Entry
func parseHbaLine(line [][]String) (entry Entry, err error) {
fieldIdx := 0

// Read the connection type.
Expand All @@ -71,9 +70,9 @@ func parseHbaLine(line [][]String) (Entry, error) {
errors.New("multiple values specified for connection type"),
"Specify exactly one connection type per line.")
}
entry.Type = line[fieldIdx][0]
if entry.Type.Value == "" {
return entry, errors.New("cannot use empty string as connection type")
entry.ConnType, err = ParseConnType(line[fieldIdx][0].Value)
if err != nil {
return entry, err
}

// Get the databases.
Expand All @@ -90,7 +89,7 @@ func parseHbaLine(line [][]String) (Entry, error) {
}
entry.User = line[fieldIdx]

if entry.Type.Value != "local" {
if entry.ConnType != ConnLocal {
fieldIdx++
if fieldIdx >= len(line) {
return entry, errors.New("end-of-line before IP address specification")
Expand Down Expand Up @@ -212,3 +211,18 @@ func checkMask(maybeMask net.IP) error {
}
return nil
}

// ParseConnType parses the connection type field.
func ParseConnType(s string) (ConnType, error) {
switch s {
case "local":
return ConnLocal, nil
case "host":
return ConnHostAny, nil
case "hostssl":
return ConnHostSSL, nil
case "hostnossl":
return ConnHostNoSSL, nil
}
return 0, errors.Newf("unknown connection type: %q", s)
}
31 changes: 18 additions & 13 deletions pkg/sql/pgwire/hba/testdata/parse
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ subtest end

subtest unknown_formats

line
unknown a b c
----
error: unknown connection type: "unknown"

line
local a b c d
----
Expand All @@ -46,8 +51,8 @@ subtest quoted_columns
line
"local" a b c
----
# TYPE DATABASE USER ADDRESS METHOD OPTIONS
"local" a b c
# TYPE DATABASE USER ADDRESS METHOD OPTIONS
local a b c

line
local a b "method"
Expand Down Expand Up @@ -226,7 +231,7 @@ host all testuser,"all" 0.0.0.0/0 cert
&hba.Conf{
Entries: {
{
Type: hba.String{Value:"host", Quoted:false},
ConnType: 6,
Database: {
{Value:"a", Quoted:false},
{Value:"b", Quoted:false},
Expand All @@ -242,7 +247,7 @@ host all testuser,"all" 0.0.0.0/0 cert
OptionQuotes: nil,
},
{
Type: hba.String{Value:"host", Quoted:false},
ConnType: 6,
Database: {
{Value:"all", Quoted:false},
},
Expand All @@ -258,7 +263,7 @@ host all testuser,"all" 0.0.0.0/0 cert
OptionQuotes: nil,
},
{
Type: hba.String{Value:"host", Quoted:false},
ConnType: 6,
Database: {
{Value:"a", Quoted:false},
{Value:"b", Quoted:false},
Expand All @@ -276,7 +281,7 @@ host all testuser,"all" 0.0.0.0/0 cert
OptionQuotes: nil,
},
{
Type: hba.String{Value:"host", Quoted:false},
ConnType: 6,
Database: {
{Value:"all", Quoted:false},
},
Expand Down Expand Up @@ -307,7 +312,7 @@ host "all","test space",something some,"us ers" all cert
&hba.Conf{
Entries: {
{
Type: hba.String{Value:"host", Quoted:false},
ConnType: 6,
Database: {
{Value:"all", Quoted:true},
{Value:"test space", Quoted:true},
Expand All @@ -333,7 +338,7 @@ subtest empty_strings
multiline
"" all all all trust
----
error: line 1: cannot use empty string as connection type
error: line 1: unknown connection type: ""

multiline
host all all all ""
Expand All @@ -359,7 +364,7 @@ host all all all gss k=v " someopt = withspaces "
&hba.Conf{
Entries: {
{
Type: hba.String{Value:"host", Quoted:false},
ConnType: 6,
Database: {
{Value:"all", Quoted:false},
},
Expand Down Expand Up @@ -427,7 +432,7 @@ host all all all gss krb_realm=other include_realm=0 krb_r
&hba.Conf{
Entries: {
{
Type: hba.String{Value:"host", Quoted:false},
ConnType: 6,
Database: {
{Value:"all", Quoted:false},
},
Expand All @@ -443,7 +448,7 @@ host all all all gss krb_realm=other include_realm=0 krb_r
OptionQuotes: {false},
},
{
Type: hba.String{Value:"host", Quoted:false},
ConnType: 6,
Database: {
{Value:"all", Quoted:false},
},
Expand Down Expand Up @@ -479,7 +484,7 @@ host "all" "all" 0.0.0.0/0 cert
&hba.Conf{
Entries: {
{
Type: hba.String{Value:"host", Quoted:false},
ConnType: 6,
Database: {
{Value:"db", Quoted:false},
},
Expand All @@ -496,7 +501,7 @@ host "all" "all" 0.0.0.0/0 cert
OptionQuotes: nil,
},
{
Type: hba.String{Value:"host", Quoted:false},
ConnType: 6,
Database: {
{Value:"all", Quoted:true},
},
Expand Down
18 changes: 9 additions & 9 deletions pkg/sql/pgwire/hba_conf.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,9 @@ func checkHBASyntaxBeforeUpdatingSetting(values *settings.Values, s string) erro
}

for _, entry := range conf.Entries {
switch entry.Type.Value {
case "host":
case "local":
switch entry.ConnType {
case hba.ConnHostAny:
case hba.ConnLocal:
if st != nil &&
!cluster.Version.IsActive(context.TODO(), st, cluster.VersionAuthLocalAndTrustRejectMethods) {
return pgerror.Newf(pgcode.ObjectNotInPrerequisiteState,
Expand All @@ -153,8 +153,8 @@ func checkHBASyntaxBeforeUpdatingSetting(values *settings.Values, s string) erro
// The syntax 'local' is not yet supported.
fallthrough
default:
return unimplemented.Newf("hba-type-"+entry.Type.Value,
"unsupported connection type: %s", entry.Type.Value)
return unimplemented.Newf("hba-type-"+entry.ConnType.String(),
"unsupported connection type: %s", entry.ConnType)
}
for _, db := range entry.Database {
if !db.IsKeyword("all") {
Expand Down Expand Up @@ -242,10 +242,10 @@ func ParseAndNormalize(val string) (*hba.Conf, error) {
}

var rootEntry = hba.Entry{
Type: hba.String{Value: "host"},
User: []hba.String{{Value: security.RootUser, Quoted: false}},
Address: hba.AnyAddr{},
Method: hba.String{Value: "cert"},
ConnType: hba.ConnHostAny,
User: []hba.String{{Value: security.RootUser, Quoted: false}},
Address: hba.AnyAddr{},
Method: hba.String{Value: "cert"},
}

// DefaultHBAConfig is used when the stored HBA configuration string
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/pgwire/testdata/auth/hba_syntax
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
set_hba
bad
local
----
ERROR: line 1: end-of-line before database specification (SQLSTATE F0000)

Expand Down