mesh-cli: follow a line to its answer, bound each account, refuse other namespaces (review of hq ADR 0272)

- A request that timed out was said as "the controller was not reached,
  nothing ran", though the line may have run. The engine now follows a line
  the controller answers as running, by its call, until it ends; when no
  answer comes in time it says the line may have run and names the call.
  "Nothing ran" is said only of a request no controller took.
- One account could hold the socket: accepting waited for a free slot, and
  a connection could stall before its deadline was set. The deadline is set
  at accept, before any lookup; each user id has two lines at once and the
  machine eight, and one over is answered busy at once.
- A container running with the operator's user id shares the number, not
  the machine: a peer whose mount or user namespace is not the engine's is
  refused, and nothing is asked.

SDK pinned at the go/cli branch's newest commit until go/v0.1.9 is tagged.

internal/meshcli/meshcli.go

@@ -28,28 +28,65 @@ import (
 // Asked is what the node-engine sends the controller on its own node's subject: the line, and the account the
 // kernel says is asking. The controller's side holds the same field names (a test on each side).
 type Asked struct {
-	Line []string `json:"line"`
+	Line []string `json:"line,omitempty"`
 	// Account is the login of the user id the kernel gave, from this machine's user database.
 	Account string `json:"account"`
 	// UID is the user id itself, so a renamed or duplicated login is still the same id.
 	UID uint32 `json:"uid"`
+	// Follow is a call a line runs as, asked about again by the account that asked it (AskTheController).
+	Follow string `json:"follow,omitempty"`
 }
 
-// Forward asks the controller, and is answered. An error is said to the program as a refusal: nothing ran.
-type Forward func(ctx context.Context, asked Asked) (cli.Answer, error)
+// Forward asks the controller and follows the line to what came of it (AskTheController): what it printed, that
+// nothing ran and why, or that it is not known.
+type Forward func(ctx context.Context, asked Asked) cli.Answer
 
 // readWithin bounds how long a connection may take to say its line: a program that connects and says nothing
 // holds nothing for long.
 const readWithin = 30 * time.Second
 
-// atOnce bounds how many lines are asked at the same time. A person types one at a time; more than this is a
-// program on the machine, and it waits.
-const atOnce = 8
+// atOnce bounds how many lines are asked at the same time, and perAccount how many of them one account's: a person
+// types one at a time, so more is a program on the machine. One over either bound is answered busy at once — the
+// engine never stops accepting for it, so one account cannot hold the socket from every other (review of ADR 0272).
+var (
+	atOnce     = 8
+	perAccount = 2
+)
 
-// peerOf reads the connecting process's credentials from the kernel. A variable so a test can say what the kernel
-// would; it is the only source of the account.
+// slots counts the lines being asked, in all and per user id.
+type slots struct {
+	mu    sync.Mutex
+	all   int
+	byUID map[uint32]int
+}
+
+func (s *slots) take(uid uint32) bool {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	if s.all >= atOnce || s.byUID[uid] >= perAccount {
+		return false
+	}
+	s.all++
+	s.byUID[uid]++
+	return true
+}
+
+func (s *slots) give(uid uint32) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	s.all--
+	if s.byUID[uid]--; s.byUID[uid] <= 0 {
+		delete(s.byUID, uid)
+	}
+}
+
+// peerOf reads the connecting process's credentials from the kernel: its user id and process id. A variable so a
+// test can say what the kernel would; it is the only source of the account.
 var peerOf = peerCredentials
 
