Skip to content

Commit

Permalink
pgwire/hba: parse connection type as bit field, not string
Browse files Browse the repository at this point in the history 
This makes the in-memory data more compact and introduces proper conn
type matching code.

Release note: None
  • Loading branch information
@knz
knz committed Jan 9, 2020
1 parent 0acee6c commit 6f93001
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 31 deletions.
54 changes: 52 additions & 2 deletions pkg/sql/pgwire/hba/hba.go
View file
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
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
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
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
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
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

0 comments on commit 6f93001

Please sign in to comment.