Take Apple signing credentials from Secrets Manager

.github/workflows/release.yml

@@ -73,8 +73,11 @@ jobs:
   build-darwin:
     name: Build Universal Darwin
     env:
-      HAS_SIGNING_CREDS: ${{ secrets.AC_USERNAME != '' }}
+      HAS_SIGNING_CREDS: ${{ secrets.APPLE_SIGNING_ROLE_ARN != '' }}
     runs-on: macos-latest
+    permissions:
+      id-token: write
+      contents: read
     steps:
       - uses: actions/checkout@v7
 
@@ -83,17 +86,68 @@ jobs:
           go-version: '1.26'
           check-latest: true
 
+      # GitHub holds ARNs, not credentials, and ARNs outlive a rotation
+      - name: Configure AWS credentials
+        if: env.HAS_SIGNING_CREDS == 'true'
+        uses: aws-actions/configure-aws-credentials@v6
+        with:
+          role-to-assume: ${{ secrets.APPLE_SIGNING_ROLE_ARN }}
+          aws-region: us-east-2
+
+      # parse-json-secrets unpacks into SIGNING_* and ASC_*, masked on the way in
+      - name: Fetch signing credentials
+        if: env.HAS_SIGNING_CREDS == 'true'
+        uses: aws-actions/aws-secretsmanager-get-secrets@v3
+        with:
+          parse-json-secrets: true
+          secret-ids: |
+            SIGNING,${{ secrets.APPLE_SIGNING_DEVELOPER_ID_ARN }}
+            ASC,${{ secrets.APPLE_NOTARY_KEY_ARN }}
+
       - name: Import certificates
         if: env.HAS_SIGNING_CREDS == 'true'
         uses: Apple-Actions/import-codesign-certs@v7
         with:
-          p12-file-base64: ${{ secrets.APPLE_DEVELOPER_CERTIFICATE_P12_BASE64 }}
-          p12-password: ${{ secrets.APPLE_DEVELOPER_CERTIFICATE_PASSWORD }}
+          p12-file-base64: ${{ env.SIGNING_P12_BASE64 }}
+          p12-password: ${{ env.SIGNING_PASSWORD }}
+
+      # The action imports but does not check the chain validates, which is how a p12
+      # missing its intermediate reaches a failing codesign
+      - name: Check the identity is usable
+        if: env.HAS_SIGNING_CREDS == 'true'
+        run: |
+          : "${SIGNING_IDENTITY_SHA1:?empty, so the secret has no identity_sha1}"
+          identities=$(security find-identity -v -p codesigning signing_temp.keychain)
+          case "$identities" in
+          *"$SIGNING_IDENTITY_SHA1"*) ;;
+          *) printf '%s\n' "$identities" >&2; exit 1 ;;
+          esac
+
+      # notarytool wants the key as a file
+      - name: Write the App Store Connect key
+        if: env.HAS_SIGNING_CREDS == 'true'
+        run: |
+          mkdir -p ~/private_keys
+          chmod 700 ~/private_keys
+          key_path="$HOME/private_keys/AuthKey_${ASC_KEY_ID}.p8"
+          (umask 077; printf '%s\n' "$ASC_PRIVATE_KEY" > "$key_path")
+          echo "ASC_P8=$key_path" >> "$GITHUB_ENV"
+
+      - name: Drop the credentials from the environment
+        if: env.HAS_SIGNING_CREDS == 'true'
+        run: |
+          # The action's own inventory, so a new field in a secret is covered
+          python3 -c '
+          import json, os
+          raw = os.environ.get("SECRETS_LIST_CLEAN_UP")
+          if raw is None and os.environ.get("SIGNING_P12_BASE64"):
+              raise SystemExit("SECRETS_LIST_CLEAN_UP is gone, fetched secrets are not being scrubbed")
+          keep = {"SIGNING_IDENTITY_SHA1", "ASC_KEY_ID", "ASC_ISSUER_ID"}
+          names = [n for n in json.loads(raw or "[]") if n not in keep]
+          print("\n".join(f"{n}=" for n in dict.fromkeys(names)))
+          ' >> "$GITHUB_ENV"
 
       - name: Build, sign, and notarize
-        env:
-          AC_USERNAME: ${{ secrets.AC_USERNAME }}
-          AC_PASSWORD: ${{ secrets.AC_PASSWORD }}
         run: |
           rm -rf release
           mkdir release
@@ -102,17 +156,34 @@ jobs:
           lipo -create -output ./release/nebula ./build/darwin-amd64/nebula ./build/darwin-arm64/nebula
           lipo -create -output ./release/nebula-cert ./build/darwin-amd64/nebula-cert ./build/darwin-arm64/nebula-cert
 
