endpoint: Fix data races while accessing GetIdentity()

[ upstream commit 10f4e2c ]

[ Backporter's notes: Minor conflicts scattered across five files. ]

Calls to GetIdentity() have been assuming both that the endpoint is
locked or not locked.

* daemon/cmd/fqdn.go / notifyOnDNSMsg():

  LookupEndpointIDByIP(). No locking.

  --> Vulnerable

* pkg/datapath/linux/config/config.go / writeStaticData()

  e.owner.Datapath().WriteEndpointConfig -> WriteEndpointConfig() -> writeStaticData()

  The endpoint is locked. Not using epCacheInfo. Must use a non-locking
  variation or a deadlock may occur.

  --> Not vulnerable

* pkg/datapath/loader/template.go

  e.realizeBPFState() ->
  -> CompileOrLoad -> ELFSubstitutions() -> elfVariableSubstitutions()
  -> CompileAndLoad() -> compileAndLoad() -> realizeBPFState() -> ReloadDatapath() ->
  -> ReloadDatapath() -> reloadHostDatapath() -> patchHostNetdevDatapath() -> ELFSubstitutions() -> elfVariableSubstitutions()

  Uses epInfoCache

  --> Not vulnerable

* pkg/envoy/server.go

  e.updateNetworkPolicy() -> UpdateNetworkPolicy()

  The endpoint is locked. Must use a non-locking variation.

  --> Not vulnerable

* pkg/hubble/parser/threefour/parser.go

  Decode() -> resolveEndpoint()

  --> Vulnerable

Fixes: #11932

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Joe Stringer <joe@cilium.io>

pkg/datapath/config.go

@@ -49,6 +49,11 @@ type EndpointConfiguration interface {
 	// GetIdentity returns a globally-significant numeric security identity.
 	GetIdentity() identity.NumericIdentity
 
+	// GetIdentityLocked returns a globally-significant numeric security
+	// identity while assuming that the backing data structure is locked.
+	// This function should be removed in favour of GetIdentity()
+	GetIdentityLocked() identity.NumericIdentity
+
 	IPv4Address() addressing.CiliumIPv4
 	IPv6Address() addressing.CiliumIPv6
 	GetNodeMAC() mac.MAC

pkg/datapath/linux/config/config.go

@@ -349,7 +349,7 @@ func (h *HeaderfileWriter) writeStaticData(fw io.Writer, e datapath.EndpointConf
 	fmt.Fprint(fw, defineMAC("NODE_MAC", e.GetNodeMAC()))
 	fmt.Fprint(fw, defineUint32("LXC_ID", uint32(e.GetID())))
 
-	secID := e.GetIdentity().Uint32()
+	secID := e.GetIdentityLocked().Uint32()
 	fmt.Fprintf(fw, defineUint32("SECLABEL", secID))
 	fmt.Fprintf(fw, defineUint32("SECLABEL_NB", byteorder.HostToNetwork(secID).(uint32)))
 
@@ -412,7 +412,7 @@ func (h *HeaderfileWriter) writeTemplateConfig(fw *bufio.Writer, e datapath.Endp
 	return fw.Flush()
 }
 
-// WriteEndpointConfig writes the BPF configuration for the template to a writer.
+// WriteTemplateConfig writes the BPF configuration for the template to a writer.
 func (h *HeaderfileWriter) WriteTemplateConfig(w io.Writer, e datapath.EndpointConfiguration) error {
 	fw := bufio.NewWriter(w)
 	return h.writeTemplateConfig(fw, e)

pkg/datapath/loader/template.go

@@ -92,6 +92,13 @@ func (t *templateCfg) GetIdentity() identity.NumericIdentity {
 	return TemplateSecurityID
 }
 
+// GetIdentityLocked is identical to GetIdentity(). This is a temporary
+// function until WriteEndpointConfig() no longer assumes that the endpoint is
+// locked.
+func (t *templateCfg) GetIdentityLocked() identity.NumericIdentity {
+	return TemplateSecurityID
+}
+
 // GetNodeMAC returns a well-known dummy MAC address which may be later
 // substituted in the ELF.
 func (t *templateCfg) GetNodeMAC() mac.MAC {

pkg/endpoint/bpf.go

@@ -71,6 +71,8 @@ func (e *Endpoint) BPFIpvlanMapPath() string {
 // strings describing the configuration of the datapath.
 //
 // For configuration of actual datapath behavior, see WriteEndpointConfig().
+//
+// e.Mutex must be held
 func (e *Endpoint) writeInformationalComments(w io.Writer) error {
 	fw := bufio.NewWriter(w)
 
@@ -104,7 +106,7 @@ func (e *Endpoint) writeInformationalComments(w io.Writer) error {
 		" * NodeMAC: %s\n"+
 		" */\n\n",
 		e.IPv6.String(), e.IPv4.String(),
-		e.GetIdentity(), bpf.LocalMapName(policymap.MapName, e.ID),
+		e.getIdentity(), bpf.LocalMapName(policymap.MapName, e.ID),
 		e.nodeMAC)
 
 	fw.WriteString("/*\n")
@@ -123,6 +125,9 @@ func (e *Endpoint) writeInformationalComments(w io.Writer) error {
 	return fw.Flush()
 }
 
+// writeHeaderfile writes the lxc_config.h header file of an endpoint
+//
+// e.Mutex must be held.
 func (e *Endpoint) writeHeaderfile(prefix string) error {
 	headerPath := filepath.Join(prefix, common.CHeaderFileName)
 	e.getLogger().WithFields(logrus.Fields{

pkg/endpoint/cache.go

@@ -75,7 +75,7 @@ func (e *Endpoint) createEpInfoCache(epdir string) *epInfoCache {
 		id:                    e.GetID(),
 		ifName:                e.ifName,
 		ipvlan:                e.HasIpvlanDataPath(),
-		identity:              e.GetIdentity(),
+		identity:              e.getIdentity(),
 		mac:                   e.GetNodeMAC(),
 		ipv4:                  e.IPv4Address(),
 		ipv6:                  e.IPv6Address(),
@@ -129,6 +129,11 @@ func (ep *epInfoCache) GetIdentity() identity.NumericIdentity {
 	return ep.identity
 }
 
+// GetIdentityLocked returns the security identity of the endpoint.
+func (ep *epInfoCache) GetIdentityLocked() identity.NumericIdentity {
+	return ep.identity
+}
+
 // Logger returns the logger for the endpoint that is being cached.
 func (ep *epInfoCache) Logger(subsystem string) *logrus.Entry {
 	return ep.endpoint.Logger(subsystem)

pkg/endpoint/endpoint.go

@@ -567,7 +567,21 @@ func (e *Endpoint) StringID() string {
 	return strconv.Itoa(int(e.ID))
 }
 
+// GetIdentityLocked is identical to GetIdentity() but assumes that a.mutex is
+// already held. This function is obsolete and should no longer be used.
+func (e *Endpoint) GetIdentityLocked() identity.NumericIdentity {
+	return e.getIdentity()
+}
+
+// GetIdentity returns the numeric security identity of the endpoint
 func (e *Endpoint) GetIdentity() identity.NumericIdentity {
+	e.unconditionalRLock()
+	defer e.runlock()
+
+	return e.getIdentity()
+}
+
+func (e *Endpoint) getIdentity() identity.NumericIdentity {
 	if e.SecurityIdentity != nil {
 		return e.SecurityIdentity.ID
 	}
@@ -2151,7 +2165,7 @@ func (e *Endpoint) GetProxyInfoByFields() (uint64, string, string, []string, str
 	if e.IsDisconnecting() {
 		err = fmt.Errorf("endpoint is in the process of being deleted")
 	}
-	return e.GetID(), e.GetIPv4Address(), e.GetIPv6Address(), e.GetLabels(), e.GetLabelsSHA(), uint64(e.GetIdentity()), err
+	return e.GetID(), e.GetIPv4Address(), e.GetIPv6Address(), e.GetLabels(), e.GetLabelsSHA(), uint64(e.getIdentity()), err
 }
 
 // WaitForFirstRegeneration waits for specific conditions before returning:

pkg/envoy/server.go

@@ -1033,7 +1033,7 @@ func (s *XDSServer) UpdateNetworkPolicy(ep logger.EndpointUpdater, policy *polic
 		if ip == "" {
 			continue
 		}
-		networkPolicy := getNetworkPolicy(ip, ep.GetIdentity(), ep.ConntrackNameLocked(), policy,
+		networkPolicy := getNetworkPolicy(ip, ep.GetIdentityLocked(), ep.ConntrackNameLocked(), policy,
 			ingressPolicyEnforced, egressPolicyEnforced)
 		err := networkPolicy.Validate()
 		if err != nil {

pkg/proxy/logger/epinfo.go

@@ -28,7 +28,7 @@ type EndpointInfoSource interface {
 	GetID() uint64
 	GetIPv4Address() string
 	GetIPv6Address() string
-	GetIdentity() identity.NumericIdentity
+	GetIdentityLocked() identity.NumericIdentity
 	GetLabels() []string
 	GetLabelsSHA() string
 	HasSidecarProxy() bool

pkg/proxy/mock_test.go

@@ -35,20 +35,20 @@ type proxyUpdaterMock struct {
 }
 
 func (m *proxyUpdaterMock) GetProxyInfoByFields() (uint64, string, string, []string, string, uint64, error) {
-	return m.GetID(), m.GetIPv4Address(), m.GetIPv6Address(), m.GetLabels(), m.GetLabelsSHA(), uint64(m.GetIdentity()), nil
+	return m.GetID(), m.GetIPv4Address(), m.GetIPv6Address(), m.GetLabels(), m.GetLabelsSHA(), uint64(m.GetIdentityLocked()), nil
 }
 
 func (m *proxyUpdaterMock) UnconditionalRLock() { m.RWMutex.RLock() }
 func (m *proxyUpdaterMock) RUnlock()            { m.RWMutex.RUnlock() }
 
-func (m *proxyUpdaterMock) GetID() uint64                         { return m.id }
-func (m *proxyUpdaterMock) GetIPv4Address() string                { return m.ipv4 }
-func (m *proxyUpdaterMock) GetIPv6Address() string                { return m.ipv6 }
-func (m *proxyUpdaterMock) GetLabels() []string                   { return m.labels }
-func (m *proxyUpdaterMock) GetEgressPolicyEnabledLocked() bool    { return true }
-func (m *proxyUpdaterMock) GetIngressPolicyEnabledLocked() bool   { return true }
-func (m *proxyUpdaterMock) GetIdentity() identity.NumericIdentity { return m.identity }
-func (m *proxyUpdaterMock) ProxyID(l4 *policy.L4Filter) string    { return "" }
+func (m *proxyUpdaterMock) GetID() uint64                               { return m.id }
+func (m *proxyUpdaterMock) GetIPv4Address() string                      { return m.ipv4 }
+func (m *proxyUpdaterMock) GetIPv6Address() string                      { return m.ipv6 }
+func (m *proxyUpdaterMock) GetLabels() []string                         { return m.labels }
+func (m *proxyUpdaterMock) GetEgressPolicyEnabledLocked() bool          { return true }
+func (m *proxyUpdaterMock) GetIngressPolicyEnabledLocked() bool         { return true }
+func (m *proxyUpdaterMock) GetIdentityLocked() identity.NumericIdentity { return m.identity }
+func (m *proxyUpdaterMock) ProxyID(l4 *policy.L4Filter) string          { return "" }
 func (m *proxyUpdaterMock) GetLabelsSHA() string {
 	return labels.NewLabelsFromModel(m.labels).SHA256Sum()
 }

pkg/testutils/endpoint.go

@@ -49,19 +49,20 @@ func NewTestEndpoint() TestEndpoint {
 	}
 }
 
-func (e *TestEndpoint) HasIpvlanDataPath() bool                 { return false }
-func (e *TestEndpoint) ConntrackLocalLocked() bool              { return false }
-func (e *TestEndpoint) RequireARPPassthrough() bool             { return false }
-func (e *TestEndpoint) RequireEgressProg() bool                 { return false }
-func (e *TestEndpoint) RequireRouting() bool                    { return false }
-func (e *TestEndpoint) RequireEndpointRoute() bool              { return false }
-func (e *TestEndpoint) GetCIDRPrefixLengths() ([]int, []int)    { return nil, nil }
-func (e *TestEndpoint) GetID() uint64                           { return e.Id }
-func (e *TestEndpoint) StringID() string                        { return "42" }
-func (e *TestEndpoint) GetIdentity() identity.NumericIdentity   { return e.Identity.ID }
-func (e *TestEndpoint) GetSecurityIdentity() *identity.Identity { return e.Identity }
-func (e *TestEndpoint) GetNodeMAC() mac.MAC                     { return e.MAC }
-func (e *TestEndpoint) GetOptions() *option.IntOptions          { return e.Opts }
+func (e *TestEndpoint) HasIpvlanDataPath() bool                     { return false }
+func (e *TestEndpoint) ConntrackLocalLocked() bool                  { return false }
+func (e *TestEndpoint) RequireARPPassthrough() bool                 { return false }
+func (e *TestEndpoint) RequireEgressProg() bool                     { return false }
+func (e *TestEndpoint) RequireRouting() bool                        { return false }
+func (e *TestEndpoint) RequireEndpointRoute() bool                  { return false }
+func (e *TestEndpoint) GetCIDRPrefixLengths() ([]int, []int)        { return nil, nil }
+func (e *TestEndpoint) GetID() uint64                               { return e.Id }
+func (e *TestEndpoint) StringID() string                            { return "42" }
+func (e *TestEndpoint) GetIdentity() identity.NumericIdentity       { return e.Identity.ID }
+func (e *TestEndpoint) GetIdentityLocked() identity.NumericIdentity { return e.Identity.ID }
+func (e *TestEndpoint) GetSecurityIdentity() *identity.Identity     { return e.Identity }
+func (e *TestEndpoint) GetNodeMAC() mac.MAC                         { return e.MAC }
+func (e *TestEndpoint) GetOptions() *option.IntOptions              { return e.Opts }
 
 func (e *TestEndpoint) IPv4Address() addressing.CiliumIPv4 {
 	addr, _ := addressing.NewCiliumIPv4("192.0.2.3")