From: Suzuki Shota <8736380+sshota0809@users.noreply.github.com> Date: Wed, 6 Jan 2021 13:52:00 +0000 (+0900) Subject: Solve #717 Validating config should try to compile regexes (#729) X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=ffd84c62dcce30136f92b6c38b2be9779c7ef695;p=blackbox_exporter.git Solve #717 Validating config should try to compile regexes (#729) * Solve issue: #717:Validating config includes trying to compile regexes Signed-off-by: sshota0809 <8736380+sshota0809@users.noreply.github.com> --- diff --git a/config/config.go b/config/config.go index 5309b0e..ee46010 100644 --- a/config/config.go +++ b/config/config.go @@ -17,6 +17,7 @@ import ( "errors" "fmt" "os" + "regexp" "runtime" "sync" "time" @@ -114,6 +115,53 @@ func (sc *SafeConfig) ReloadConfig(confFile string) (err error) { return nil } +// Regexp encapsulates a regexp.Regexp and makes it YAML marshalable. +type Regexp struct { + *regexp.Regexp + original string +} + +// NewRegexp creates a new anchored Regexp and returns an error if the +// passed-in regular expression does not compile. +func NewRegexp(s string) (Regexp, error) { + regex, err := regexp.Compile(s) + return Regexp{ + Regexp: regex, + original: s, + }, err +} + +// UnmarshalYAML implements the yaml.Unmarshaler interface. +func (re *Regexp) UnmarshalYAML(unmarshal func(interface{}) error) error { + var s string + if err := unmarshal(&s); err != nil { + return err + } + r, err := NewRegexp(s) + if err != nil { + return fmt.Errorf("\"Could not compile regular expression\" regexp=\"%s\"", s) + } + *re = r + return nil +} + +// MarshalYAML implements the yaml.Marshaler interface. +func (re Regexp) MarshalYAML() (interface{}, error) { + if re.original != "" { + return re.original, nil + } + return nil, nil +} + +// MustNewRegexp works like NewRegexp, but panics if the regular expression does not compile. +func MustNewRegexp(s string) Regexp { + re, err := NewRegexp(s) + if err != nil { + panic(err) + } + return re +} + type Module struct { Prober string `yaml:"prober,omitempty"` Timeout time.Duration `yaml:"timeout,omitempty"` @@ -134,8 +182,8 @@ type HTTPProbe struct { FailIfNotSSL bool `yaml:"fail_if_not_ssl,omitempty"` Method string `yaml:"method,omitempty"` Headers map[string]string `yaml:"headers,omitempty"` - FailIfBodyMatchesRegexp []string `yaml:"fail_if_body_matches_regexp,omitempty"` - FailIfBodyNotMatchesRegexp []string `yaml:"fail_if_body_not_matches_regexp,omitempty"` + FailIfBodyMatchesRegexp []Regexp `yaml:"fail_if_body_matches_regexp,omitempty"` + FailIfBodyNotMatchesRegexp []Regexp `yaml:"fail_if_body_not_matches_regexp,omitempty"` FailIfHeaderMatchesRegexp []HeaderMatch `yaml:"fail_if_header_matches,omitempty"` FailIfHeaderNotMatchesRegexp []HeaderMatch `yaml:"fail_if_header_not_matches,omitempty"` Body string `yaml:"body,omitempty"` @@ -144,12 +192,12 @@ type HTTPProbe struct { type HeaderMatch struct { Header string `yaml:"header,omitempty"` - Regexp string `yaml:"regexp,omitempty"` + Regexp Regexp `yaml:"regexp,omitempty"` AllowMissing bool `yaml:"allow_missing,omitempty"` } type QueryResponse struct { - Expect string `yaml:"expect,omitempty"` + Expect Regexp `yaml:"expect,omitempty"` Send string `yaml:"send,omitempty"` StartTLS bool `yaml:"starttls,omitempty"` } @@ -289,6 +337,7 @@ func (s *QueryResponse) UnmarshalYAML(unmarshal func(interface{}) error) error { if err := unmarshal((*plain)(s)); err != nil { return err } + return nil } @@ -303,7 +352,7 @@ func (s *HeaderMatch) UnmarshalYAML(unmarshal func(interface{}) error) error { return errors.New("header name must be set for HTTP header matchers") } - if s.Regexp == "" { + if s.Regexp.Regexp == nil || s.Regexp.Regexp.String() == "" { return errors.New("regexp must be set for HTTP header matchers") } diff --git a/config/config_test.go b/config/config_test.go index 21c8136..a47aa70 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -63,6 +63,22 @@ func TestLoadBadConfigs(t *testing.T) { ConfigFile: "testdata/invalid-http-header-match.yml", ExpectedError: "error parsing config file: regexp must be set for HTTP header matchers", }, + { + ConfigFile: "testdata/invalid-http-body-match-regexp.yml", + ExpectedError: "error parsing config file: \"Could not compile regular expression\" regexp=\":[\"", + }, + { + ConfigFile: "testdata/invalid-http-body-not-match-regexp.yml", + ExpectedError: "error parsing config file: \"Could not compile regular expression\" regexp=\":[\"", + }, + { + ConfigFile: "testdata/invalid-http-header-match-regexp.yml", + ExpectedError: "error parsing config file: \"Could not compile regular expression\" regexp=\":[\"", + }, + { + ConfigFile: "testdata/invalid-tcp-query-response-regexp.yml", + ExpectedError: "error parsing config file: \"Could not compile regular expression\" regexp=\":[\"", + }, } for i, test := range tests { err := sc.ReloadConfig(test.ConfigFile) diff --git a/config/testdata/invalid-http-body-match-regexp.yml b/config/testdata/invalid-http-body-match-regexp.yml new file mode 100644 index 0000000..f7afb00 --- /dev/null +++ b/config/testdata/invalid-http-body-match-regexp.yml @@ -0,0 +1,7 @@ +modules: + http_headers: + prober: http + timeout: 5s + http: + fail_if_body_matches_regexp: + - ':[' diff --git a/config/testdata/invalid-http-body-not-match-regexp.yml b/config/testdata/invalid-http-body-not-match-regexp.yml new file mode 100644 index 0000000..682158b --- /dev/null +++ b/config/testdata/invalid-http-body-not-match-regexp.yml @@ -0,0 +1,7 @@ +modules: + http_headers: + prober: http + timeout: 5s + http: + fail_if_body_not_matches_regexp: + - ':[' diff --git a/config/testdata/invalid-http-header-match-regexp.yml b/config/testdata/invalid-http-header-match-regexp.yml new file mode 100644 index 0000000..9d63967 --- /dev/null +++ b/config/testdata/invalid-http-header-match-regexp.yml @@ -0,0 +1,9 @@ +modules: + http_headers: + prober: http + timeout: 5s + http: + fail_if_header_not_matches: + - header: Access-Control-Allow-Origin + allow_missing: false + regexp: ':[' diff --git a/config/testdata/invalid-tcp-query-response-regexp.yml b/config/testdata/invalid-tcp-query-response-regexp.yml new file mode 100644 index 0000000..22a3165 --- /dev/null +++ b/config/testdata/invalid-tcp-query-response-regexp.yml @@ -0,0 +1,8 @@ +modules: + tcp_test: + prober: tcp + timeout: 5s + tcp: + query_response: + - expect: ":[" + - send: ". STARTTLS" diff --git a/prober/http.go b/prober/http.go index 5452fa1..02c2cb1 100644 --- a/prober/http.go +++ b/prober/http.go @@ -25,7 +25,6 @@ import ( "net/http/httptrace" "net/textproto" "net/url" - "regexp" "strconv" "strings" "sync" @@ -47,23 +46,13 @@ func matchRegularExpressions(reader io.Reader, httpConfig config.HTTPProbe, logg return false } for _, expression := range httpConfig.FailIfBodyMatchesRegexp { - re, err := regexp.Compile(expression) - if err != nil { - level.Error(logger).Log("msg", "Could not compile regular expression", "regexp", expression, "err", err) - return false - } - if re.Match(body) { + if expression.Regexp.Match(body) { level.Error(logger).Log("msg", "Body matched regular expression", "regexp", expression) return false } } for _, expression := range httpConfig.FailIfBodyNotMatchesRegexp { - re, err := regexp.Compile(expression) - if err != nil { - level.Error(logger).Log("msg", "Could not compile regular expression", "regexp", expression, "err", err) - return false - } - if !re.Match(body) { + if !expression.Regexp.Match(body) { level.Error(logger).Log("msg", "Body did not match regular expression", "regexp", expression) return false } @@ -83,14 +72,8 @@ func matchRegularExpressionsOnHeaders(header http.Header, httpConfig config.HTTP } } - re, err := regexp.Compile(headerMatchSpec.Regexp) - if err != nil { - level.Error(logger).Log("msg", "Could not compile regular expression", "regexp", headerMatchSpec.Regexp, "err", err) - return false - } - for _, val := range values { - if re.MatchString(val) { + if headerMatchSpec.Regexp.MatchString(val) { level.Error(logger).Log("msg", "Header matched regular expression", "header", headerMatchSpec.Header, "regexp", headerMatchSpec.Regexp, "value_count", len(values)) return false @@ -108,16 +91,10 @@ func matchRegularExpressionsOnHeaders(header http.Header, httpConfig config.HTTP } } - re, err := regexp.Compile(headerMatchSpec.Regexp) - if err != nil { - level.Error(logger).Log("msg", "Could not compile regular expression", "regexp", headerMatchSpec.Regexp, "err", err) - return false - } - anyHeaderValueMatched := false for _, val := range values { - if re.MatchString(val) { + if headerMatchSpec.Regexp.MatchString(val) { anyHeaderValueMatched = true break } diff --git a/prober/http_test.go b/prober/http_test.go index 3ebb536..0839099 100644 --- a/prober/http_test.go +++ b/prober/http_test.go @@ -334,30 +334,30 @@ func TestFailIfNotSSL(t *testing.T) { func TestFailIfBodyMatchesRegexp(t *testing.T) { testcases := map[string]struct { respBody string - regexps []string + regexps []config.Regexp expectedResult bool }{ "one regex, match": { respBody: "Bad news: could not connect to database server", - regexps: []string{"could not connect to database"}, + regexps: []config.Regexp{config.MustNewRegexp("could not connect to database")}, expectedResult: false, }, "one regex, no match": { respBody: "Download the latest version here", - regexps: []string{"could not connect to database"}, + regexps: []config.Regexp{config.MustNewRegexp("could not connect to database")}, expectedResult: true, }, "multiple regexes, match": { respBody: "internal error", - regexps: []string{"could not connect to database", "internal error"}, + regexps: []config.Regexp{config.MustNewRegexp("could not connect to database"), config.MustNewRegexp("internal error")}, expectedResult: false, }, "multiple regexes, no match": { respBody: "hello world", - regexps: []string{"could not connect to database", "internal error"}, + regexps: []config.Regexp{config.MustNewRegexp("could not connect to database"), config.MustNewRegexp("internal error")}, expectedResult: true, }, } @@ -410,7 +410,7 @@ func TestFailIfBodyNotMatchesRegexp(t *testing.T) { testCTX, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() result := ProbeHTTP(testCTX, ts.URL, - config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, FailIfBodyNotMatchesRegexp: []string{"Download the latest version here"}}}, registry, log.NewNopLogger()) + config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, FailIfBodyNotMatchesRegexp: []config.Regexp{config.MustNewRegexp("Download the latest version here")}}}, registry, log.NewNopLogger()) body := recorder.Body.String() if result { t.Fatalf("Regexp test succeeded unexpectedly, got %s", body) @@ -424,7 +424,7 @@ func TestFailIfBodyNotMatchesRegexp(t *testing.T) { recorder = httptest.NewRecorder() registry = prometheus.NewRegistry() result = ProbeHTTP(testCTX, ts.URL, - config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, FailIfBodyNotMatchesRegexp: []string{"Download the latest version here"}}}, registry, log.NewNopLogger()) + config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, FailIfBodyNotMatchesRegexp: []config.Regexp{config.MustNewRegexp("Download the latest version here")}}}, registry, log.NewNopLogger()) body = recorder.Body.String() if !result { t.Fatalf("Regexp test failed unexpectedly, got %s", body) @@ -440,7 +440,7 @@ func TestFailIfBodyNotMatchesRegexp(t *testing.T) { recorder = httptest.NewRecorder() registry = prometheus.NewRegistry() result = ProbeHTTP(testCTX, ts.URL, - config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, FailIfBodyNotMatchesRegexp: []string{"Download the latest version here", "Copyright 2015"}}}, registry, log.NewNopLogger()) + config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, FailIfBodyNotMatchesRegexp: []config.Regexp{config.MustNewRegexp("Download the latest version here"), config.MustNewRegexp("Copyright 2015")}}}, registry, log.NewNopLogger()) body = recorder.Body.String() if result { t.Fatalf("Regexp test succeeded unexpectedly, got %s", body) @@ -454,7 +454,7 @@ func TestFailIfBodyNotMatchesRegexp(t *testing.T) { recorder = httptest.NewRecorder() registry = prometheus.NewRegistry() result = ProbeHTTP(testCTX, ts.URL, - config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, FailIfBodyNotMatchesRegexp: []string{"Download the latest version here", "Copyright 2015"}}}, registry, log.NewNopLogger()) + config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, FailIfBodyNotMatchesRegexp: []config.Regexp{config.MustNewRegexp("Download the latest version here"), config.MustNewRegexp("Copyright 2015")}}}, registry, log.NewNopLogger()) body = recorder.Body.String() if !result { t.Fatalf("Regexp test failed unexpectedly, got %s", body) @@ -467,15 +467,15 @@ func TestFailIfHeaderMatchesRegexp(t *testing.T) { Values []string ShouldSucceed bool }{ - {config.HeaderMatch{"Content-Type", "text/javascript", false}, []string{"text/javascript"}, false}, - {config.HeaderMatch{"Content-Type", "text/javascript", false}, []string{"application/octet-stream"}, true}, - {config.HeaderMatch{"content-type", "text/javascript", false}, []string{"application/octet-stream"}, true}, - {config.HeaderMatch{"Content-Type", ".*", false}, []string{""}, false}, - {config.HeaderMatch{"Content-Type", ".*", false}, []string{}, false}, - {config.HeaderMatch{"Content-Type", ".*", true}, []string{""}, false}, - {config.HeaderMatch{"Content-Type", ".*", true}, []string{}, true}, - {config.HeaderMatch{"Set-Cookie", ".*Domain=\\.example\\.com.*", false}, []string{"gid=1; Expires=Tue, 19-Mar-2019 20:08:29 GMT; Domain=.example.com; Path=/"}, false}, - {config.HeaderMatch{"Set-Cookie", ".*Domain=\\.example\\.com.*", false}, []string{"zz=4; expires=Mon, 01-Jan-1990 00:00:00 GMT; Domain=www.example.com; Path=/", "gid=1; Expires=Tue, 19-Mar-2019 20:08:29 GMT; Domain=.example.com; Path=/"}, false}, + {config.HeaderMatch{"Content-Type", config.MustNewRegexp("text/javascript"), false}, []string{"text/javascript"}, false}, + {config.HeaderMatch{"Content-Type", config.MustNewRegexp("text/javascript"), false}, []string{"application/octet-stream"}, true}, + {config.HeaderMatch{"content-type", config.MustNewRegexp("text/javascript"), false}, []string{"application/octet-stream"}, true}, + {config.HeaderMatch{"Content-Type", config.MustNewRegexp(".*"), false}, []string{""}, false}, + {config.HeaderMatch{"Content-Type", config.MustNewRegexp(".*"), false}, []string{}, false}, + {config.HeaderMatch{"Content-Type", config.MustNewRegexp(".*"), true}, []string{""}, false}, + {config.HeaderMatch{"Content-Type", config.MustNewRegexp(".*"), true}, []string{}, true}, + {config.HeaderMatch{"Set-Cookie", config.MustNewRegexp(".*Domain=\\.example\\.com.*"), false}, []string{"gid=1; Expires=Tue, 19-Mar-2019 20:08:29 GMT; Domain=.example.com; Path=/"}, false}, + {config.HeaderMatch{"Set-Cookie", config.MustNewRegexp(".*Domain=\\.example\\.com.*"), false}, []string{"zz=4; expires=Mon, 01-Jan-1990 00:00:00 GMT; Domain=www.example.com; Path=/", "gid=1; Expires=Tue, 19-Mar-2019 20:08:29 GMT; Domain=.example.com; Path=/"}, false}, } for i, test := range tests { @@ -516,14 +516,14 @@ func TestFailIfHeaderNotMatchesRegexp(t *testing.T) { Values []string ShouldSucceed bool }{ - {config.HeaderMatch{"Content-Type", "text/javascript", false}, []string{"text/javascript"}, true}, - {config.HeaderMatch{"content-type", "text/javascript", false}, []string{"text/javascript"}, true}, - {config.HeaderMatch{"Content-Type", "text/javascript", false}, []string{"application/octet-stream"}, false}, - {config.HeaderMatch{"Content-Type", ".*", false}, []string{""}, true}, - {config.HeaderMatch{"Content-Type", ".*", false}, []string{}, false}, - {config.HeaderMatch{"Content-Type", ".*", true}, []string{}, true}, - {config.HeaderMatch{"Set-Cookie", ".*Domain=\\.example\\.com.*", false}, []string{"zz=4; expires=Mon, 01-Jan-1990 00:00:00 GMT; Domain=www.example.com; Path=/"}, false}, - {config.HeaderMatch{"Set-Cookie", ".*Domain=\\.example\\.com.*", false}, []string{"zz=4; expires=Mon, 01-Jan-1990 00:00:00 GMT; Domain=www.example.com; Path=/", "gid=1; Expires=Tue, 19-Mar-2019 20:08:29 GMT; Domain=.example.com; Path=/"}, true}, + {config.HeaderMatch{"Content-Type", config.MustNewRegexp("text/javascript"), false}, []string{"text/javascript"}, true}, + {config.HeaderMatch{"content-type", config.MustNewRegexp("text/javascript"), false}, []string{"text/javascript"}, true}, + {config.HeaderMatch{"Content-Type", config.MustNewRegexp("text/javascript"), false}, []string{"application/octet-stream"}, false}, + {config.HeaderMatch{"Content-Type", config.MustNewRegexp(".*"), false}, []string{""}, true}, + {config.HeaderMatch{"Content-Type", config.MustNewRegexp(".*"), false}, []string{}, false}, + {config.HeaderMatch{"Content-Type", config.MustNewRegexp(".*"), true}, []string{}, true}, + {config.HeaderMatch{"Set-Cookie", config.MustNewRegexp(".*Domain=\\.example\\.com.*"), false}, []string{"zz=4; expires=Mon, 01-Jan-1990 00:00:00 GMT; Domain=www.example.com; Path=/"}, false}, + {config.HeaderMatch{"Set-Cookie", config.MustNewRegexp(".*Domain=\\.example\\.com.*"), false}, []string{"zz=4; expires=Mon, 01-Jan-1990 00:00:00 GMT; Domain=www.example.com; Path=/", "gid=1; Expires=Tue, 19-Mar-2019 20:08:29 GMT; Domain=.example.com; Path=/"}, true}, } for i, test := range tests { diff --git a/prober/tcp.go b/prober/tcp.go index 3e8339b..dcbfe0f 100644 --- a/prober/tcp.go +++ b/prober/tcp.go @@ -19,7 +19,6 @@ import ( "crypto/tls" "fmt" "net" - "regexp" "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" @@ -146,19 +145,14 @@ func ProbeTCP(ctx context.Context, target string, module config.Module, registry for i, qr := range module.TCP.QueryResponse { level.Info(logger).Log("msg", "Processing query response entry", "entry_number", i) send := qr.Send - if qr.Expect != "" { - re, err := regexp.Compile(qr.Expect) - if err != nil { - level.Error(logger).Log("msg", "Could not compile into regular expression", "regexp", qr.Expect, "err", err) - return false - } + if qr.Expect.Regexp != nil { var match []int // Read lines until one of them matches the configured regexp. for scanner.Scan() { level.Debug(logger).Log("msg", "Read line", "line", scanner.Text()) - match = re.FindSubmatchIndex(scanner.Bytes()) + match = qr.Expect.Regexp.FindSubmatchIndex(scanner.Bytes()) if match != nil { - level.Info(logger).Log("msg", "Regexp matched", "regexp", re, "line", scanner.Text()) + level.Info(logger).Log("msg", "Regexp matched", "regexp", qr.Expect.Regexp, "line", scanner.Text()) break } } @@ -168,11 +162,11 @@ func ProbeTCP(ctx context.Context, target string, module config.Module, registry } if match == nil { probeFailedDueToRegex.Set(1) - level.Error(logger).Log("msg", "Regexp did not match", "regexp", re, "line", scanner.Text()) + level.Error(logger).Log("msg", "Regexp did not match", "regexp", qr.Expect.Regexp, "line", scanner.Text()) return false } probeFailedDueToRegex.Set(0) - send = string(re.Expand(nil, []byte(send), scanner.Bytes(), match)) + send = string(qr.Expect.Regexp.Expand(nil, []byte(send), scanner.Bytes(), match)) } if send != "" { level.Debug(logger).Log("msg", "Sending line", "line", send) diff --git a/prober/tcp_test.go b/prober/tcp_test.go index 753fc89..0402a8d 100644 --- a/prober/tcp_test.go +++ b/prober/tcp_test.go @@ -360,14 +360,14 @@ func TestTCPConnectionQueryResponseStartTLS(t *testing.T) { TCP: config.TCPProbe{ IPProtocolFallback: true, QueryResponse: []config.QueryResponse{ - {Expect: "^220.*ESMTP.*$"}, + {Expect: config.MustNewRegexp("^220.*ESMTP.*$")}, {Send: "EHLO tls.prober"}, - {Expect: "^250-STARTTLS"}, + {Expect: config.MustNewRegexp("^250-STARTTLS")}, {Send: "STARTTLS"}, - {Expect: "^220"}, + {Expect: config.MustNewRegexp("^220")}, {StartTLS: true}, {Send: "EHLO tls.prober"}, - {Expect: "^250-AUTH"}, + {Expect: config.MustNewRegexp("^250-AUTH")}, {Send: "QUIT"}, }, TLSConfig: pconfig.TLSConfig{ @@ -458,7 +458,7 @@ func TestTCPConnectionQueryResponseIRC(t *testing.T) { QueryResponse: []config.QueryResponse{ {Send: "NICK prober"}, {Send: "USER prober prober prober :prober"}, - {Expect: "^:[^ ]+ 001"}, + {Expect: config.MustNewRegexp("^:[^ ]+ 001")}, }, }, } @@ -526,7 +526,7 @@ func TestTCPConnectionQueryResponseMatching(t *testing.T) { IPProtocolFallback: true, QueryResponse: []config.QueryResponse{ { - Expect: "SSH-2.0-(OpenSSH_6.9p1) Debian-2", + Expect: config.MustNewRegexp("SSH-2.0-(OpenSSH_6.9p1) Debian-2"), Send: "CONFIRM ${1}", }, }, @@ -676,7 +676,7 @@ func TestPrometheusTimeoutTCP(t *testing.T) { IPProtocolFallback: true, QueryResponse: []config.QueryResponse{ { - Expect: "SSH-2.0-(OpenSSH_6.9p1) Debian-2", + Expect: config.MustNewRegexp("SSH-2.0-(OpenSSH_6.9p1) Debian-2"), }, }, }}, registry, log.NewNopLogger()) {