Skip to content

Commit

Permalink
x-pack/filebeat/input/http_endpoint: canonicalise tls consistency err…
Browse files Browse the repository at this point in the history
…or (#33720)

Configs may be run in arbitrary order during tests, so make sure that
the error is invariant with respect to the address and reason. This has
no impact on the logic of the input in production.
  • Loading branch information
efd6 authored Nov 23, 2022
1 parent 6fbbf24 commit 4f9aadc
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 9 deletions.
22 changes: 19 additions & 3 deletions x-pack/filebeat/input/http_endpoint/input.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,16 +164,32 @@ func (p *pool) serve(ctx v2.Context, e *httpEndpoint, pub stateless.Publisher) e
}

func checkTLSConsistency(addr string, old, new *tlscommon.ServerConfig) error {
if old == nil && new == nil {
return nil
}
if (old == nil) != (new == nil) {
return fmt.Errorf("inconsistent TLS usage on %s: mixed TLS and unencrypted", addr)
return invalidTLSStateErr{addr: addr, reason: "mixed TLS and unencrypted", old: old, new: new}
}
if !reflect.DeepEqual(old, new) {
return fmt.Errorf("inconsistent TLS configuration on %s: configuration options do not agree: old=%s new=%s",
addr, renderTLSConfig(old), renderTLSConfig(new))
return invalidTLSStateErr{addr: addr, reason: "configuration options do not agree", old: old, new: new}
}
return nil
}

type invalidTLSStateErr struct {
addr string
reason string
old, new *tlscommon.ServerConfig
}

func (e invalidTLSStateErr) Error() string {
if e.old == nil || e.new == nil {
return fmt.Sprintf("inconsistent TLS usage on %s: %s", e.addr, e.reason)
}
return fmt.Sprintf("inconsistent TLS configuration on %s: %s: old=%s new=%s",
e.addr, e.reason, renderTLSConfig(e.old), renderTLSConfig(e.new))
}

func renderTLSConfig(tls *tlscommon.ServerConfig) string {
c, err := conf.NewConfigFrom(tls)
if err != nil {
Expand Down
21 changes: 15 additions & 6 deletions x-pack/filebeat/input/http_endpoint/input_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ var serverPoolTests = []struct {
},
},
},
wantErr: errors.New("inconsistent TLS usage on 127.0.0.1:9001: mixed TLS and unencrypted"),
wantErr: invalidTLSStateErr{addr: "127.0.0.1:9001", reason: "mixed TLS and unencrypted"},
},
{
name: "inconsistent_tls_config",
Expand Down Expand Up @@ -198,10 +198,7 @@ var serverPoolTests = []struct {
},
},
},
wantErr: errors.New(`inconsistent TLS configuration on 127.0.0.1:9001: ` +
`configuration options do not agree: ` +
`old={"ca_sha256":[],"certificate":"","certificate_authorities":[],"cipher_suites":[],"client_authentication":0,"curve_types":[],"key":"","key_passphrase":"","supported_protocols":[],"verification_mode":1} ` +
`new={"ca_sha256":[],"certificate":"","certificate_authorities":[],"cipher_suites":[],"client_authentication":0,"curve_types":[],"key":"","key_passphrase":"","supported_protocols":[],"verification_mode":3}`),
wantErr: invalidTLSStateErr{addr: "127.0.0.1:9001", reason: "configuration options do not agree"},
},
}

Expand Down Expand Up @@ -241,7 +238,7 @@ func TestServerPool(t *testing.T) {
case err := <-fails:
if test.wantErr == nil {
t.Errorf("unexpected error calling serve: %#q", err)
} else if test.wantErr.Error() != err.Error() {
} else if !errors.Is(err, test.wantErr) {
t.Errorf("unexpected error calling serve: got=%#q, want=%#q", err, test.wantErr)
}
default:
Expand Down Expand Up @@ -272,6 +269,18 @@ func TestServerPool(t *testing.T) {
}
}

// Is is included to simplify testing, but is not exposed to avoid unwanted error
// matching outside tests.
func (e invalidTLSStateErr) Is(err error) bool {
if err, ok := err.(invalidTLSStateErr); ok { //nolint:errorlint // "An Is method should only shallowly compare err and the target and not call Unwrap on either."
// However for our purposes here, we will abuse
// the Is convention and also consider the addr
// and reason fields.
return e.addr == err.addr && e.reason == err.reason
}
return false
}

func newCtx(log, id string) (_ v2.Context, cancel func()) {
ctx, cancel := context.WithCancel(context.Background())
return v2.Context{
Expand Down

0 comments on commit 4f9aadc

Please sign in to comment.