diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 1b43089e64f0..a2abd6494526 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -222,7 +222,7 @@ automatic splitting at root level, if root level element is an array. {pull}3415 - Fix double channel close panic when reloading. {pull}35324[35324] - Fix BPF filter setting not being applied to sniffers. {issue}35363[35363] {pull}35484[35484] -- Fix handling of Npcap installation options from Fleet. {pull}35541[35541] +- Fix handling of Npcap installation options from Fleet. {pull}35541[35541] {pull}35935[35935] *Winlogbeat* diff --git a/packetbeat/beater/install_npcap.go b/packetbeat/beater/install_npcap.go index 370f7712e977..3363baccf1da 100644 --- a/packetbeat/beater/install_npcap.go +++ b/packetbeat/beater/install_npcap.go @@ -23,16 +23,22 @@ import ( "os" "path/filepath" "runtime" + "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,7 +60,7 @@ func installNpcap(b *beat.Beat) error { return nil } - canInstall, err := canInstallNpcap(b) + canInstall, err := canInstallNpcap(b, cfg) if err != nil { return err } @@ -67,6 +73,8 @@ func installNpcap(b *beat.Beat) error { ctx, cancel := context.WithTimeout(context.Background(), installTimeout) defer cancel() + muInstall.Lock() + defer muInstall.Unlock() if npcap.Installer == nil { return nil } @@ -95,7 +103,7 @@ 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) (bool, error) { type npcapInstallCfg struct { NeverInstall bool `config:"npcap.never_install"` } @@ -105,10 +113,15 @@ 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. + return false, nil + } for _, c := range cfg.Streams { if c.NeverInstall { return false, nil @@ -119,7 +132,7 @@ 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) } diff --git a/packetbeat/beater/install_npcap_test.go b/packetbeat/beater/install_npcap_test.go index 7a888b4e0c89..0118a448b622 100644 --- a/packetbeat/beater/install_npcap_test.go +++ b/packetbeat/beater/install_npcap_test.go @@ -213,7 +213,7 @@ func TestCanInstallNpcap(t *testing.T) { BeatConfig: cfg, Manager: boolManager{managed: test.managed}, } - got, err := canInstallNpcap(b) + got, err := canInstallNpcap(b, b.BeatConfig) if err != nil { t.Errorf("unexpected error from canInstallNpcap: %v", err) } diff --git a/packetbeat/beater/packetbeat.go b/packetbeat/beater/packetbeat.go index fbc0e1c1fb90..b985b231f970 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 494b8f890b62..513dbe2871c7 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,