diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index d001bc58b3d..9476cca3d61 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -136,6 +136,7 @@ https://github.com/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff] *Packetbeat* +- Fix handling of Npcap installation options from Fleet. {pull}35541[35541] {pull}35935[35935] *Winlogbeat* diff --git a/packetbeat/_meta/config/windows_npcap.yml.tmpl b/packetbeat/_meta/config/windows_npcap.yml.tmpl index 62605c20250..23647cc6d01 100644 --- a/packetbeat/_meta/config/windows_npcap.yml.tmpl +++ b/packetbeat/_meta/config/windows_npcap.yml.tmpl @@ -8,6 +8,6 @@ #packetbeat.npcap: # # If a specific local version of Npcap is required, installation by packetbeat # # can be blocked by setting never_install to true. No action is taken if this -# # option is set to true. +# # option is set to true unless no Npcap is already installed. # never_install: false {{- end -}} diff --git a/packetbeat/beater/install_npcap.go b/packetbeat/beater/install_npcap.go index 370f7712e97..c1413fdb6d9 100644 --- a/packetbeat/beater/install_npcap.go +++ b/packetbeat/beater/install_npcap.go @@ -23,16 +23,23 @@ import ( "os" "path/filepath" "runtime" + "strings" + "sync" "time" "github.com/elastic/beats/v7/libbeat/beat" "github.com/elastic/beats/v7/packetbeat/npcap" + conf "github.com/elastic/elastic-agent-libs/config" "github.com/elastic/elastic-agent-libs/logp" ) const installTimeout = 120 * time.Second -func installNpcap(b *beat.Beat) error { +// muInstall protects use of npcap.Installer. The only writes to npcap.Installer +// are here and during init in x-pack/packetbeat/npcap/npcap_windows.go +var muInstall sync.Mutex + +func installNpcap(b *beat.Beat, cfg *conf.C) error { if !b.Info.ElasticLicensed { return nil } @@ -54,19 +61,29 @@ func installNpcap(b *beat.Beat) error { return nil } - canInstall, err := canInstallNpcap(b) - if err != nil { - return err - } log := logp.NewLogger("npcap_install") - if !canInstall { - log.Warn("npcap installation/upgrade disabled by user") - return nil + // Only check whether we have been requested to never_install if there + // is already an Npcap installation present. This should not be necessary, + // but the start-up logic of packetbeat is tightly coupled to the presence + // of a backing sniffer. This should really not be necessary, but the changes + // to modify this behaviour are non-trivial, so just avoid the issue. + isInstalled := strings.HasPrefix(npcap.Version(), "Npcap version") + if isInstalled { + canInstall, err := canInstallNpcap(b, cfg, log) + if err != nil { + return err + } + if !canInstall { + log.Warn("npcap installation/upgrade disabled by user") + return nil + } } ctx, cancel := context.WithTimeout(context.Background(), installTimeout) defer cancel() + muInstall.Lock() + defer muInstall.Unlock() if npcap.Installer == nil { return nil } @@ -95,9 +112,10 @@ func installNpcap(b *beat.Beat) error { // configurations from agent normalised to the internal packetbeat format by this point. // In the case that the beat is managed, any data stream that has npcap.never_install // set to true will result in a block on the installation. -func canInstallNpcap(b *beat.Beat) (bool, error) { +func canInstallNpcap(b *beat.Beat, rawcfg *conf.C, log *logp.Logger) (bool, error) { type npcapInstallCfg struct { - NeverInstall bool `config:"npcap.never_install"` + Type string `config:"type"` + NeverInstall bool `config:"npcap.never_install"` } // Agent managed case. @@ -105,12 +123,19 @@ func canInstallNpcap(b *beat.Beat) (bool, error) { var cfg struct { Streams []npcapInstallCfg `config:"streams"` } - err := b.BeatConfig.Unpack(&cfg) + err := rawcfg.Unpack(&cfg) if err != nil { return false, fmt.Errorf("failed to unpack npcap config from agent configuration: %w", err) } + if len(cfg.Streams) == 0 { + // We have no stream to monitor, so we don't need to install + // anything. We may be in the middle of a config check. + log.Debug("cannot install because no configured stream") + return false, nil + } for _, c := range cfg.Streams { if c.NeverInstall { + log.Debugf("cannot install because %s has never_install set to true", c.Type) return false, nil } } @@ -119,9 +144,12 @@ func canInstallNpcap(b *beat.Beat) (bool, error) { // Packetbeat case. var cfg npcapInstallCfg - err := b.BeatConfig.Unpack(&cfg) + err := rawcfg.Unpack(&cfg) if err != nil { return false, fmt.Errorf("failed to unpack npcap config from packetbeat configuration: %w", err) } + if cfg.NeverInstall { + log.Debugf("cannot install because %s has never_install set to true", cfg.Type) + } return !cfg.NeverInstall, err } diff --git a/packetbeat/beater/install_npcap_test.go b/packetbeat/beater/install_npcap_test.go index 7a888b4e0c8..3d8c678edaf 100644 --- a/packetbeat/beater/install_npcap_test.go +++ b/packetbeat/beater/install_npcap_test.go @@ -23,6 +23,7 @@ import ( "github.com/elastic/beats/v7/libbeat/beat" "github.com/elastic/beats/v7/libbeat/management" "github.com/elastic/elastic-agent-libs/config" + "github.com/elastic/elastic-agent-libs/logp" ) var canInstallNpcapTests = []struct { @@ -210,10 +211,9 @@ func TestCanInstallNpcap(t *testing.T) { t.Fatalf("unexpected error: %v", err) } b := &beat.Beat{ - BeatConfig: cfg, - Manager: boolManager{managed: test.managed}, + Manager: boolManager{managed: test.managed}, } - got, err := canInstallNpcap(b) + got, err := canInstallNpcap(b, cfg, logp.NewLogger("npcap_install_test")) if err != nil { t.Errorf("unexpected error from canInstallNpcap: %v", err) } diff --git a/packetbeat/beater/packetbeat.go b/packetbeat/beater/packetbeat.go index fbc0e1c1fb9..b985b231f97 100644 --- a/packetbeat/beater/packetbeat.go +++ b/packetbeat/beater/packetbeat.go @@ -93,14 +93,6 @@ func New(b *beat.Beat, rawConfig *conf.C) (beat.Beater, error) { configurator = initialConfig().FromStatic } - // Install Npcap if needed. This need to happen before any other - // work on Windows, including config checking, because that involves - // probing interfaces. - err := installNpcap(b) - if err != nil { - return nil, err - } - factory := newProcessorFactory(b.Info.Name, make(chan error, maxSniffers), b, configurator) if err := factory.CheckConfig(rawConfig); err != nil { return nil, err diff --git a/packetbeat/beater/processor.go b/packetbeat/beater/processor.go index 494b8f890b6..513dbe2871c 100644 --- a/packetbeat/beater/processor.go +++ b/packetbeat/beater/processor.go @@ -123,6 +123,24 @@ func (p *processorFactory) Create(pipeline beat.PipelineConnector, cfg *conf.C) logp.Err("Failed to generate ID from config: %v, %v", err, config) return nil, err } + if len(config.Interfaces) != 0 { + // Install Npcap if needed. This needs to happen before any other + // work on Windows, including config checking, because that involves + // probing interfaces. + // + // Users may block installation of Npcap, so we defer the install + // until we have a configuration that will tell us if it has been + // blocked. To do this we must have a valid config. + // + // When Packetbeat is managed by fleet we will only have this if + // Create has been called via the agent Reload process. We take + // the opportunity to not install the DLL if there is no configured + // interface. + err := installNpcap(p.beat, cfg) + if err != nil { + return nil, err + } + } publisher, err := publish.NewTransactionPublisher( p.beat.Info.Name, diff --git a/packetbeat/docs/packetbeat-options.asciidoc b/packetbeat/docs/packetbeat-options.asciidoc index 5266dac8d33..4a74dd5593e 100644 --- a/packetbeat/docs/packetbeat-options.asciidoc +++ b/packetbeat/docs/packetbeat-options.asciidoc @@ -61,7 +61,7 @@ On Windows {beatname} requires an Npcap DLL installation. This is provided by {b for users of the Elastic Licenced version. In some cases users may wish to use their own installed version. In order to do this the `packetbeat.npcap.never_install` option can be used. Setting this option to `true` will not attempt to install the -bundled Npcap library on start-up. +bundled Npcap library on start-up unless no Npcap is already installed. [source,yaml] ------------------------------------------------------------------------------