diff --git a/backend/agent/bind9.go b/backend/agent/bind9.go index 8cb8bb4c3..ed302cc3b 100644 --- a/backend/agent/bind9.go +++ b/backend/agent/bind9.go @@ -520,6 +520,10 @@ func parseNamedDefaultPath(output []byte) string { // // It returns the BIND 9 app instance or an error if the BIND 9 is not // recognized or any error occurs. +// +// ToDo: Enable the linter check after splitting this function in #1991. +// +//nolint:gocyclo func detectBind9App(p supportedProcess, executor storkutil.CommandExecutor, explicitConfigPath string, parser bind9FileParser) (App, error) { cmdline, err := p.getCmdline() if err != nil { @@ -657,6 +661,13 @@ func detectBind9App(p supportedProcess, executor storkutil.CommandExecutor, expl return nil, errors.Wrapf(err, "failed to parse BIND 9 config file %s", prefixedBind9ConfPath) } + if bind9Config.HasNoParse() { + // If some of the configuration parts are elided, it may cause issues with + // interactions of the Stork agent with BIND 9. The user should be warned. + log.Warn("BIND 9 config file contains @stork:no-parse directives. Skipping parsing selected config parts improves performance but may cause issues with interactions of the Stork agent with BIND 9.") + log.Warn("Make sure that you understand the implications of eliding selected config parts, e.g., allow-transfer statements in zones.") + } + // look for control address in config ctrlAddress, ctrlPort, ctrlKey := getCtrlAddressFromBind9Config(cfgText) if ctrlPort == 0 || len(ctrlAddress) == 0 { diff --git a/backend/appcfg/bind9/addressmatchlist.go b/backend/appcfg/bind9/addressmatchlist.go index 645bbbae9..ee3a59b7c 100644 --- a/backend/appcfg/bind9/addressmatchlist.go +++ b/backend/appcfg/bind9/addressmatchlist.go @@ -3,9 +3,9 @@ package bind9config // Checks if the address match list excludes the specified IP address. func (aml *AddressMatchList) ExcludesIPAddress(ipAddress string) bool { for _, element := range aml.Elements { - if (element.IPAddress == ipAddress && element.Negation) || - (element.ACLName == "none" && !element.Negation) || - (element.ACLName == "any" && element.Negation) { + if (element.IPAddressOrACLName == ipAddress && element.Negation) || + (element.IPAddressOrACLName == "none" && !element.Negation) || + (element.IPAddressOrACLName == "any" && element.Negation) { return true } } diff --git a/backend/appcfg/bind9/addressmatchlist_test.go b/backend/appcfg/bind9/addressmatchlist_test.go index 82edda102..2016d20bf 100644 --- a/backend/appcfg/bind9/addressmatchlist_test.go +++ b/backend/appcfg/bind9/addressmatchlist_test.go @@ -10,8 +10,8 @@ import ( func TestAddressMatchListExcludesIPAddress(t *testing.T) { aml := &AddressMatchList{ Elements: []*AddressMatchListElement{ - {IPAddress: "127.0.0.1", Negation: true}, - {IPAddress: "::1", Negation: false}, + {IPAddressOrACLName: "127.0.0.1", Negation: true}, + {IPAddressOrACLName: "::1", Negation: false}, }, } require.True(t, aml.ExcludesIPAddress("127.0.0.1")) @@ -24,7 +24,7 @@ func TestAddressMatchListExcludesIPAddress(t *testing.T) { func TestAddressMatchListExcludesIPAddressWithNone(t *testing.T) { aml := &AddressMatchList{ Elements: []*AddressMatchListElement{ - {ACLName: "none"}, + {IPAddressOrACLName: "none"}, }, } require.True(t, aml.ExcludesIPAddress("127.0.0.1")) @@ -37,7 +37,7 @@ func TestAddressMatchListExcludesIPAddressWithNone(t *testing.T) { func TestAddressMatchListExcludesIPAddressWithAny(t *testing.T) { aml := &AddressMatchList{ Elements: []*AddressMatchListElement{ - {ACLName: "any"}, + {IPAddressOrACLName: "any"}, }, } require.False(t, aml.ExcludesIPAddress("127.0.0.1")) diff --git a/backend/appcfg/bind9/allowtransfer.go b/backend/appcfg/bind9/allowtransfer.go index df1ffcb93..6a96f93b6 100644 --- a/backend/appcfg/bind9/allowtransfer.go +++ b/backend/appcfg/bind9/allowtransfer.go @@ -7,6 +7,6 @@ func (at *AllowTransfer) IsDisabled() bool { // By default, the transfer is disabled. It is also disabled when it is none. // If any of the elements is not none, the transfer is enabled. return len(at.AddressMatchList.Elements) == 0 || !slices.ContainsFunc(at.AddressMatchList.Elements, func(ame *AddressMatchListElement) bool { - return ame.ACLName != "none" + return ame.IPAddressOrACLName != "none" }) } diff --git a/backend/appcfg/bind9/allowtransfer_test.go b/backend/appcfg/bind9/allowtransfer_test.go index fd38383e3..e9719c336 100644 --- a/backend/appcfg/bind9/allowtransfer_test.go +++ b/backend/appcfg/bind9/allowtransfer_test.go @@ -22,7 +22,7 @@ func TestAllowTransferIsDisabledNone(t *testing.T) { AddressMatchList: &AddressMatchList{ Elements: []*AddressMatchListElement{ { - ACLName: "none", + IPAddressOrACLName: "none", }, }, }, @@ -37,10 +37,10 @@ func TestAllowTransferIsNotDisabled(t *testing.T) { AddressMatchList: &AddressMatchList{ Elements: []*AddressMatchListElement{ { - ACLName: "none", + IPAddressOrACLName: "none", }, { - IPAddress: "127.0.0.1", + IPAddressOrACLName: "127.0.0.1", }, }, }, diff --git a/backend/appcfg/bind9/config.go b/backend/appcfg/bind9/config.go index eb1befb11..55d7e0f34 100644 --- a/backend/appcfg/bind9/config.go +++ b/backend/appcfg/bind9/config.go @@ -10,6 +10,17 @@ import ( const DefaultViewName = "_default" +// Checks if the configuration contains no-parse directives. +func (c *Config) HasNoParse() bool { + for _, statement := range c.Statements { + if statement.HasNoParse() { + return true + } + } + return false +} + +// Returns the options or nil if the options are not found. func (c *Config) GetOptions() *Options { for _, statement := range c.Statements { if statement.Options != nil { @@ -78,9 +89,9 @@ func (c *Config) getKeyFromAddressMatchList(level int, addressMatchList *Address case element.ACL != nil: // Recursively search for a key in the inline ACL. return c.getKeyFromAddressMatchList(level+1, element.ACL.AddressMatchList) - case element.ACLName != "": + case element.IPAddressOrACLName != "": // Recursively search for a key in the referenced ACL. - acl := c.GetACL(element.ACLName) + acl := c.GetACL(element.IPAddressOrACLName) if acl != nil { return c.getKeyFromAddressMatchList(level+1, acl.AddressMatchList) } diff --git a/backend/appcfg/bind9/config_test.go b/backend/appcfg/bind9/config_test.go index 7746aacfe..18fe9c785 100644 --- a/backend/appcfg/bind9/config_test.go +++ b/backend/appcfg/bind9/config_test.go @@ -7,6 +7,25 @@ import ( "github.com/stretchr/testify/require" ) +// Test checking if the configuration contains no-parse directives. +func TestConfigHasNoParse(t *testing.T) { + cfg := &Config{ + Statements: []*Statement{ + {Options: &Options{ + Clauses: []*OptionClause{}, + }}, + {NoParse: &NoParse{}}, + }, + } + require.True(t, cfg.HasNoParse()) +} + +// Test checking if the configuration does not contain no-parse directives. +func TestConfigHasNoParseNone(t *testing.T) { + cfg := &Config{} + require.False(t, cfg.HasNoParse()) +} + // Tests that GetView returns expected view. func TestGetView(t *testing.T) { cfg, err := NewParser().ParseFile("testdata/named.conf") diff --git a/backend/appcfg/bind9/listenon.go b/backend/appcfg/bind9/listenon.go index 105b83e4b..f352d7786 100644 --- a/backend/appcfg/bind9/listenon.go +++ b/backend/appcfg/bind9/listenon.go @@ -16,7 +16,7 @@ func GetDefaultListenOnClauses() *ListenOnClauses { AddressMatchList: &AddressMatchList{ Elements: []*AddressMatchListElement{ { - IPAddress: "127.0.0.1", + IPAddressOrACLName: "127.0.0.1", }, }, }, @@ -67,8 +67,8 @@ func (l *ListenOn) GetPreferredIPAddress(allowTransferMatchList *AddressMatchLis return "::1" } for _, element := range l.AddressMatchList.Elements { - if element.IPAddress != "" && !element.Negation && !allowTransferMatchList.ExcludesIPAddress(element.IPAddress) { - return element.IPAddress + if element.IPAddressOrACLName != "" && !element.Negation && !allowTransferMatchList.ExcludesIPAddress(element.IPAddressOrACLName) { + return element.IPAddressOrACLName } } return "" @@ -86,7 +86,7 @@ func (l *ListenOn) GetPort() int64 { // Checks if the listen-on clause includes the specified IP address. func (l *ListenOn) IncludesIPAddress(ipAddress string) bool { for _, element := range l.AddressMatchList.Elements { - if element.IPAddress == ipAddress && !element.Negation { + if element.IPAddressOrACLName == ipAddress && !element.Negation { return true } } diff --git a/backend/appcfg/bind9/listenon_test.go b/backend/appcfg/bind9/listenon_test.go index ca0eda242..52ac813fc 100644 --- a/backend/appcfg/bind9/listenon_test.go +++ b/backend/appcfg/bind9/listenon_test.go @@ -13,7 +13,7 @@ func TestGetDefaultListenOnClauses(t *testing.T) { listenOnClauses := GetDefaultListenOnClauses() require.Len(t, *listenOnClauses, 1) require.Len(t, (*listenOnClauses)[0].AddressMatchList.Elements, 1) - require.Equal(t, "127.0.0.1", (*listenOnClauses)[0].AddressMatchList.Elements[0].IPAddress) + require.Equal(t, "127.0.0.1", (*listenOnClauses)[0].AddressMatchList.Elements[0].IPAddressOrACLName) require.Equal(t, int64(53), (*listenOnClauses)[0].GetPort()) require.True(t, (*listenOnClauses)[0].IncludesIPAddress("127.0.0.1")) require.False(t, (*listenOnClauses)[0].IncludesIPAddress("0.0.0.0")) @@ -28,7 +28,7 @@ func TestGetMatchingListenOnDefault(t *testing.T) { require.NotNil(t, listenOn) require.Len(t, *listenOnClauses, 1) require.Len(t, (*listenOnClauses)[0].AddressMatchList.Elements, 1) - require.Equal(t, "127.0.0.1", (*listenOnClauses)[0].AddressMatchList.Elements[0].IPAddress) + require.Equal(t, "127.0.0.1", (*listenOnClauses)[0].AddressMatchList.Elements[0].IPAddressOrACLName) require.Equal(t, int64(53), listenOn.GetPort()) } @@ -38,19 +38,19 @@ func TestGetMatchingListenOnMultipleZeroAddress(t *testing.T) { listenOnClauses := ListenOnClauses{ &ListenOn{ AddressMatchList: &AddressMatchList{ - Elements: []*AddressMatchListElement{{IPAddress: "192.0.2.1"}}, + Elements: []*AddressMatchListElement{{IPAddressOrACLName: "192.0.2.1"}}, }, }, &ListenOn{ AddressMatchList: &AddressMatchList{ - Elements: []*AddressMatchListElement{{IPAddress: "0.0.0.0"}}, + Elements: []*AddressMatchListElement{{IPAddressOrACLName: "0.0.0.0"}}, }, }, } listenOn := listenOnClauses.GetMatchingListenOn(53) require.NotNil(t, listenOn) require.Len(t, listenOn.AddressMatchList.Elements, 1) - require.Equal(t, "0.0.0.0", listenOn.AddressMatchList.Elements[0].IPAddress) + require.Equal(t, "0.0.0.0", listenOn.AddressMatchList.Elements[0].IPAddressOrACLName) require.Equal(t, int64(53), listenOn.GetPort()) } @@ -60,19 +60,19 @@ func TestGetMatchingListenOnMultipleLoopbackAddress(t *testing.T) { listenOnClauses := ListenOnClauses{ &ListenOn{ AddressMatchList: &AddressMatchList{ - Elements: []*AddressMatchListElement{{IPAddress: "192.0.2.1"}}, + Elements: []*AddressMatchListElement{{IPAddressOrACLName: "192.0.2.1"}}, }, }, &ListenOn{ AddressMatchList: &AddressMatchList{ - Elements: []*AddressMatchListElement{{IPAddress: "127.0.0.1"}}, + Elements: []*AddressMatchListElement{{IPAddressOrACLName: "127.0.0.1"}}, }, }, } listenOn := listenOnClauses.GetMatchingListenOn(53) require.NotNil(t, listenOn) require.Len(t, listenOn.AddressMatchList.Elements, 1) - require.Equal(t, "127.0.0.1", listenOn.AddressMatchList.Elements[0].IPAddress) + require.Equal(t, "127.0.0.1", listenOn.AddressMatchList.Elements[0].IPAddressOrACLName) require.Equal(t, int64(53), listenOn.GetPort()) } @@ -82,20 +82,20 @@ func TestGetMatchingListenOnMultipleLoopbackAddressPortNumber(t *testing.T) { listenOnClauses := ListenOnClauses{ &ListenOn{ AddressMatchList: &AddressMatchList{ - Elements: []*AddressMatchListElement{{IPAddress: "192.0.2.1"}}, + Elements: []*AddressMatchListElement{{IPAddressOrACLName: "192.0.2.1"}}, }, Port: storkutil.Ptr(int64(853)), }, &ListenOn{ AddressMatchList: &AddressMatchList{ - Elements: []*AddressMatchListElement{{IPAddress: "127.0.0.1"}}, + Elements: []*AddressMatchListElement{{IPAddressOrACLName: "127.0.0.1"}}, }, }, } listenOn := listenOnClauses.GetMatchingListenOn(853) require.NotNil(t, listenOn) require.Len(t, listenOn.AddressMatchList.Elements, 1) - require.Equal(t, "192.0.2.1", listenOn.AddressMatchList.Elements[0].IPAddress) + require.Equal(t, "192.0.2.1", listenOn.AddressMatchList.Elements[0].IPAddressOrACLName) require.Equal(t, int64(853), listenOn.GetPort()) } @@ -105,19 +105,19 @@ func TestGetMatchingListenOnMultipleZeroAddressIPv6(t *testing.T) { listenOnClauses := ListenOnClauses{ &ListenOn{ AddressMatchList: &AddressMatchList{ - Elements: []*AddressMatchListElement{{IPAddress: "2001:db8:1::1"}}, + Elements: []*AddressMatchListElement{{IPAddressOrACLName: "2001:db8:1::1"}}, }, }, &ListenOn{ AddressMatchList: &AddressMatchList{ - Elements: []*AddressMatchListElement{{IPAddress: "::"}}, + Elements: []*AddressMatchListElement{{IPAddressOrACLName: "::"}}, }, }, } listenOn := listenOnClauses.GetMatchingListenOn(53) require.NotNil(t, listenOn) require.Len(t, listenOn.AddressMatchList.Elements, 1) - require.Equal(t, "::", listenOn.AddressMatchList.Elements[0].IPAddress) + require.Equal(t, "::", listenOn.AddressMatchList.Elements[0].IPAddressOrACLName) require.Equal(t, int64(53), listenOn.GetPort()) } @@ -127,19 +127,19 @@ func TestGetMatchingListenOnMultipleLoopbackAddressIPv6(t *testing.T) { listenOnClauses := ListenOnClauses{ &ListenOn{ AddressMatchList: &AddressMatchList{ - Elements: []*AddressMatchListElement{{IPAddress: "2001:db8:1::1"}}, + Elements: []*AddressMatchListElement{{IPAddressOrACLName: "2001:db8:1::1"}}, }, }, &ListenOn{ AddressMatchList: &AddressMatchList{ - Elements: []*AddressMatchListElement{{IPAddress: "::1"}}, + Elements: []*AddressMatchListElement{{IPAddressOrACLName: "::1"}}, }, }, } listenOn := listenOnClauses.GetMatchingListenOn(53) require.NotNil(t, listenOn) require.Len(t, listenOn.AddressMatchList.Elements, 1) - require.Equal(t, "::1", listenOn.AddressMatchList.Elements[0].IPAddress) + require.Equal(t, "::1", listenOn.AddressMatchList.Elements[0].IPAddressOrACLName) require.Equal(t, int64(53), listenOn.GetPort()) } @@ -148,19 +148,19 @@ func TestGetMatchingListenOnMultipleLoopbackAddressPortNumberIPv6(t *testing.T) listenOnClauses := ListenOnClauses{ &ListenOn{ AddressMatchList: &AddressMatchList{ - Elements: []*AddressMatchListElement{{IPAddress: "2001:db8:1::1"}}, + Elements: []*AddressMatchListElement{{IPAddressOrACLName: "2001:db8:1::1"}}, }, Port: storkutil.Ptr(int64(853)), }, &ListenOn{ AddressMatchList: &AddressMatchList{ - Elements: []*AddressMatchListElement{{IPAddress: "::1"}}, + Elements: []*AddressMatchListElement{{IPAddressOrACLName: "::1"}}, }, }, } listenOn := listenOnClauses.GetMatchingListenOn(853) require.NotNil(t, listenOn) require.Len(t, listenOn.AddressMatchList.Elements, 1) - require.Equal(t, "2001:db8:1::1", listenOn.AddressMatchList.Elements[0].IPAddress) + require.Equal(t, "2001:db8:1::1", listenOn.AddressMatchList.Elements[0].IPAddressOrACLName) require.Equal(t, int64(853), listenOn.GetPort()) } diff --git a/backend/appcfg/bind9/options.go b/backend/appcfg/bind9/options.go index b35048df2..4b7fa80eb 100644 --- a/backend/appcfg/bind9/options.go +++ b/backend/appcfg/bind9/options.go @@ -1,5 +1,15 @@ package bind9config +// Checks if the options contain no-parse directives. +func (o *Options) HasNoParse() bool { + for _, clause := range o.Clauses { + if clause.NoParse != nil { + return true + } + } + return false +} + // Gets the allow-transfer clause from options. func (o *Options) GetAllowTransfer() *AllowTransfer { for _, clause := range o.Clauses { diff --git a/backend/appcfg/bind9/options_test.go b/backend/appcfg/bind9/options_test.go index 267f064bf..52cba3ba4 100644 --- a/backend/appcfg/bind9/options_test.go +++ b/backend/appcfg/bind9/options_test.go @@ -7,6 +7,22 @@ import ( storkutil "isc.org/stork/util" ) +// Test checking if the options contains no-parse directives. +func TestOptionsHasNoParse(t *testing.T) { + options := &Options{ + Clauses: []*OptionClause{ + {NoParse: &NoParse{}}, + }, + } + require.True(t, options.HasNoParse()) +} + +// Test checking if the options does not contain no-parse directives. +func TestOptionsHasNoParseNone(t *testing.T) { + options := &Options{} + require.False(t, options.HasNoParse()) +} + // Test getting the allow-transfer clause from options. func TestOptionsGetAllowTransferPort(t *testing.T) { options := &Options{ diff --git a/backend/appcfg/bind9/parser.go b/backend/appcfg/bind9/parser.go index 29a696535..73c4c7d1a 100644 --- a/backend/appcfg/bind9/parser.go +++ b/backend/appcfg/bind9/parser.go @@ -1,10 +1,10 @@ package bind9config import ( - "fmt" "io" "os" "path/filepath" + "strings" "sync" "github.com/alecthomas/participle/v2" @@ -12,16 +12,6 @@ import ( "github.com/pkg/errors" ) -// The following patterns have been copied from the ip-num library: -// See https://github.com/ip-num/ip-num/blob/master/src/Validator.ts. -// -// The original IPv6 range pattern was corrected by adding proper escaping -// to digital character wildcards and any character wildcards. -const ( - ipv6MatchPattern = `((([0-9A-Fa-f]{1,4}:){7}([0-9A-Fa-f]{1,4}|:))|(([0-9A-Fa-f]{1,4}:){6}(:[0-9A-Fa-f]{1,4}|((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){5}(((:[0-9A-Fa-f]{1,4}){1,2})|:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){4}(((:[0-9A-Fa-f]{1,4}){1,3})|((:[0-9A-Fa-f]{1,4})?:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){3}(((:[0-9A-Fa-f]{1,4}){1,4})|((:[0-9A-Fa-f]{1,4}){0,2}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){2}(((:[0-9A-Fa-f]{1,4}){1,5})|((:[0-9A-Fa-f]{1,4}){0,3}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){1}(((:[0-9A-Fa-f]{1,4}){1,6})|((:[0-9A-Fa-f]{1,4}){0,4}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(:(((:[0-9A-Fa-f]{1,4}){1,7})|((:[0-9A-Fa-f]{1,4}){0,5}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:)))(%.+)?` - ipv6RangePattern = `(?:(?:([0-9A-Fa-f]{1,4}:){7}(?:[0-9A-Fa-f]{1,4}|:))|(?:([0-9A-Fa-f]{1,4}:){6}(?::[0-9A-Fa-f]{1,4}|(?:(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(?:\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(?:(?:[0-9A-Fa-f]{1,4}:){5}(?:(?:(?::[0-9A-Fa-f]{1,4}){1,2})|:(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(?:(?:[0-9A-Fa-f]{1,4}:){4}(?:(?:(?::[0-9A-Fa-f]{1,4}){1,3})|(?:(?::[0-9A-Fa-f]{1,4})?:(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(?:(?:[0-9A-Fa-f]{1,4}:){3}(?:(?:(?::[0-9A-Fa-f]{1,4}){1,4})|(?:(?::[0-9A-Fa-f]{1,4}){0,2}:(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(?:(?:[0-9A-Fa-f]{1,4}:){2}(?:(?:(?::[0-9A-Fa-f]{1,4}){1,5})|(?:(?::[0-9A-Fa-f]{1,4}){0,3}:(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(?:(?:[0-9A-Fa-f]{1,4}:){1}(?:(?:(?::[0-9A-Fa-f]{1,4}){1,6})|(?:(?::[0-9A-Fa-f]{1,4}){0,4}:(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(?::(?:(?:(?::[0-9A-Fa-f]{1,4}){1,7})|(?:(?::[0-9A-Fa-f]{1,4}){0,5}:(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:)))(?:%.+)?\s*(?:/(?:12[0-8]|1[0-1][0-9]|[1-9][0-9]|[0-9]))?` -) - // Config is the root of the Bind9 configuration. It contains a list of // top-level statements. The statements typically contain clauses with // configuration elements. @@ -35,8 +25,13 @@ type Config struct { // Statement is a single top-level configuration element. type Statement struct { + // A Stork-specific annotation to skip parsing statements between the + // @stork:no-parse:scope and @stork:no-parse:end directives, or after + // the @stork:no-parse:global directive. + NoParse *NoParse `parser:"@@"` + // The "include statement is used to include another configuration file. - Include *Include `parser:"'include' @@"` + Include *Include `parser:"| 'include' @@"` // The "acl" statement is used to define an access control list. ACL *ACL `parser:"| 'acl' @@"` @@ -67,6 +62,62 @@ type Statement struct { UnnamedStatement *UnnamedStatement `parser:"| @@"` } +// A Stork-specific annotation to skip parsing statements between the +// @stork:no-parse:scope and @stork:no-parse:end directives, or after +// the @stork:no-parse:global directive. +type NoParse struct { + NoParseScope *NoParseScope `parser:"( @@"` + NoParseGlobal *NoParseGlobal `parser:"| @@ )"` +} + +// Checks if the @stork:no-parse:global directive was used. +func (n *NoParse) IsGlobal() bool { + return n.NoParseGlobal != nil +} + +// Returns the unparsed contents within the @stork:no-parse:scope +// and @stork:no-parse:end directives, or after the @stork:no-parse:global +// directive. +func (n *NoParse) GetContentsString() string { + switch { + case n.NoParseScope != nil: + return n.NoParseScope.Contents.GetString() + case n.NoParseGlobal != nil: + return n.NoParseGlobal.Contents.GetString() + default: + return "" + } +} + +// Represents the @stork:no-parse:scope/@stork:no-parse:end directives. +type NoParseScope struct { + Preamble string `parser:"@NoParseScope"` + Contents RawContents `parser:"@NoParseContents"` + End string `parser:"@NoParseEnd"` +} + +// Represents the @stork:no-parse:global directive. +type NoParseGlobal struct { + Preamble string `parser:"@NoParseGlobal"` + Contents RawContents `parser:"@NoParseGlobalContents"` +} + +// Unparsed contents between the @stork:no-parse:scope and @stork:no-parse:end +// directives, or after the @stork:no-parse:global directive. +type RawContents string + +// Captures the unparsed contents between the @stork:no-parse:scope +// and @stork:no-parse:end directives and removes the trailing +// @stork:no-parse: suffix which is appended by the lexer. +func (c *RawContents) Capture(values []string) error { + if len(values) == 0 { + return nil + } + values[len(values)-1] = strings.TrimSuffix(values[len(values)-1], "//@stork:no-parse:") + *c = RawContents(strings.Join(values, "")) + return nil +} + // Include is the statement used to include another configuration file. // The included file can be parsed and its configuration statements expand // the parent configuration. The "include" statement has the following format: @@ -107,11 +158,10 @@ type AddressMatchList struct { // AddressMatchListElement is an element of an address match list. type AddressMatchListElement struct { - Negation bool `parser:"@('!')?"` - ACL *ACL `parser:"( '{' @@ '}'"` - KeyID string `parser:"| ( 'key' ( @Ident | @String ) )"` - IPAddress string `parser:"| ( @IPv4Address | @IPv6AddressRange | @IPv6Address | @IPv4AddressQuoted | @IPv6AddressRangeQuoted | @IPv6AddressQuoted )"` - ACLName string `parser:"| ( @Ident | @String ) )"` + Negation bool `parser:"@('!')?"` + ACL *ACL `parser:"( '{' @@ '}'"` + KeyID string `parser:"| ( 'key' ( @Ident | @String ) )"` + IPAddressOrACLName string `parser:"| ( @Ident | @String ) )"` } // Key is the statement used to define an algorithm and secret. It has the following @@ -153,13 +203,17 @@ type Options struct { // The response-policy clause cache for better access performance. responsePolicy *ResponsePolicy // The list of clauses (e.g., allow-transfer, listen-on, response-policy etc.). - Clauses []*OptionClause `parser:"'{' ( @@ ';'+ )* '}'"` + Clauses []*OptionClause `parser:"'{' ( @@ ';'* )* '}'"` } // OptionClause is a single clause of an options statement. type OptionClause struct { + // A Stork-specific annotation to skip parsing statements between the + // @stork:no-parse:scope and @stork:no-parse:end directives, or after + // the @stork:no-parse:global directive. + NoParse *NoParse `parser:"@@"` // The allow-transfer clause restricting who can perform AXFR. - AllowTransfer *AllowTransfer `parser:"'allow-transfer' @@"` + AllowTransfer *AllowTransfer `parser:"| 'allow-transfer' @@"` // The listen-on clause specifying the addresses the server listens // on the DNS requests. ListenOn *ListenOn `parser:"| 'listen-on' @@"` @@ -196,8 +250,12 @@ type View struct { // ViewClause is a single clause of a view statement. type ViewClause struct { + // A Stork-specific annotation to skip parsing statements between the + // @stork:no-parse:scope and @stork:no-parse:end directives, or after + // the @stork:no-parse:global directive. + NoParse *NoParse `parser:"@@"` // The match-clients clause associating the view with ACLs. - MatchClients *MatchClients `parser:"'match-clients' @@"` + MatchClients *MatchClients `parser:"| 'match-clients' @@"` // The allow-transfer clause restricting who can perform AXFR. AllowTransfer *AllowTransfer `parser:"| 'allow-transfer' @@"` // The response-policy clause specifying the response policy zones. @@ -220,14 +278,17 @@ type Zone struct { Name string `parser:"( @String | @Ident )"` // The class of the zone statement. Class string `parser:"( @String | @Ident )?"` - // The list of clauses (e.g., match-clients, zone etc.). - Clauses []*ZoneClause `parser:"'{' ( @@ ';'* )* '}'"` + // The list of clauses (e.g., match-clients, allow-transfer etc.). + // This is made optional to allow quicker parsing of the zone definition, + // with the zone-level options elided. + Clauses []*ZoneClause `parser:"( '{' ( @@ ';'* )* '}' )?"` } // ZoneClause is a single clause of a zone statement. type ZoneClause struct { + NoParse *NoParse `parser:"@@"` // The allow-transfer clause restricting who can perform AXFR. - AllowTransfer *AllowTransfer `parser:"'allow-transfer' @@"` + AllowTransfer *AllowTransfer `parser:"| 'allow-transfer' @@"` // Any option clause. Option *Option `parser:"| @@"` } @@ -241,7 +302,7 @@ type MatchClients struct { // AllowTransfer is the clause for restricting who can perform AXFR // globally, for a particular view or zone. type AllowTransfer struct { - Port *int64 `parser:"( 'port' @Number )?"` + Port *int64 `parser:"( 'port' @Ident )?"` Transport *string `parser:"( 'transport' ( @String | @Ident ) )?"` AddressMatchList *AddressMatchList `parser:"'{' @@ '}'"` } @@ -249,7 +310,7 @@ type AllowTransfer struct { // ListenOn is the clause specifying the addresses the servers listens on the // DNS requests. It also contains additional options. type ListenOn struct { - Port *int64 `parser:"( 'port' @Number )?"` + Port *int64 `parser:"( 'port' @Ident )?"` Proxy *string `parser:"( 'proxy' ( @String | @Ident ) )?"` TLS *string `parser:"( 'tls' ( @String | @Ident ) )?"` HTTP *string `parser:"( 'http' ( @String | @Ident ) )?"` @@ -259,13 +320,13 @@ type ListenOn struct { // ResponsePolicy is the clause specifying the response policy zones. type ResponsePolicy struct { Zones []*ResponsePolicyZone `parser:"'{' ( @@ ';'+ )* '}'"` - Switches []string `parser:"( @String | @Ident | @Number | @Asterisk )*"` + Switches []string `parser:"( @String | @Ident )*"` } // ResponsePolicyZone is a single response policy zone entry. type ResponsePolicyZone struct { Zone string `parser:"'zone' ( @String | @Ident )"` - Switches []string `parser:"( @String | @Ident | @Number | @Asterisk )*"` + Switches []string `parser:"( @String | @Ident )*"` } // NamedStatement is a generic catch-all named statement. It is used to parse @@ -306,7 +367,7 @@ type UnnamedStatement struct { // Many options in the options statement have this format. type Option struct { Identifier string `parser:"@Ident"` - Switches []string `parser:"( @IPv4Address | @IPv6AddressRange | @IPv6Address | @IPv4AddressQuoted | @IPv6AddressRangeQuoted | @IPv6AddressQuoted | @String | @Ident | @Number | @Asterisk )*"` + Switches []string `parser:"( @String | @Ident )*"` Contents *GenericClauseContents `parser:"( '{' @@ '}' )?"` Suboptions []Suboption `parser:"( @@ )*"` } @@ -315,7 +376,7 @@ type Option struct { // option. Suboptions can appear after curly braces in the option. type Suboption struct { Identifier string `parser:"@Ident"` - Switches []string `parser:"( @IPv4Address | @IPv6AddressRange | @IPv6Address | @IPv4AddressQuoted | @IPv6AddressRangeQuoted | @IPv6AddressQuoted | @String | @Ident | @Number | @Asterisk )*"` + Switches []string `parser:"( @String | @Ident )*"` Contents *GenericClauseContents `parser:"( '{' @@ '}' )?"` } @@ -353,6 +414,76 @@ func (b *GenericClauseContents) Parse(lex *lexer.PeekingLexer) error { } } +var ( + // Custom lexer. It is used to tokenize the input stream into tokens + // meaningful for the named configuration parser. It drops the comments + // and whitespace. It also drops the configuration parts annotated with + // the @stork:no-parse directives. For example, to skip parsing a given + // zone definition, annotate it with: + // + // //@stork:no-parse:scope + // zone "example.com" { + // type master; + // allow-transfer port 853 { any; }; + // file "/etc/bind/db.example.com"; + // }; + // //@stork:no-parse:end + // + // If only specific parts of the zone definition should be skipped, one + // can do: + // + // zone "example.com" { + // //@stork:no-parse:scope + // type master; + // file "/etc/bind/db.example.com"; + // //@stork:no-parse:end + // allow-transfer port 853 { any; }; + // }; + // + // The @stork:no-parse directive can be used for other statements as well. + // It is not limited to the zone definition. For example, it can be used + // to skip parsing an included file, options, views and the inner statements + // within these configuration elements. + // + // If the interesting configuration part is at the beginning of a file and + // the parse to be skipped is at the end, use the @stork:no-parse:global + // directive to annotate the rest of the file to be skipped. + //nolint:gochecknoglobals + bind9Lexer = lexer.MustStateful(lexer.Rules{ + "Root": { + {Name: "noParse", Pattern: `//@stork:no-parse:`, Action: lexer.Push("NoParse")}, + {Name: "comment", Pattern: `(//|#)[^\n]*`}, + {Name: "cppStyleComment", Pattern: `\/\*([^*]|(\*+[^*\/]))*\*+\/`}, + {Name: "String", Pattern: `"(\\"|[^"])*"`}, + {Name: "Ident", Pattern: `[0-9a-zA-Z-_\.\:\/\*]+`}, + {Name: "whitespace", Pattern: `[ \t\n\r]+`}, + {Name: "Punct", Pattern: `[;,{}!]`}, + }, + "NoParse": { + {Name: "NoParseScope", Pattern: `scope`, Action: lexer.Push("NoParseScope")}, + {Name: "NoParseGlobal", Pattern: `global`, Action: lexer.Push("NoParseGlobal")}, + {Name: "NoParseEnd", Pattern: `end`, Action: lexer.Pop()}, + lexer.Return(), + }, + "NoParseScope": { + {Name: "NoParseContents", Pattern: `[\S\s]*?//@stork:no-parse:`, Action: lexer.Pop()}, + lexer.Return(), + }, + "NoParseGlobal": { + {Name: "NoParseGlobalContents", Pattern: `[\s\S]*`}, + }, + }) + + // The parser uses the custom lexer. + //nolint:gochecknoglobals + bind9Parser = participle.MustBuild[Config]( + // Use custom lexer instead of the default one. + participle.Lexer(bind9Lexer), + // Remove quotes from the strings and other quoted tokens. + participle.Unquote("String"), + ) +) + // Parser is a parser for the BIND 9 configuration. type Parser struct{} @@ -361,72 +492,8 @@ func NewParser() *Parser { return &Parser{} } -// Parses the BIND 9 configuration from a file using custom lexer. -func (p *Parser) ParseFile(filename string) (*Config, error) { - file, err := os.Open(filename) - if err != nil { - return nil, errors.Wrapf(err, "failed to open BIND 9 config file: %s", filename) - } - defer file.Close() - return p.Parse(filename, file) -} - -// Parses the BIND 9 configuration using custom lexer. -func (p *Parser) Parse(filename string, fileReader io.Reader) (*Config, error) { - // Define the custom lexer. It is used to tokenize the input stream - // into tokens meaningful for named configuration parser. Note that - // many of the rules below can be considered simplistic (e.g., the - // IPv4 or IPv6 address matching rules). However, it is not the purpose - // of this parser to validate the named configuration file syntax. - // Bind is responsible for validating it. We just want to reliably - // recognize the tokens in the named configuration file. - lexer := lexer.MustSimple([]lexer.SimpleRule{ - // Comments can begin with either "//" or "#". They are elided from - // the token stream. - {Name: "Comment", Pattern: `(//|#)[^\n]*`}, - // C-style comments are also elided from the token stream. - {Name: "CppStyleComment", Pattern: `\/\*([^*]|(\*+[^*\/]))*\*+\/`}, - // IPv4 addresses and subnets can be specified with or without quotes. - // This variant assumes the lack of quotes. - {Name: "IPv4Address", Pattern: `(?:([0-9]{1,3}\.){3}(?:[0-9]{1,3}))(?:/(?:[0-9]{1,2}))?`}, - {Name: "IPv6AddressRange", Pattern: ipv6RangePattern}, - // IPv6 addresses and subnets can be specified with or without quotes. - // This variant assumes the lack of quotes. - {Name: "IPv6Address", Pattern: ipv6MatchPattern}, - // IPv4 addresses and subnets can be specified with quotes. - {Name: "IPv4AddressQuoted", Pattern: `"(?:([0-9]{1,3}\.){3}(?:[0-9]{1,3}))(?:/(?:[0-9]{1,2}))?"`}, - {Name: "IPv6AddressRangeQuoted", Pattern: fmt.Sprintf(`"%s"`, ipv6RangePattern)}, - // IPv6 addresses and subnets can be specified with quotes. - {Name: "IPv6AddressQuoted", Pattern: fmt.Sprintf(`"%s"`, ipv6MatchPattern)}, - // Strings are always quoted. - {Name: "String", Pattern: `"(\\"|[^"])*"`}, - // Numbers. - {Name: "Number", Pattern: `[-+]?(\d*\.)?\d+`}, - // Identifiers are alphanumeric strings specified without quotes. - // Note that the Bind9 configuration parser allows for specifying - // configuration element names (and values) in quotes or without quotes. - // The identifier handles this second case. - {Name: "Ident", Pattern: `[0-9a-zA-Z-_\.]+`}, - // Asterisk. - {Name: "Asterisk", Pattern: `\*`}, - // Punctuation characters. - {Name: "Punct", Pattern: `[;,.{}!*]`}, - // Whitespace characters. - {Name: "Whitespace", Pattern: `[ \t\n\r]+`}, - // End of line characters. - {Name: "EOL", Pattern: `[\n\r]+`}, - }) - - parser := participle.MustBuild[Config]( - // Use custom lexer instead of the default one. - participle.Lexer(lexer), - // Remove quotes from the strings and other quoted tokens. - participle.Unquote("String", "IPv4AddressQuoted", "IPv6AddressQuoted", "IPv6AddressRangeQuoted"), - // Ignore whitespace and comments. - participle.Elide("Whitespace", "Comment", "CppStyleComment"), - // Use lookahead to improve the parsing accuracy. - participle.UseLookahead(2), - ) +// Parses the BIND 9 configuration from a file using a custom parser. +func (p *Parser) parse(filename string, fileReader io.Reader, parser *participle.Parser[Config]) (*Config, error) { // Run the parser. config, err := parser.Parse(filename, fileReader) if err != nil { @@ -439,3 +506,18 @@ func (p *Parser) Parse(filename string, fileReader io.Reader) (*Config, error) { } return config, nil } + +// Parses the BIND 9 configuration from a file. +func (p *Parser) ParseFile(filename string) (*Config, error) { + file, err := os.Open(filename) + if err != nil { + return nil, errors.Wrapf(err, "failed to open BIND 9 config file: %s", filename) + } + defer file.Close() + return p.Parse(filename, file) +} + +// Parses the BIND 9 configuration. +func (p *Parser) Parse(filename string, fileReader io.Reader) (*Config, error) { + return p.parse(filename, fileReader, bind9Parser) +} diff --git a/backend/appcfg/bind9/parser_test.go b/backend/appcfg/bind9/parser_test.go index 2d85c89cb..2cf3f46b0 100644 --- a/backend/appcfg/bind9/parser_test.go +++ b/backend/appcfg/bind9/parser_test.go @@ -75,7 +75,7 @@ func TestParseFile(t *testing.T) { require.Nil(t, statement.Options.Clauses[1].AllowTransfer.Transport) require.NotNil(t, statement.Options.Clauses[1].AllowTransfer.AddressMatchList) require.Len(t, statement.Options.Clauses[1].AllowTransfer.AddressMatchList.Elements, 1) - require.Equal(t, "any", statement.Options.Clauses[1].AllowTransfer.AddressMatchList.Elements[0].ACLName) + require.Equal(t, "any", statement.Options.Clauses[1].AllowTransfer.AddressMatchList.Elements[0].IPAddressOrACLName) require.NotNil(t, statement.Options.Clauses[2].Option) require.Equal(t, "also-notify", statement.Options.Clauses[2].Option.Identifier) require.NotNil(t, statement.Options.Clauses[3].Option) @@ -95,11 +95,11 @@ func TestParseFile(t *testing.T) { require.Equal(t, "myserver", *statement.Options.Clauses[6].ListenOn.HTTP) require.NotNil(t, statement.Options.Clauses[6].ListenOn.AddressMatchList) require.Len(t, statement.Options.Clauses[6].ListenOn.AddressMatchList.Elements, 1) - require.Equal(t, "127.0.0.1", statement.Options.Clauses[6].ListenOn.AddressMatchList.Elements[0].IPAddress) + require.Equal(t, "127.0.0.1", statement.Options.Clauses[6].ListenOn.AddressMatchList.Elements[0].IPAddressOrACLName) require.NotNil(t, statement.Options.Clauses[7].ListenOnV6) require.NotNil(t, statement.Options.Clauses[7].ListenOnV6.AddressMatchList) require.Len(t, statement.Options.Clauses[7].ListenOnV6.AddressMatchList.Elements, 1) - require.Equal(t, "::1", statement.Options.Clauses[7].ListenOnV6.AddressMatchList.Elements[0].IPAddress) + require.Equal(t, "::1", statement.Options.Clauses[7].ListenOnV6.AddressMatchList.Elements[0].IPAddressOrACLName) require.NotNil(t, statement.Options.Clauses[8].ResponsePolicy) require.Len(t, statement.Options.Clauses[8].ResponsePolicy.Zones, 2) require.Equal(t, "rpz.example.com", statement.Options.Clauses[8].ResponsePolicy.Zones[0].Zone) @@ -180,6 +180,208 @@ func TestParseFile(t *testing.T) { require.Equal(t, "logging", statement.UnnamedStatement.Identifier) } +// Test that the parser correctly handles the @stork:no-parse directive. +func TestNoParseSelectedZone(t *testing.T) { + cfg, err := NewParser().Parse(" ", strings.NewReader(` + zone "example.com" { + type forward; + }; + //@stork:no-parse:scope + zone "example.org" { + type forward; + }; + //@stork:no-parse:end + zone "example.net" { + type forward; + }; + `)) + require.NoError(t, err) + require.NotNil(t, cfg) + require.Len(t, cfg.Statements, 3) + require.NotNil(t, cfg.Statements[0].Zone) + require.Equal(t, "example.com", cfg.Statements[0].Zone.Name) + require.NotNil(t, cfg.Statements[1].NoParse) + require.False(t, cfg.Statements[1].NoParse.IsGlobal()) + require.Contains(t, cfg.Statements[1].NoParse.GetContentsString(), ` + zone "example.org" { + type forward; + }; + `) + require.NotNil(t, cfg.Statements[2].Zone) + require.Equal(t, "example.net", cfg.Statements[2].Zone.Name) +} + +// Test selectively skipping parsing the inner contents of a zone definition. +func TestNoParseSelectedZoneOptions(t *testing.T) { + cfg, err := NewParser().Parse(" ", strings.NewReader(` + zone "example.org" { + //@stork:no-parse:scope + type forward; + //@stork:no-parse:end + allow-transfer port 853 { any; }; + }; + `)) + require.NoError(t, err) + require.NotNil(t, cfg) + require.Len(t, cfg.Statements, 1) + require.NotNil(t, cfg.Statements[0].Zone) + require.Equal(t, "example.org", cfg.Statements[0].Zone.Name) + require.Len(t, cfg.Statements[0].Zone.Clauses, 2) + require.NotNil(t, cfg.Statements[0].Zone.Clauses[0].NoParse) + require.False(t, cfg.Statements[0].Zone.Clauses[0].NoParse.IsGlobal()) + require.Contains(t, cfg.Statements[0].Zone.Clauses[0].NoParse.GetContentsString(), "type forward;") + require.NotNil(t, cfg.Statements[0].Zone.Clauses[1].AllowTransfer) + require.EqualValues(t, 853, *cfg.Statements[0].Zone.Clauses[1].AllowTransfer.Port) +} + +// Test that the parser correctly handles the @stork:no-parse directive +// for the view options. +func TestNoParseViewOptions(t *testing.T) { + cfg, err := NewParser().Parse(" ", strings.NewReader(` + view "foo" { + zone "example.com" { + type primary; + }; + //@stork:no-parse:scope + zone "example.net" { + type primary; + }; + //@stork:no-parse:end + zone "example.org" { + type primary; + }; + }; + `)) + require.NoError(t, err) + require.NotNil(t, cfg) + require.Len(t, cfg.Statements, 1) + require.NotNil(t, cfg.Statements[0].View) + require.Equal(t, "foo", cfg.Statements[0].View.Name) + require.Len(t, cfg.Statements[0].View.Clauses, 3) + require.NotNil(t, cfg.Statements[0].View.Clauses[0].Zone) + require.Equal(t, "example.com", cfg.Statements[0].View.Clauses[0].Zone.Name) + require.NotNil(t, cfg.Statements[0].View.Clauses[1].NoParse) + require.False(t, cfg.Statements[0].View.Clauses[1].NoParse.IsGlobal()) + require.NotNil(t, cfg.Statements[0].View.Clauses[2].Zone) + require.Equal(t, "example.org", cfg.Statements[0].View.Clauses[2].Zone.Name) +} + +// Test that the parser correctly handles the @stork:no-parse directive +// for the options. +func TestNoParseOptions(t *testing.T) { + cfg, err := NewParser().Parse(" ", strings.NewReader(` + options { + allow-transfer port 853 { any; }; + //@stork:no-parse:scope + listen-on port 853 { 127.0.0.1; }; + //@stork:no-parse:end + listen-on-v6 port 853 { ::1; }; + }; + `)) + require.NoError(t, err) + require.NotNil(t, cfg) + require.Len(t, cfg.Statements, 1) + require.NotNil(t, cfg.Statements[0].Options) + require.Len(t, cfg.Statements[0].Options.Clauses, 3) +} + +// Test that an error is returned when the @stork:no-parse:scope is not +// followed by the @stork:no-parse:end directive. +func TestNoParseNoEnd(t *testing.T) { + _, err := NewParser().Parse(" ", strings.NewReader(` + zone "example.com" { + type forward; + }; + //@stork:no-parse:scope + zone "example.org" { + type forward; + }; + zone "example.net" { + type forward; + }; + `)) + require.Error(t, err) + require.ErrorContains(t, err, `expected `) +} + +// Test that the @stork:no-parse:global directive is correctly parsed +// and parsing the rest of the file is skipped. +func TestNoParseGlobal(t *testing.T) { + cfg, err := NewParser().Parse(" ", strings.NewReader(` + zone "example.com" { + type forward; + }; + //@stork:no-parse:global + zone "example.org" { + type forward; + }; + `)) + require.NoError(t, err) + require.Len(t, cfg.Statements, 2) + require.NotNil(t, cfg.Statements[0].Zone) + require.Equal(t, "example.com", cfg.Statements[0].Zone.Name) + require.Len(t, cfg.Statements[0].Zone.Clauses, 1) + require.NotNil(t, cfg.Statements[1].NoParse) + require.True(t, cfg.Statements[1].NoParse.IsGlobal()) + require.Contains(t, cfg.Statements[1].NoParse.GetContentsString(), ` + zone "example.org" { + type forward; + }; + `) +} + +// Test that an error is returned when the @stork:no-parse:global directive +// is used in the middle of a statement. +func TestNoParseGlobalMidStatement(t *testing.T) { + _, err := NewParser().Parse(" ", strings.NewReader(` + zone "example.com" { + //@stork:no-parse:global + type forward; + }; + `)) + require.Error(t, err) + require.ErrorContains(t, err, `(expected "}")`) +} + +// Test that the @stork:no-parse:end is ignored for the @stork:no-parse:global +// directive. +func TestNoParseGlobalExtraneousEnd(t *testing.T) { + cfg, err := NewParser().Parse(" ", strings.NewReader(` + zone "example.com" { + type forward; + }; + //@stork:no-parse:global + zone "example.org" { + type forward; + allow-transfer port 853 { any; }; + }; + //@stork:no-parse:end + zone "example.net" { + type forward; + }; + `)) + require.NoError(t, err) + require.NotNil(t, cfg) + require.Len(t, cfg.Statements, 2) + require.NotNil(t, cfg.Statements[0].Zone) + require.Equal(t, "example.com", cfg.Statements[0].Zone.Name) + require.NotNil(t, cfg.Statements[1].NoParse) + require.True(t, cfg.Statements[1].NoParse.IsGlobal()) +} + +// Test that an error is returned when the @stork:no-parse:end directive +// is used without the @stork:no-parse:scope directive. +func TestNoParseOnlyEnd(t *testing.T) { + _, err := NewParser().Parse(" ", strings.NewReader(` + //@stork:no-parse:end + zone "example.com" { + type forward; + }; + `)) + require.Error(t, err) + require.ErrorContains(t, err, `unexpected token "end"`) +} + // Test that an attempt to parse a non-existent file returns an error. func TestParseFileError(t *testing.T) { cfg, err := NewParser().ParseFile("testdata/non-existent.conf") @@ -241,13 +443,13 @@ func TestParseIncludes(t *testing.T) { require.NotNil(t, acl1) require.Equal(t, "test1", acl1.Name) require.Len(t, acl1.AddressMatchList.Elements, 1) - require.Equal(t, "1.2.3.4", acl1.AddressMatchList.Elements[0].IPAddress) + require.Equal(t, "1.2.3.4", acl1.AddressMatchList.Elements[0].IPAddressOrACLName) acl2 := cfg.GetACL("test2") require.NotNil(t, acl2) require.Equal(t, "test2", acl2.Name) require.Len(t, acl2.AddressMatchList.Elements, 1) - require.Equal(t, "0.0.0.0", acl2.AddressMatchList.Elements[0].IPAddress) + require.Equal(t, "0.0.0.0", acl2.AddressMatchList.Elements[0].IPAddressOrACLName) } // Test the case when the configuration file includes itself. @@ -283,7 +485,7 @@ func TestParseIncludeSelf(t *testing.T) { require.NotNil(t, cfg.Statements[1].ACL) require.Equal(t, "test", cfg.Statements[1].ACL.Name) require.Len(t, cfg.Statements[1].ACL.AddressMatchList.Elements, 1) - require.Equal(t, "1.2.3.4", cfg.Statements[1].ACL.AddressMatchList.Elements[0].IPAddress) + require.Equal(t, "1.2.3.4", cfg.Statements[1].ACL.AddressMatchList.Elements[0].IPAddressOrACLName) } // Test that the parser doesn't fail when parsing the query-source option. @@ -530,3 +732,161 @@ func TestParseOptionWithSuboptions(t *testing.T) { require.Equal(t, "update", cfg.Statements[0].Options.Clauses[0].Option.Suboptions[1].Identifier) require.Equal(t, "100", cfg.Statements[0].Options.Clauses[0].Option.Suboptions[1].Switches[0]) } + +// Test parsing ACL with negated key. +func TestParseACLWithNegatedKey(t *testing.T) { + cfgText := ` + acl "trusted-networks" { + !key guest-key; + } + ` + cfg, err := NewParser().Parse(" ", strings.NewReader(cfgText)) + require.NoError(t, err) + require.NotNil(t, cfg) + require.Len(t, cfg.Statements, 1) + require.NotNil(t, cfg.Statements[0].ACL) + require.Equal(t, "trusted-networks", cfg.Statements[0].ACL.Name) + require.NotNil(t, cfg.Statements[0].ACL.AddressMatchList) + require.Len(t, cfg.Statements[0].ACL.AddressMatchList.Elements, 1) + require.True(t, cfg.Statements[0].ACL.AddressMatchList.Elements[0].Negation) + require.Equal(t, "guest-key", cfg.Statements[0].ACL.AddressMatchList.Elements[0].KeyID) +} + +// Test parsing ACL with a key. +func TestParseACLWithKey(t *testing.T) { + cfgText := ` + acl "guest-networks" { + key "guest-key"; + } + ` + cfg, err := NewParser().Parse(" ", strings.NewReader(cfgText)) + require.NoError(t, err) + require.NotNil(t, cfg) + require.Len(t, cfg.Statements, 1) + require.NotNil(t, cfg.Statements[0].ACL) + require.Equal(t, "guest-networks", cfg.Statements[0].ACL.Name) + require.NotNil(t, cfg.Statements[0].ACL.AddressMatchList) + require.Len(t, cfg.Statements[0].ACL.AddressMatchList.Elements, 1) + require.False(t, cfg.Statements[0].ACL.AddressMatchList.Elements[0].Negation) + require.Equal(t, "guest-key", cfg.Statements[0].ACL.AddressMatchList.Elements[0].KeyID) +} + +// Test parsing ACL with an unquoted ACL name. +func TestParseACLWithUnquotedACLName(t *testing.T) { + cfgText := ` + acl "trusted-networks" { + localnets; + } + ` + cfg, err := NewParser().Parse(" ", strings.NewReader(cfgText)) + require.NoError(t, err) + require.NotNil(t, cfg) + require.Len(t, cfg.Statements, 1) + require.NotNil(t, cfg.Statements[0].ACL) + require.Equal(t, "trusted-networks", cfg.Statements[0].ACL.Name) + require.NotNil(t, cfg.Statements[0].ACL.AddressMatchList) + require.Len(t, cfg.Statements[0].ACL.AddressMatchList.Elements, 1) + require.Equal(t, "localnets", cfg.Statements[0].ACL.AddressMatchList.Elements[0].IPAddressOrACLName) +} + +// Test parsing ACL with a quoted ACL name. +func TestParseACLWithQuotedACLName(t *testing.T) { + cfgText := ` + acl "trusted-networks" { + "localhosts"; + } + ` + cfg, err := NewParser().Parse(" ", strings.NewReader(cfgText)) + require.NoError(t, err) + require.NotNil(t, cfg) + require.Len(t, cfg.Statements, 1) + require.NotNil(t, cfg.Statements[0].ACL) + require.Equal(t, "trusted-networks", cfg.Statements[0].ACL.Name) + require.NotNil(t, cfg.Statements[0].ACL.AddressMatchList) + require.Len(t, cfg.Statements[0].ACL.AddressMatchList.Elements, 1) + require.Equal(t, "localhosts", cfg.Statements[0].ACL.AddressMatchList.Elements[0].IPAddressOrACLName) +} + +// Test parsing ACL with a quoted IPv4 address. +func TestParseACLWithQuotedIPv4Address(t *testing.T) { + cfgText := ` + acl "trusted-networks" { + "10.0.0.1"; + } + ` + cfg, err := NewParser().Parse(" ", strings.NewReader(cfgText)) + require.NoError(t, err) + require.NotNil(t, cfg) + require.Len(t, cfg.Statements, 1) + require.NotNil(t, cfg.Statements[0].ACL) + require.Equal(t, "trusted-networks", cfg.Statements[0].ACL.Name) + require.NotNil(t, cfg.Statements[0].ACL.AddressMatchList) + require.Len(t, cfg.Statements[0].ACL.AddressMatchList.Elements, 1) + require.Equal(t, "10.0.0.1", cfg.Statements[0].ACL.AddressMatchList.Elements[0].IPAddressOrACLName) +} + +// A benchmark that measures the performance of the @stork:no-parse directive. +// It creates a set of zones and runs two independent checks. First, how long +// it takes to parse the zones. Second, how long it takes to process the config +// when @stork:no-parse elides the zones. +// +// For 10000 we've got the following results: +// +// BenchmarkNoParseZones/No_parse-12 2 774297854 ns/op 1344496 B/op 122 allocs/op +// BenchmarkNoParseZones/NoParseGlobal-12 2 717727792 ns/op 1306192 B/op 106 allocs/op +// BenchmarkNoParseZones/Parse-12 1 6126068000 ns/op 2984310664 B/op 10009899 allocs/op +// PASS +// ok isc.org/stork/appcfg/bind9 12.047s +// +// Clearly, skipping the zones during parsing significantly improves the +// configuration file parsing performance. +func BenchmarkNoParseZones(b *testing.B) { + zones := testutil.GenerateRandomZones(10000) + zoneTemplate := ` + zone "%s" { + type master; + allow-transfer port 853 { any; }; + file "/etc/bind/db.%s"; + }; + ` + builder := strings.Builder{} + for _, zone := range zones { + zoneText := fmt.Sprintf(zoneTemplate, zone.Name, zone.Name) + builder.WriteString(zoneText) + } + parser := NewParser() + require.NotNil(b, parser) + + b.Run("No parse", func(b *testing.B) { + // Surround the zones with the @stork:no-parse:begin/end directscope + noParseBuilder := strings.Builder{} + noParseBuilder.WriteString("//@stork:no-parse:scope\n") + noParseBuilder.WriteString(builder.String()) + noParseBuilder.WriteString("//@stork:no-parse:end\n") + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, err := parser.Parse("", strings.NewReader(noParseBuilder.String())) + require.NoError(b, err) + } + }) + + b.Run("NoParseGlobal", func(b *testing.B) { + // Precede the zones with the @stork:no-parse:global directive. + noParseBuilder := strings.Builder{} + noParseBuilder.WriteString("//@stork:no-parse:global\n") + noParseBuilder.WriteString(builder.String()) + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, err := parser.Parse("", strings.NewReader(noParseBuilder.String())) + require.NoError(b, err) + } + }) + + b.Run("Parse", func(b *testing.B) { + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, err := parser.Parse("", strings.NewReader(builder.String())) + require.NoError(b, err) + } + }) +} diff --git a/backend/appcfg/bind9/rawcontents.go b/backend/appcfg/bind9/rawcontents.go new file mode 100644 index 000000000..abe1705df --- /dev/null +++ b/backend/appcfg/bind9/rawcontents.go @@ -0,0 +1,9 @@ +package bind9config + +// Returns the string representation of the unparsed contents. +func (c *RawContents) GetString() string { + if c != nil { + return string(*c) + } + return "" +} diff --git a/backend/appcfg/bind9/rawcontents_test.go b/backend/appcfg/bind9/rawcontents_test.go new file mode 100644 index 000000000..4b8a6d6ed --- /dev/null +++ b/backend/appcfg/bind9/rawcontents_test.go @@ -0,0 +1,19 @@ +package bind9config + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// Test getting raw contents as a string. +func TestRawContentsGetString(t *testing.T) { + rawContents := RawContents("test") + require.Equal(t, "test", rawContents.GetString()) +} + +// Test getting raw contents as a string when it is nil. +func TestRawContentsNilGetString(t *testing.T) { + var rawContents *RawContents + require.Equal(t, "", rawContents.GetString()) +} diff --git a/backend/appcfg/bind9/statement.go b/backend/appcfg/bind9/statement.go new file mode 100644 index 000000000..6af127180 --- /dev/null +++ b/backend/appcfg/bind9/statement.go @@ -0,0 +1,17 @@ +package bind9config + +// Checks if the statement contains no-parse directives. +func (s *Statement) HasNoParse() bool { + switch { + case s.NoParse != nil: + return true + case s.Zone != nil: + return s.Zone.HasNoParse() + case s.View != nil: + return s.View.HasNoParse() + case s.Options != nil: + return s.Options.HasNoParse() + default: + return false + } +} diff --git a/backend/appcfg/bind9/statement_test.go b/backend/appcfg/bind9/statement_test.go new file mode 100644 index 000000000..edc3f1c54 --- /dev/null +++ b/backend/appcfg/bind9/statement_test.go @@ -0,0 +1,57 @@ +package bind9config + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// Test checking if the statement contains no-parse directives. +func TestStatementHasNoParseGlobal(t *testing.T) { + statement := &Statement{ + NoParse: &NoParse{}, + } + require.True(t, statement.HasNoParse()) +} + +// Test checking if the statement does not contain no-parse directives. +func TestStatementHasNoParseNone(t *testing.T) { + statement := &Statement{} + require.False(t, statement.HasNoParse()) +} + +// Test checking if the statement contains no-parse directives in the zone. +func TestStatementHasNoParseZone(t *testing.T) { + statement := &Statement{ + Zone: &Zone{ + Clauses: []*ZoneClause{ + {NoParse: &NoParse{}}, + }, + }, + } + require.True(t, statement.HasNoParse()) +} + +// Test checking if the statement contains no-parse directives in the view. +func TestStatementHasNoParseView(t *testing.T) { + statement := &Statement{ + View: &View{ + Clauses: []*ViewClause{ + {NoParse: &NoParse{}}, + }, + }, + } + require.True(t, statement.HasNoParse()) +} + +// Test checking if the statement contains no-parse directives in the options. +func TestStatementHasNoParseOptions(t *testing.T) { + statement := &Statement{ + Options: &Options{ + Clauses: []*OptionClause{ + {NoParse: &NoParse{}}, + }, + }, + } + require.True(t, statement.HasNoParse()) +} diff --git a/backend/appcfg/bind9/view.go b/backend/appcfg/bind9/view.go index 9835f9610..5b6a6f4a0 100644 --- a/backend/appcfg/bind9/view.go +++ b/backend/appcfg/bind9/view.go @@ -1,5 +1,18 @@ package bind9config +// Checks if the view contains no-parse directives. +func (v *View) HasNoParse() bool { + for _, clause := range v.Clauses { + switch { + case clause.NoParse != nil: + return true + case clause.Zone != nil: + return clause.Zone.HasNoParse() + } + } + return false +} + // Returns the allow-transfer clause for the view or nil if it is not found. func (v *View) GetAllowTransfer() *AllowTransfer { for _, clause := range v.Clauses { diff --git a/backend/appcfg/bind9/view_test.go b/backend/appcfg/bind9/view_test.go index 1bce523d1..a8736bad5 100644 --- a/backend/appcfg/bind9/view_test.go +++ b/backend/appcfg/bind9/view_test.go @@ -7,6 +7,36 @@ import ( storkutil "isc.org/stork/util" ) +// Test checking if the view contains no-parse directives. +func TestViewHasNoParse(t *testing.T) { + view := &View{ + Clauses: []*ViewClause{ + {NoParse: &NoParse{}}, + }, + } + require.True(t, view.HasNoParse()) +} + +// Test checking if the view contains no-parse directives in the zone. +func TestViewZoneHasNoParseZone(t *testing.T) { + view := &View{ + Clauses: []*ViewClause{ + {Zone: &Zone{ + Clauses: []*ZoneClause{ + {NoParse: &NoParse{}}, + }, + }}, + }, + } + require.True(t, view.HasNoParse()) +} + +// Test checking if the view does not contain no-parse directives. +func TestViewHasNoParseNone(t *testing.T) { + view := &View{} + require.False(t, view.HasNoParse()) +} + // Tests that allow-transfer is returned when specified. func TestViewGetAllowTransfer(t *testing.T) { view := &View{ diff --git a/backend/appcfg/bind9/zone.go b/backend/appcfg/bind9/zone.go index 24d40d639..44a57588d 100644 --- a/backend/appcfg/bind9/zone.go +++ b/backend/appcfg/bind9/zone.go @@ -1,5 +1,15 @@ package bind9config +// Checks if the zone contains no-parse directives. +func (z *Zone) HasNoParse() bool { + for _, clause := range z.Clauses { + if clause.NoParse != nil { + return true + } + } + return false +} + // Returns the allow-transfer clause for the zone or nil if it is not found. func (z *Zone) GetAllowTransfer() *AllowTransfer { for _, clause := range z.Clauses { diff --git a/backend/appcfg/bind9/zone_test.go b/backend/appcfg/bind9/zone_test.go index 1a87f7daf..780e676b8 100644 --- a/backend/appcfg/bind9/zone_test.go +++ b/backend/appcfg/bind9/zone_test.go @@ -7,6 +7,22 @@ import ( storkutil "isc.org/stork/util" ) +// Test checking if the zone contains no-parse directives. +func TestZoneHasNoParse(t *testing.T) { + zone := &Zone{ + Clauses: []*ZoneClause{ + {NoParse: &NoParse{}}, + }, + } + require.True(t, zone.HasNoParse()) +} + +// Test checking if the zone does not contain no-parse directives. +func TestZoneHasNoParseNone(t *testing.T) { + zone := &Zone{} + require.False(t, zone.HasNoParse()) +} + // Tests that allow-transfer is returned when specified. func TestZoneGetAllowTransfer(t *testing.T) { zone := &Zone{ diff --git a/changelog_unreleased/1912-performance-issues-with-bind9-large-config-parsing.md b/changelog_unreleased/1912-performance-issues-with-bind9-large-config-parsing.md new file mode 100644 index 000000000..ab2cb076e --- /dev/null +++ b/changelog_unreleased/1912-performance-issues-with-bind9-large-config-parsing.md @@ -0,0 +1,11 @@ +[func] marcin + + Improved performance of BIND 9 configuration parsing by the agent. + Added support for annotating parts of the BIND 9 configuration to + skip parsing them. These annotations are useful in large deployments + when parsing the BIND 9 configuration file can take significant + amount of time. Use //@stork:no-parse:scope and + //@stork:no-parse:end to skip parsing selected part of the + configuration file. Use //@stork:no-parse:global to skip parsing + the rest of the configuration file following the annotation. + (Gitlab #1912) diff --git a/docker/config/agent-bind9/named.conf b/docker/config/agent-bind9/named.conf index 376537144..4facdc64d 100644 --- a/docker/config/agent-bind9/named.conf +++ b/docker/config/agent-bind9/named.conf @@ -78,6 +78,7 @@ view "guest" { }; }; +//@stork:no-parse:global logging { channel transfers { file "/var/log/bind/transfers" versions 3 size 10M;