-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Elastic-Agent] Modify output to be insecure if flag is provided #28007
Changes from 4 commits
99e8ef2
13fe45d
cbff1f6
14e76ed
da05f40
1da9c62
bfe5d9a
cbc327b
3237773
b224286
1881cab
595fd40
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
// or more contributor license agreements. Licensed under the Elastic License; | ||
// you may not use this file except in compliance with the Elastic License. | ||
|
||
package modifiers | ||
|
||
import ( | ||
"github.com/elastic/beats/v7/libbeat/common/transport/tlscommon" | ||
"github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/configuration" | ||
"github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/transpiler" | ||
"github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/core/logger" | ||
) | ||
|
||
const ( | ||
sslKey = "ssl" | ||
verificationModeKey = "verification_mode" | ||
sslVerificationKey = "ssl.verification_mode" | ||
) | ||
|
||
// InjectInsecureOutput injects a verification none into output configuration. | ||
func InjectInsecureOutput(fleetConfig *configuration.FleetAgentConfig) func(*logger.Logger, *transpiler.AST) error { | ||
return func(_ *logger.Logger, rootAst *transpiler.AST) error { | ||
// if verification mode is not set abort | ||
if fleetConfig == nil || | ||
fleetConfig.Server == nil || | ||
fleetConfig.Server.TLS == nil || | ||
fleetConfig.Server.TLS.VerificationMode == tlscommon.VerifyFull { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This does seem to have the affect that it will only do something if the Elastic Agent is running Fleet Server, but it will affect all beats. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you're right. changed to use client config instead |
||
// no change | ||
return nil | ||
} | ||
|
||
// look for outputs | ||
// inject verification mode to each output | ||
outputsNode, ok := transpiler.Lookup(rootAst, outputsKey) | ||
if !ok { | ||
// no outputs from configuration; skip | ||
return nil | ||
} | ||
|
||
outputsList, ok := outputsNode.Value().(*transpiler.Dict) | ||
if !ok { | ||
return nil | ||
} | ||
|
||
outputsNodeCollection, ok := outputsList.Value().([]transpiler.Node) | ||
if !ok { | ||
return nil | ||
} | ||
|
||
modeString := fleetConfig.Server.TLS.VerificationMode.String() | ||
|
||
for _, outputNode := range outputsNodeCollection { | ||
outputKV, ok := outputNode.(*transpiler.Key) | ||
if !ok { | ||
continue | ||
} | ||
|
||
output, ok := outputKV.Value().(*transpiler.Dict) | ||
if !ok { | ||
continue | ||
} | ||
|
||
// do not overwrite already specified config | ||
_, found := output.Find(sslVerificationKey) | ||
if found { | ||
continue | ||
} | ||
|
||
// it may be broken down | ||
if sslNode, found := output.Find(sslKey); found { | ||
if _, found := sslNode.Find(verificationModeKey); found { | ||
continue | ||
} | ||
} | ||
|
||
output.Insert( | ||
transpiler.NewKey(sslVerificationKey, transpiler.NewStrVal(modeString)), | ||
) | ||
|
||
} | ||
|
||
return nil | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
// or more contributor license agreements. Licensed under the Elastic License; | ||
// you may not use this file except in compliance with the Elastic License. | ||
|
||
package modifiers | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/google/go-cmp/cmp" | ||
"github.com/stretchr/testify/assert" | ||
"github.com/stretchr/testify/require" | ||
|
||
"github.com/elastic/beats/v7/libbeat/common/transport/tlscommon" | ||
"github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/configuration" | ||
"github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/transpiler" | ||
) | ||
|
||
func TestInjectInsecure(t *testing.T) { | ||
cases := []struct { | ||
Name string | ||
Config map[string]interface{} | ||
Expected map[string]interface{} | ||
VerificationMode tlscommon.TLSVerificationMode | ||
}{ | ||
{ | ||
Name: "no change on default", | ||
Config: map[string]interface{}{ | ||
"outputs": map[string]interface{}{ | ||
"elasticsearch": map[string]interface{}{ | ||
"key": "value", | ||
}, | ||
}, | ||
}, | ||
Expected: map[string]interface{}{ | ||
"outputs": map[string]interface{}{ | ||
"elasticsearch": map[string]interface{}{ | ||
"key": "value", | ||
}, | ||
}, | ||
}, | ||
VerificationMode: tlscommon.VerifyFull, // default | ||
}, | ||
{ | ||
Name: "inject none", | ||
Config: map[string]interface{}{ | ||
"outputs": map[string]interface{}{ | ||
"elasticsearch": map[string]interface{}{ | ||
"key": "value", | ||
}, | ||
}, | ||
}, | ||
Expected: map[string]interface{}{ | ||
"outputs": map[string]interface{}{ | ||
"elasticsearch": map[string]interface{}{ | ||
"key": "value", | ||
"ssl.verification_mode": "none", | ||
}, | ||
}, | ||
}, | ||
VerificationMode: tlscommon.VerifyNone, | ||
}, | ||
} | ||
|
||
for _, tc := range cases { | ||
t.Run(tc.Name, func(t *testing.T) { | ||
fn := InjectInsecureOutput(&configuration.FleetAgentConfig{ | ||
Server: &configuration.FleetServerConfig{ | ||
TLS: &tlscommon.Config{ | ||
VerificationMode: tc.VerificationMode, | ||
}, | ||
}, | ||
}) | ||
|
||
ast, err := transpiler.NewAST(tc.Config) | ||
require.NoError(t, err) | ||
expectedAST, err := transpiler.NewAST(tc.Expected) | ||
require.NoError(t, err) | ||
|
||
require.NoError(t, fn(nil, ast)) | ||
|
||
visitor := &transpiler.MapVisitor{} | ||
expectedVisitor := &transpiler.MapVisitor{} | ||
|
||
ast.Accept(visitor) | ||
expectedAST.Accept(expectedVisitor) | ||
|
||
if !assert.True(t, cmp.Equal(expectedVisitor.Content, visitor.Content)) { | ||
diff := cmp.Diff(expectedVisitor.Content, visitor.Content) | ||
if diff != "" { | ||
t.Errorf("mismatch (-want +got):\n%s", diff) | ||
} | ||
} | ||
}) | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like it will affect more than just Fleet Server running under Elastic Agent, it will also affect all the other beats, correct?
If this does affect the other beats, I don't think we want this, because how will this work when it comes to multiple outputs? I believe it will have the effect that if
--insecure
is used all outputs will then become insecure.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes that's the purpose. if we wont pass, events wont get consumed
this is to ease up on initial experience. you wont use insecure in prod. at least i hope nobody will
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They would get consumed if the policy is configured correctly. I don't think it should be Elastic Agent's job to override the output settings from Kibana.
If we had proper health reporting for outputs, it would be clear there is an issue. The real issue is that the Elastic Agent reports healthy when it is not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if value is provided by kibana it should not be overriden, i'll add testcase to make it more visible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kibana on the other hand would needs to have the info that hey this cert we're using is self signed and may no be properly configured to support some config automatically (providing values to out config option).
insecure is our concept and i think it's up to us to help user using this with FRE.