Skip to content

Conversation

@ycombinator
Copy link
Contributor

@ycombinator ycombinator commented Jun 19, 2025

Proposed commit message

This PR introduces a new type:

type FIPSAwareInput interface {
    IsFIPSCapable() bool
}

Filebeat inputs that wish to report if they are FIPS-capable (or not) are expected to implement this interface. Inputs that do not implement this interface are considered to be FIPS-capable.

For inputs that implement the above interface, FIPS-capable builds of Filebeat will check the input's FIPS-capability. If the input is not FIPS-capable, the input will fail to start with an error message and Filebeat will exit, e.g.:

{"log.level":"error","@timestamp":"2025-06-23T23:50:39.852-0700","log.origin":{"function":"github.com/elastic/beats/v7/libbeat/cmd/instance.handleError","file.name":"instance/beat.go","file.line":1355},"message":"Exiting: Failed to start crawler: starting input failed: input o365audit is not FIPS capable","service.name":"filebeat","ecs.version":"1.6.0"}
Exiting: Failed to start crawler: starting input failed: input o365audit is not FIPS capable

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Disruptive User Impact

None; the changes in this PR merely allow for Filebeat modules to be excluded from FIPS-capable Filebeat artifacts; there are no modules being excluded in this PR.

How to test this PR locally

$ mage clean && FIPS=true mage build
$ cat <<EOF > filebeat-test.yml
filebeat.inputs:
- type: o365audit
  id: id-nofips
  enabled: true
  application_id: foo
  tenant_id: bar
  client_secret: qux

output.console:
  enabled: true
EOF
$ ./filebeat -c ./filebeat-test.yml -e

@ycombinator ycombinator added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Jun 19, 2025
@ycombinator ycombinator requested a review from a team as a code owner June 19, 2025 05:24
@ycombinator ycombinator added the backport-8.19 Automated backport to the 8.19 branch label Jun 19, 2025
@ycombinator ycombinator requested review from belimawr and rdner June 19, 2025 05:24
@elasticmachine
Copy link
Contributor

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Jun 19, 2025
@github-actions
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@cmacknz
Copy link
Member

cmacknz commented Jun 19, 2025

I don't love that we have to duplicate the input list twice, I think this is going to result in people forgetting to add new modules and inputs to the FIPS file or miss adding them to one of the non-FIPS platform specific files long term. There is a new and indefinite mental burden to maintaining and testing these variations in the Beats.

I think we can deal with this in a simpler way with less overhead. When Filebeat is told to run an input type it doesn't recognize, it just exits with an error. So we should be able to get the same effect by having a FIPS variant of the o365 (and other inputs we want to exclude) exit on startup rather than duplicating the entire input list.

For example if I create the following configuration:

filebeat.inputs:
- type: none
  id: id-none