+// sameNamespaces refuses a peer whose mount or user namespace is not the engine's. A variable so a test can say.
+var sameNamespaces = namespacesOf
+
 // lookupAccount names a user id from this machine's user database.
 var lookupAccount = func(uid uint32) (string, error) {
 	u, err := user.LookupId(strconv.FormatUint(uint64(uid), 10))
@@ -99,7 +136,7 @@ func Serve(ctx context.Context, socket string, forward Forward, say func(string)
 		<-ctx.Done()
 		l.Close()
 	}()
-	slots := make(chan struct{}, atOnce)
+	held := &slots{byUID: map[uint32]int{}}
 	var wg sync.WaitGroup
 	defer wg.Wait()
 	for {
@@ -114,23 +151,19 @@ func Serve(ctx context.Context, socket string, forward Forward, say func(string)
 			}
 			return fmt.Errorf("mesh-cli's socket: %w", err)
 		}
-		select {
-		case slots <- struct{}{}:
-		case <-ctx.Done():
-			conn.Close()
-			return nil
-		}
+		// Bounded from the first byte: nothing below may wait on a connection for longer than this before the
+		// line is read, whatever the peer does (review of ADR 0272).
+		_ = conn.SetDeadline(time.Now().Add(readWithin))
 		wg.Add(1)
 		go func() {
 			defer wg.Done()
-			defer func() { <-slots }()
-			answer(ctx, conn, forward, say)
+			answer(ctx, conn, held, forward, say)
 		}()
 	}
 }
 
 // answer serves one connection.
-func answer(ctx context.Context, conn *net.UnixConn, forward Forward, say func(string)) {
+func answer(ctx context.Context, conn *net.UnixConn, held *slots, forward Forward, say func(string)) {
 	defer conn.Close()
 	reply := func(a cli.Answer) {
 		_ = conn.SetWriteDeadline(time.Now().Add(readWithin))
@@ -138,29 +171,38 @@ func answer(ctx context.Context, conn *net.UnixConn, forward Forward, say func(s
 			say(fmt.Sprintf("mesh-cli: the answer could not be written back: %v", err))
 		}
 	}
-	uid, err := peerOf(conn)
+	uid, pid, err := peerOf(conn)
 	if err != nil {
 		// Never sent as anyone: an account the kernel did not give is no account at all.
 		reply(cli.Refusal("the node-engine could not read which account is asking, so nothing was asked: " + err.Error()))
 		return
 	}
+	// The user id is this machine's account only in this machine's namespaces: a container running with the
+	// operator's id shares the number and nothing else (review of ADR 0272).
+	if err := sameNamespaces(pid); err != nil {
+		reply(cli.Refusal("the asking process does not run in this machine's own namespaces, so it is not this " +
+			"machine's account and nothing was asked: " + err.Error()))
+		return
+	}
+	if !held.take(uid) {
+		reply(cli.Refusal(fmt.Sprintf("busy: user id %d has %d lines being asked already, or the machine %d; ask "+
+			"again when one has ended. Nothing was asked", uid, perAccount, atOnce)))
+		return
+	}
+	defer held.give(uid)
 	account, err := lookupAccount(uid)
 	if err != nil || account == "" {
 		reply(cli.Refusal(fmt.Sprintf("user id %d is not in this machine's user database, so nothing was asked", uid)))
 		return
 	}
-	_ = conn.SetReadDeadline(time.Now().Add(readWithin))
 	req, err := cli.ReadRequest(conn)
 	if err != nil {
 		reply(cli.Refusal("the request could not be read, so nothing was asked: " + err.Error()))
 		return
 	}
-	asking, cancel := context.WithTimeout(ctx, cli.AskWithin-time.Minute/2)
+	// From here the wait is the controller's: as long as mesh-cli waits, and no longer.
+	_ = conn.SetDeadline(time.Now().Add(cli.AskWithin))
+	asking, cancel := context.WithTimeout(ctx, cli.AskWithin-30*time.Second)
 	defer cancel()
-	a, err := forward(asking, Asked{Line: req.Line, Account: account, UID: uid})
-	if err != nil {
-		reply(cli.Refusal("the controller was not reached, so nothing ran: " + err.Error()))
-		return
-	}
-	reply(a)
+	reply(forward(asking, Asked{Line: req.Line, Account: account, UID: uid}))
 }