Skip to content

Commit

Permalink
x-pack/filebeat/input/httpjson: Fix basic auth nil pointer deref (#37591
Browse files Browse the repository at this point in the history
)

For chained requests, setting user and password values for basic
authentication via a pointer to a requestFactory struct was done before
the struct was initialized, resulting in a nil pointer dereference and
runtime panic. Moving it to after the initialization resolved the issue.
  • Loading branch information
chrisberkhout authored Jan 12, 2024
1 parent e18d7f8 commit 0c387c5
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ https://github.com/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff]

*Filebeat*

- Fix nil pointer dereference in the httpjson input {pull}37591[37591]
- [Gcs Input] - Added missing locks for safe concurrency {pull}34914[34914]
- Fix the ignore_inactive option being ignored in Filebeat's filestream input {pull}34770[34770]
- Fix TestMultiEventForEOFRetryHandlerInput unit test of CometD input {pull}34903[34903]
Expand Down
17 changes: 8 additions & 9 deletions x-pack/filebeat/input/httpjson/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -316,13 +316,8 @@ func newRequestFactory(ctx context.Context, config config, log *logp.Logger, met
if err != nil {
return nil, fmt.Errorf("failed in creating chain http client with error: %w", err)
}
if ch.Step.Auth != nil && ch.Step.Auth.Basic.isEnabled() {
rf.user = ch.Step.Auth.Basic.User
rf.password = ch.Step.Auth.Basic.Password
}

responseProcessor := newChainResponseProcessor(ch, client, xmlDetails, metrics, log)

rf = &requestFactory{
url: *ch.Step.Request.URL.URL,
method: ch.Step.Request.Method,
Expand All @@ -336,6 +331,10 @@ func newRequestFactory(ctx context.Context, config config, log *logp.Logger, met
chainClient: client,
chainResponseProcessor: responseProcessor,
}
if ch.Step.Auth != nil && ch.Step.Auth.Basic.isEnabled() {
rf.user = ch.Step.Auth.Basic.User
rf.password = ch.Step.Auth.Basic.Password
}
} else if ch.While != nil {
ts, _ := newBasicTransformsFromConfig(registeredTransforms, ch.While.Request.Transforms, requestNamespace, log)
policy := newHTTPPolicy(evaluateResponse, ch.While.Until, log)
Expand All @@ -344,10 +343,6 @@ func newRequestFactory(ctx context.Context, config config, log *logp.Logger, met
if err != nil {
return nil, fmt.Errorf("failed in creating chain http client with error: %w", err)
}
if ch.While.Auth != nil && ch.While.Auth.Basic.isEnabled() {
rf.user = ch.While.Auth.Basic.User
rf.password = ch.While.Auth.Basic.Password
}

responseProcessor := newChainResponseProcessor(ch, client, xmlDetails, metrics, log)
rf = &requestFactory{
Expand All @@ -364,6 +359,10 @@ func newRequestFactory(ctx context.Context, config config, log *logp.Logger, met
chainClient: client,
chainResponseProcessor: responseProcessor,
}
if ch.While.Auth != nil && ch.While.Auth.Basic.isEnabled() {
rf.user = ch.While.Auth.Basic.User
rf.password = ch.While.Auth.Basic.Password
}
}
rfs = append(rfs, rf)
}
Expand Down
70 changes: 70 additions & 0 deletions x-pack/filebeat/input/httpjson/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,76 @@ func TestCtxAfterDoRequest(t *testing.T) {
)
}

func Test_newRequestFactory_UsesBasicAuthInChainedRequests(t *testing.T) {
ctx := context.Background()
log := logp.NewLogger("")
cfg := defaultChainConfig()

url, _ := url.Parse("https://example.com")
cfg.Request.URL = &urlConfig{
URL: url,
}

enabled := true
user := "basicuser"
password := "basicuser"
cfg.Auth = &authConfig{
Basic: &basicAuthConfig{
Enabled: &enabled,
User: user,
Password: password,
},
}

step := cfg.Chain[0].Step
step.Auth = cfg.Auth

while := cfg.Chain[0].While
while.Auth = cfg.Auth

type args struct {
cfg config
step *stepConfig
while *whileConfig
}
tests := []struct {
name string
args args
}{
{
name: "Step",
args: args{
cfg: cfg,
step: step,
while: nil,
},
},
{
name: "While",
args: args{
cfg: cfg,
step: nil,
while: while,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {

tt.args.cfg.Chain[0].Step = tt.args.step
tt.args.cfg.Chain[0].While = tt.args.while
requestFactories, err := newRequestFactory(ctx, tt.args.cfg, log, nil, nil)
assert.NoError(t, err)
assert.NotNil(t, requestFactories)
for _, rf := range requestFactories {
assert.Equal(t, rf.user, user)
assert.Equal(t, rf.password, password)
}

})
}
}

func Test_newChainHTTPClient(t *testing.T) {
cfg := defaultChainConfig()
cfg.Request.URL = &urlConfig{URL: &url.URL{}}
Expand Down

0 comments on commit 0c387c5

Please sign in to comment.