-          if [ -n "$AC_USERNAME" ]; then
-            codesign -s "10BC1FDDEB6CE753550156C0669109FAC49E4D1E" -f -v --timestamp --options=runtime -i "net.defined.nebula" ./release/nebula
-            codesign -s "10BC1FDDEB6CE753550156C0669109FAC49E4D1E" -f -v --timestamp --options=runtime -i "net.defined.nebula-cert" ./release/nebula-cert
+          # Unset in a fork, which has no credentials to sign with
+          if [ -n "$SIGNING_IDENTITY_SHA1" ]; then
+            codesign -s "$SIGNING_IDENTITY_SHA1" -f -v --timestamp --options=runtime -i "net.defined.nebula" ./release/nebula
+            codesign -s "$SIGNING_IDENTITY_SHA1" -f -v --timestamp --options=runtime -i "net.defined.nebula-cert" ./release/nebula-cert
           fi
 
           zip -j release/nebula-darwin.zip release/nebula-cert release/nebula
 
-          if [ -n "$AC_USERNAME" ]; then
-            xcrun notarytool submit ./release/nebula-darwin.zip --team-id "576H3XS7FP" --apple-id "$AC_USERNAME" --password "$AC_PASSWORD" --wait
+          if [ -n "$ASC_P8" ]; then
+            xcrun notarytool submit ./release/nebula-darwin.zip --key "$ASC_P8" --key-id "$ASC_KEY_ID" --issuer "$ASC_ISSUER_ID" --wait
           fi
 
+      - name: Drop the signing key
+        if: always() && env.HAS_SIGNING_CREDS == 'true'
+        run: |
+          # Locked, not deleted: import-codesign-certs deletes it in its own post
+          # step and fails the job if it is already gone. Locked is unusable.
+          security lock-keychain signing_temp.keychain || true
+          rm -f "$ASC_P8"
+          # Nothing later in this job needs AWS
+          python3 -c '
+          import json, os
+          names = json.loads(os.environ.get("SECRETS_LIST_CLEAN_UP") or "[]")
+          names += ["ASC_P8", "SIGNING_IDENTITY_SHA1", "ASC_KEY_ID", "ASC_ISSUER_ID",
+                    "AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY", "AWS_SESSION_TOKEN"]
+          print("\n".join(f"{n}=" for n in dict.fromkeys(names)))
+          ' >> "$GITHUB_ENV"
+
       - name: Upload artifacts
         uses: actions/upload-artifact@v7
         with:

main.go

