auth: use ACLsDisabledACL when ACLs are disabled (#18754)

The RPC handlers expect to see `nil` ACL objects whenever ACLs are disabled. By
using `nil` as a sentinel value, we have the risk of nil pointer exceptions and
improper handling of `nil` when returned from our various auth methods that can
lead to privilege escalation bugs. This is the final patch in a series to
eliminate the use of `nil` ACLs as a sentinel value for when ACLs are disabled.

This patch adds a new virtual ACL policy field for when ACLs are disabled and
updates our authentication logic to use it. Included:

* Extends auth package tests to demonstrate that nil ACLs are treated as failed
  auth and disabled ACLs succeed auth.
* Adds a new `AllowDebug` ACL check for the weird special casing we have for
  pprof debugging when ACLs are disabled.
* Removes the remaining unexported methods (and repeated tests) from the
  `nomad/acl.go` file.
* Update the semgrep rules to detect improper nil ACL checking and remove the
  old invalid ACL checks.
* Update the contributing guide for RPC authentication.

Ref: hashicorp/nomad-enterprise#1218
Ref: #18703
Ref: #18715
Ref: #16799
Ref: #18730
Ref: #18744

.semgrep/rpc_endpoint.yml

@@ -2,9 +2,7 @@
 # SPDX-License-Identifier: BUSL-1.1
 
 rules:
-  # Check potentially unauthenticated RPC endpoints. Technically more
-  # authorization (authz) oriented than authn, but before Nomad 1.4/1.5 that
-  # distinction wasn't as important.
+  # Check potentially RPC endpoints with missing authentication/authorization.
   - id: "rpc-potentially-unauthenticated"
     patterns:
       - pattern: |
@@ -40,7 +38,6 @@ rules:
           }
           ...
 
-
       # Pattern used by endpoints that are used only for client-to-server.
       # Authorization can be done after forwarding, but must check the
       # AllowClientOp policy
@@ -56,7 +53,6 @@ rules:
           }
           ...
 
-
       # Pattern used by endpoints that are used only for client-to-server.
       # Authorization can be done after forwarding, but must check the
       # AllowClientOp policy. This should not be added to any new endpoints.
@@ -72,7 +68,8 @@ rules:
           }
           ...
 
-      # Pattern used by ACL endpoints that need to interact with the token directly
+      # Pattern used by ACL endpoints that need to interact with the token
+      # directly.
       - pattern-not-inside: |
           authErr := $A.$B.Authenticate($A.ctx, args)
           ...
@@ -82,52 +79,7 @@ rules:
           ...
           ... := args.GetIdentity().GetACLToken()
           ...
-      # Pattern used by endpoints called exclusively between agents
-      # (server -> server or client -> server)
-      - pattern-not-inside: |
-          authErr := $A.$B.Authenticate($A.ctx, args)
-          ...
-          ... := validateTLSCertificateLevel(...)
-          ...
-          if done, err := $A.$B.forward($METHOD, ...); done {
-            return err
-          }
-      # Pattern used by endpoints that support both normal ACLs and workload
-      # identity but break authentication and authorization up
-      # TODO: currently this is just for Variables and should be removed once
-      # https://github.com/hashicorp/nomad/issues/15875 is complete.
-      - pattern-not-inside: |
-          authErr := $A.$B.Authenticate($A.ctx, args)
-          ...
-          if done, err := $A.$B.forward($METHOD, ...); done {
-            return err
-          }
-          ...
-          ... := $T.handleMixedAuthEndpoint(...)
-          ...
-      # Second pattern used by endpoints that support both normal ACLs and
-      # workload identity but break authentication and authorization up
-      # TODO: currently this is just for Variables and should be removed once
-      # https://github.com/hashicorp/nomad/issues/15875 is complete.
-      - pattern-not-inside: |
-          authErr := $A.$B.Authenticate($A.ctx, args)
-          ...
-          if done, err := $A.$B.forward($METHOD, ...); done {
-            return err
-          }
-          ...
-          ... := svePreApply($A, args, args.Var)
-          ...
-      # Pattern used by some Node endpoints.
-      - pattern-not-inside: |
-          authErr := $A.$B.Authenticate($A.ctx, args)
-          ...
-          if done, err := $A.$B.forward($METHOD, ...); done {
-            return err
-          }
-          ...
-          return $A.deregister(...)
-          ...
+
       - metavariable-pattern:
           metavariable: $METHOD
           patterns:
@@ -142,8 +94,6 @@ rules:
             - pattern-not: 'structs.ACLOIDCAuthURLRPCMethod'
             - pattern-not: 'structs.ACLOIDCCompleteAuthRPCMethod'
             - pattern-not: 'structs.ACLLoginRPCMethod'
-            - pattern-not: '"CSIPlugin.Get"'
-            - pattern-not: '"CSIPlugin.List"'
             - pattern-not: '"Status.Leader"'
             - pattern-not: '"Status.Peers"'
             - pattern-not: '"Status.Version"'
@@ -155,3 +105,24 @@ rules:
     paths:
       include:
         - "nomad/*_endpoint.go"
+
+
+  # ACL objects should never be nil-checked in RPC handlers before checking
+  # authorization, as nil ACLs are always programmer errors.
+  - id: "rpc-authz-bypass"
+    patterns:
+      # Pattern that will accidentally bypass authorization checks.
+      - pattern: |
+          ...
+          if aclObj != nil && aclObj.$ACL_CHECK(...) {
+          ...
+          }
+          ...
+
+    message: "RPC method ACL check $ACL_CHECK appears to bypass authorization by first checking for nil ACLs"
+    languages:
+      - "go"
+    severity: "WARNING"
+    paths:
+      include:
+        - "nomad/*_endpoint.go"