packetbeat/beater: make Npcap installation lazy

CHANGELOG.next.asciidoc

@@ -185,6 +185,7 @@ automatic splitting at root level, if root level element is an array. {pull}3415
 
 *Packetbeat*
 
+- Fix handling of Npcap installation options from Fleet. {pull}35541[35541] {pull}35935[35935]
 
 *Winlogbeat*
 

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 -}}

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,22 +112,30 @@ 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.
 	if b.Manager.Enabled() {
 		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
 }

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)
 			}

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

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,

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]
 ------------------------------------------------------------------------------