@@ -22,12 +22,9 @@ type m = map[string]any
 
 func Main(c *config.C, configTest bool, buildVersion string, l *slog.Logger, deviceFactory overlay.DeviceFactory) (retcon *Control, reterr error) {
 	ctx, cancel := context.WithCancel(context.Background())
-	// The goroutines started below stop only when this context does, and only a caller holding the
-	// Control can arrange that. Cancel whenever we are not handing one back, which covers an error
-	// and a config test alike: a config test used to leave the lighthouse query worker, and a
-	// hostname resolver per dns named static host, running for the life of the process.
+	// Automatically cancel the context if Main returns an error, to signal all created goroutines to quit.
 	defer func() {
-		if retcon == nil {
+		if reterr != nil {
 			cancel()
 		}
 	}()

main_test.go

@@ -1,82 +0,0 @@
-package nebula
-
-import (
-	"fmt"
-	"net/netip"
-	"os"
-	"path/filepath"
-	"testing"
-	"time"
-
-	"github.com/slackhq/nebula/cert"
-	cert_test "github.com/slackhq/nebula/cert_test"
-	"github.com/slackhq/nebula/config"
-	"github.com/slackhq/nebula/test"
-	"github.com/stretchr/testify/require"
-	"go.uber.org/goleak"
-)
-
-// TestMain_ConfigTestReleasesItsGoroutines pins the rule that Main only leaves goroutines running
-// when it hands back a Control to stop them with.
-//
-// A config test gets no Control, so anything it started had nothing to stop it: the lighthouse
-// query worker, and a hostname resolver per dns named static host, ran for the life of the
-// process. That matters to every embedder that validates a config in process rather than by
-// exec'ing, dnclient and the apple clients included, because they do it on each config load and
-// the leak accumulates.
-func TestMain_ConfigTestReleasesItsGoroutines(t *testing.T) {
-	defer goleak.VerifyNone(t, goleak.IgnoreCurrent())
-
-	l := test.NewLogger()
-	dir := t.TempDir()
-
-	before := time.Now().Add(-time.Hour)
-	after := time.Now().Add(time.Hour)
-	ca, _, caKey, caPEM := cert_test.NewTestCaCert(cert.Version2, cert.Curve_CURVE25519, before, after, nil, nil, nil)
-	networks := []netip.Prefix{netip.MustParsePrefix("10.0.0.1/24")}
-	_, _, keyPEM, certPEM := cert_test.NewTestCert(
-		cert.Version2, cert.Curve_CURVE25519, ca, caKey, "config-test", before, after, networks, nil, nil)
-
-	caPath := filepath.Join(dir, "ca.pem")
-	certPath := filepath.Join(dir, "cert.pem")
-	keyPath := filepath.Join(dir, "key.pem")
-	require.NoError(t, os.WriteFile(caPath, caPEM, 0o600))
-	require.NoError(t, os.WriteFile(certPath, certPEM, 0o600))
-	require.NoError(t, os.WriteFile(keyPath, keyPEM, 0o600))
-
-	// A static host by address, not by name: the query worker is the goroutine under test and a
-	// hostname would drag a real dns lookup into a unit test.
-	configBody := fmt.Sprintf(`
-pki:
-  ca: %s
-  cert: %s
-  key: %s
-static_host_map:
-  "10.0.0.2": ["192.0.2.1:4242"]
-lighthouse:
-  hosts:
-    - "10.0.0.2"
-listen:
-  host: 127.0.0.1
-  port: 0
-tun:
-  disabled: true
-firewall:
-  outbound:
-    - port: any
-      proto: any
-      host: any
-  inbound:
-    - port: any
-      proto: any
-      host: any
-`, caPath, certPath, keyPath)
-	require.NoError(t, os.WriteFile(filepath.Join(dir, "config.yml"), []byte(configBody), 0o600))
-
-	c := config.NewC(l)
-	require.NoError(t, c.Load(dir))
-
-	ctrl, err := Main(c, true, "config-test", l, nil)
-	require.NoError(t, err)
-	require.Nil(t, ctrl, "a config test hands back nothing to stop, so it must stop itself")
-}

overlay/tun_darwin.go

@@ -30,10 +30,7 @@ type tun struct {
 	Routes      atomic.Pointer[[]Route]
 	routeTree   atomic.Pointer[bart.Table[routing.Gateways]]
 	linkAddr    *netroute.LinkAddr
-	// hostOwned means the fd arrived from the OS, which has already configured addressing, mtu
-	// and routes for it. NEPacketTunnelProvider on darwin does this.
-	hostOwned bool
-	l         *slog.Logger
+	l           *slog.Logger
 }
 
 type ifReq struct {
@@ -153,48 +150,8 @@ func (t *tun) deviceBytes() (o [16]byte) {
 	return
 }
 
-// newTunFromFd adopts a utun the host already created and configured, which is how a darwin
-// network extension is handed its device. Everything about moving packets is shared with newTun,
-// only the setup differs: the host owns addressing and routing here.
-func newTunFromFd(c *config.C, l *slog.Logger, deviceFd int, vpnNetworks []netip.Prefix) (*tun, error) {
-	if err := unix.SetNonblock(deviceFd, true); err != nil {
-		// We own the fd from the moment it is handed to us
-		_ = unix.Close(deviceFd)
-		return nil, fmt.Errorf("failed to set the tun fd to non-blocking mode: %w", err)
-	}
-
-	file := os.NewFile(uintptr(deviceFd), "/dev/tun")
-	t := &tun{
-		f:           file,
-		Device:      utunNameFromFd(deviceFd),
-		vpnNetworks: vpnNetworks,
-		DefaultMTU:  c.GetInt("tun.mtu", DefaultMTU),
-		hostOwned:   true,
-		l:           l,
-	}
-
-	if err := t.reload(c, true); err != nil {
-		_ = file.Close()
-		return nil, err
-	}
-
-	c.RegisterReloadCallback(func(c *config.C) {
-		if err := t.reload(c, false); err != nil {
-			util.LogWithContextIfNeeded("failed to reload tun device", err, t.l)
-		}
-	})
-
-	return t, nil
-}
-
-// utunNameFromFd asks the socket what interface it is, for logs. A blank name is not worth
-// failing a tunnel over, so an error just leaves it empty.
-func utunNameFromFd(fd int) string {
-	name, err := unix.GetsockoptString(fd, unix.AF_SYS_CONTROL, _UTUN_OPT_IFNAME)
-	if err != nil {
-		return ""
-	}
-	return name
+func newTunFromFd(_ *config.C, _ *slog.Logger, _ int, _ []netip.Prefix) (*tun, error) {
+	return nil, fmt.Errorf("newTunFromFd not supported in Darwin")
 }
 
 func (t *tun) Close() error {
@@ -205,12 +162,6 @@ func (t *tun) Close() error {
 }
 
 func (t *tun) Activate() error {
-	// The host handed us a configured device. Its addresses, mtu and routes come from the network
-	// settings it applied, and a sandboxed extension cannot change them anyway.
-	if t.hostOwned {
-		return nil
-	}
-
 	devName := t.deviceBytes()
 
 	s, err := unix.Socket(
@@ -424,11 +375,6 @@ func getLinkAddr(name string) (*netroute.LinkAddr, error) {
 }
 
 func (t *tun) addRoutes(logErrors bool) error {
-	// The route tree is still ours, the system routing table is not
-	if t.hostOwned {
-		return nil
-	}
-
 	routes := *t.Routes.Load()
 
 	for _, r := range routes {
@@ -458,10 +404,6 @@ func (t *tun) addRoutes(logErrors bool) error {
 }
 
 func (t *tun) removeRoutes(routes []Route) error {
-	if t.hostOwned {
-		return nil
-	}
-
 	for _, r := range routes {
 		if !r.Install {
 			continue