- type: filestream
  id: my-filestream-id
  enabled: false
  paths:
    - /var/log/*.log

filebeat.config.modules:
  path: ${path.config}/modules.d/*.yml
  reload.enabled: false

setup.template.settings:
  index.number_of_shards: 1

output.elasticsearch:
  hosts: ["localhost:9200"]
  username: "elastic"
  password: "changeme"

This is what Filebeat logs at startup and then exits with a non-zero exit code.

...
{"log.level":"info","@timestamp":"2025-06-19T15:36:10.227-0400","log.logger":"monitoring","log.origin":{"function":"github.com/elastic/beats/v7/libbeat/monitoring/report/log.(*reporter).logTotals","file.name":"log/log.go","file.line":201},"message":"Uptime: 5.16567975s","service.name":"filebeat","ecs.version":"1.6.0"}
{"log.level":"info","@timestamp":"2025-06-19T15:36:10.227-0400","log.logger":"monitoring","log.origin":{"function":"github.com/elastic/beats/v7/libbeat/monitoring/report/log.(*reporter).snapshotLoop","file.name":"log/log.go","file.line":168},"message":"Stopping metrics logging.","service.name":"filebeat","ecs.version":"1.6.0"}
{"log.level":"info","@timestamp":"2025-06-19T15:36:10.227-0400","log.origin":{"function":"github.com/elastic/beats/v7/libbeat/cmd/instance.(*Beat).launch","file.name":"instance/beat.go","file.line":542},"message":"filebeat stopped.","service.name":"filebeat","ecs.version":"1.6.0"}
{"log.level":"error","@timestamp":"2025-06-19T15:36:10.227-0400","log.origin":{"function":"github.com/elastic/beats/v7/libbeat/cmd/instance.handleError","file.name":"instance/beat.go","file.line":1355},"message":"Exiting: Failed to start crawler: starting input failed: error while initializing input: Error creating input. No such input type exist: 'none'","service.name":"filebeat","ecs.version":"1.6.0"}
Exiting: Failed to start crawler: starting input failed: error while initializing input: Error creating input. No such input type exist: 'none'

@cmacknz
Copy link
Member

cmacknz commented Jun 19, 2025

We could go even simpler and just have the inputs log they don't support FIPS and do this with documentation. There is no requirement to exclude non-FIPS functionality from an application, it just helps people see which things are non-compliant more easily.

@efd6
Copy link
Contributor

efd6 commented Jun 19, 2025

I'd like to see a simpler approach, and documentation would fit with that. The approach that has been taken has been pretty drastic; any reference to a package that is not FIPS compliant has been banished, even when the relevant part of the package is not called or the call is not in the context of cryptography or security. This approach in beats is at odds with the less draconian approaches used elsewhere.

@belimawr
Copy link
Contributor

I agree with Craig, this should be on the input side, rather than we having to maintain a 'list' FIPS/non-FIPS input used to build Filebeat.

Documentation + having the input refusing to start (maybe even causing Filebeat to exit) looks to be the simplest, more maintainable, option to me.

@ycombinator
Copy link
Contributor Author

Thanks for the feedback, @cmacknz @efd6 and @belimawr. I will rework this PR to allow FIPS-disabling from within the input so it's not duplicated and make sure its documented for each input as well.

@ycombinator ycombinator force-pushed the fips-disable-fb-modules branch from cb03ffa to af83f36 Compare June 24, 2025 06:25
@ycombinator ycombinator requested a review from a team as a code owner June 24, 2025 06:25
@ycombinator
Copy link
Contributor Author

ycombinator commented Jun 24, 2025

@cmacknz @belimawr @efd6, Based on your feedback, I've reworked this PR and updated it's description to match.

Please review this approach and let me know what you think. If you're happy with it, I'll add tests and remove the example o365audit input implementation from this PR and move it to it's own PR, including a doc update for that input.

Thanks!

Copy link
Contributor

@belimawr belimawr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I like this approach! I just added a couple comments to document exported methods.

@cmacknz
Copy link
Member

cmacknz commented Jun 24, 2025

This is much nicer, my concerns from #44920 (comment) have been addressed. Thank you!

LGTM but I'll defer data plane approval to @belimawr here since he has open comments you don't need my approval specifically.

@ycombinator ycombinator enabled auto-merge (squash) June 24, 2025 20:35
@ycombinator ycombinator merged commit 322d694 into elastic:main Jun 24, 2025
43 of 47 checks passed
mergify bot pushed a commit that referenced this pull request Jun 24, 2025
…4920)

* Add ability for Filebeat plugins (inputs) to specify that they should be excluded from FIPS builds

* Remove ExcludeForFIPS implementation

* Create and check FIPSAwareInput interface

* Make o365 input use FIPSAwareInput interface

* Running mage fmt

* Remove debugging statement

* Explain behavior for types not implementing the interface

* Adding godoc comments for implementations of the IsFIPSCapable() method

* Add unit test for checkFIPSCapability()

* Running mage fmt

* Remove example implementation

(cherry picked from commit 322d694)
@ycombinator ycombinator deleted the fips-disable-fb-modules branch June 24, 2025 22:35
ycombinator added a commit that referenced this pull request Jun 24, 2025
…4920) (#45014)

* Add ability for Filebeat plugins (inputs) to specify that they should be excluded from FIPS builds

* Remove ExcludeForFIPS implementation

* Create and check FIPSAwareInput interface

* Make o365 input use FIPSAwareInput interface

* Running mage fmt

* Remove debugging statement

* Explain behavior for types not implementing the interface

* Adding godoc comments for implementations of the IsFIPSCapable() method

* Add unit test for checkFIPSCapability()

* Running mage fmt

* Remove example implementation

(cherry picked from commit 322d694)

Co-authored-by: Shaunak Kashyap <[email protected]>
@ycombinator ycombinator restored the fips-disable-fb-modules branch June 25, 2025 18:36
@ycombinator ycombinator deleted the fips-disable-fb-modules branch June 25, 2025 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-8.19 Automated backport to the 8.19 branch Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants