sudo_intercept.so: send the secret immediately after connecting.

Sending the secret out of band, before the message size is read,
should make it harder to mount a DoS attack.

include/intercept.pb-c.h

@@ -86,7 +86,6 @@ struct  _HelloResponse
 
 /*
  * Policy check request from sudo_intercept.so.
- * Must include the correct secret value.
  * Note that the plugin API only currently supports passing
  * the new environment in to the open() function.
  */
@@ -100,11 +99,10 @@ struct  _PolicyCheckRequest
   size_t n_envp;
   char **envp;
   int32_t intercept_fd;
-  uint64_t secret;
 };
 #define POLICY_CHECK_REQUEST__INIT \
  { PROTOBUF_C_MESSAGE_INIT (&policy_check_request__descriptor) \
-    , (char *)protobuf_c_empty_string, (char *)protobuf_c_empty_string, 0,NULL, 0,NULL, 0, 0 }
+    , (char *)protobuf_c_empty_string, (char *)protobuf_c_empty_string, 0,NULL, 0,NULL, 0 }
 
 
 struct  _PolicyAcceptMessage

src/exec_intercept.c

@@ -53,6 +53,7 @@
 enum intercept_state {
     RECV_HELLO_INITIAL,
     RECV_HELLO,
+    RECV_SECRET,
     RECV_POLICY_CHECK,
     RECV_CONNECTION,
     POLICY_ACCEPT,
@@ -97,7 +98,7 @@ intercept_setup(int fd, struct sudo_event_base *evbase,
     }
 
     /* If we've already seen a ClientHello, expect a policy check first. */
-    closure->state = intercept_secret ? RECV_POLICY_CHECK : RECV_HELLO_INITIAL;
+    closure->state = intercept_secret ? RECV_SECRET : RECV_HELLO_INITIAL;
     closure->details = details;
     closure->listen_sock = -1;
 
@@ -294,13 +295,6 @@ intercept_check_policy(PolicyCheckRequest *req,
 	closure->errstr = N_("invalid PolicyCheckRequest");
 	goto done;
     }
-    if (req->secret != intercept_secret) {
-	closure->errstr = N_("invalid PolicyCheckRequest");
-	sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO,
-	    "secret mismatch: got %" PRIu64 ", expected %" PRIu64, req->secret,
-	    intercept_secret);
-	goto done;
-    }
 
     if (sudo_debug_needed(SUDO_DEBUG_INFO)) {
 	sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO,
@@ -455,6 +449,37 @@ done:
     debug_return_bool(ret);
 }
 
+/*
+ * Read shared secret from sudo_intercept.so and verify w/ intercept_secret.
+ * Returns true on success, false on mismatch and -1 on error.
+ */
+static int
+intercept_verify_secret(int fd)
+{
+    uint64_t secret;
+    ssize_t nread;
+    debug_decl(intercept_read_secret, SUDO_DEBUG_EXEC);
+
+    /* Read shared secret (uint64_t in host byte order). */
+    nread = recv(fd, &secret, sizeof(secret), 0);
+    if (nread != sizeof(secret)) {
+	if (nread == -1)
+	    debug_return_int(-1);
+	/* Treat short read as a secret mismatch. */
+	sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO,
+	    "short read, expected %zu, got %zd", sizeof(secret), nread);
+	debug_return_int(false);
+    }
+    if (secret != intercept_secret) {
+	sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO,
+	    "secret mismatch: got %" PRIu64 ", expected %" PRIu64, secret,
+	    intercept_secret);
+	debug_return_int(false);
+    }
+
+    debug_return_int(true);
+}
+
 /*
  * Read a message from sudo_intercept.so and act on it.
  */
@@ -470,6 +495,20 @@ intercept_read(int fd, struct intercept_closure *closure)
     int ttyfd = -1;
     debug_decl(intercept_read, SUDO_DEBUG_EXEC);
 
+    if (closure->state == RECV_SECRET) {
+	switch (intercept_verify_secret(fd)) {
+	case true:
+	    closure->state = RECV_POLICY_CHECK;
+	    break;
+	case false:
+	    goto done;
+	default:
+	    if (errno == EINTR || errno == EAGAIN)
+		debug_return_bool(true);
+	    goto done;
+	}
+    }
+
     if (closure->len == 0) {
 	uint32_t req_len;
 
@@ -479,7 +518,7 @@ intercept_read(int fd, struct intercept_closure *closure)
 	    if (nread == -1) {
 		if (errno == EINTR || errno == EAGAIN)
 		    debug_return_bool(true);
-		sudo_warn("read");
+		sudo_warn("recv");
 	    }
 	    goto done;
 	}
@@ -510,7 +549,7 @@ intercept_read(int fd, struct intercept_closure *closure)
     case -1:
 	if (errno == EINTR || errno == EAGAIN)
 	    debug_return_bool(true);
-	sudo_warn("read");
+	sudo_warn("recv");
 	goto done;
     default:
 	closure->off += nread;

src/intercept.pb-c.c

@@ -507,7 +507,7 @@ const ProtobufCMessageDescriptor hello_response__descriptor =
   (ProtobufCMessageInit) hello_response__init,
   NULL,NULL,NULL    /* reserved[123] */
 };
-static const ProtobufCFieldDescriptor policy_check_request__field_descriptors[6] =
+static const ProtobufCFieldDescriptor policy_check_request__field_descriptors[5] =
 {
   {
     "command",
@@ -569,18 +569,6 @@ static const ProtobufCFieldDescriptor policy_check_request__field_descriptors[6]
     0,             /* flags */
     0,NULL,NULL    /* reserved1,reserved2, etc */
   },
-  {
-    "secret",
-    6,
-    PROTOBUF_C_LABEL_NONE,
-    PROTOBUF_C_TYPE_FIXED64,
-    0,   /* quantifier_offset */
-    offsetof(PolicyCheckRequest, secret),
-    NULL,
-    NULL,
-    0,             /* flags */
-    0,NULL,NULL    /* reserved1,reserved2, etc */
-  },
 };
 static const unsigned policy_check_request__field_indices_by_name[] = {
   2,   /* field[2] = argv */
@@ -588,12 +576,11 @@ static const unsigned policy_check_request__field_indices_by_name[] = {
   1,   /* field[1] = cwd */
   3,   /* field[3] = envp */
   4,   /* field[4] = intercept_fd */
-  5,   /* field[5] = secret */
 };
 static const ProtobufCIntRange policy_check_request__number_ranges[1 + 1] =
 {
   { 1, 0 },
-  { 0, 6 }
+  { 0, 5 }
 };
 const ProtobufCMessageDescriptor policy_check_request__descriptor =
 {
@@ -603,7 +590,7 @@ const ProtobufCMessageDescriptor policy_check_request__descriptor =
   "PolicyCheckRequest",
   "",
   sizeof(PolicyCheckRequest),
-  6,
+  5,
   policy_check_request__field_descriptors,
   policy_check_request__field_indices_by_name,
   1,  policy_check_request__number_ranges,

src/intercept.proto

@@ -30,7 +30,6 @@ message HelloResponse {
 
 /*
  * Policy check request from sudo_intercept.so.
- * Must include the correct secret value.
  * Note that the plugin API only currently supports passing
  * the new environment in to the open() function.
  */
@@ -40,7 +39,6 @@ message PolicyCheckRequest {
   repeated string argv = 3;
   repeated string envp = 4;
   int32 intercept_fd = 5;
-  fixed64 secret = 6;
 }
 
 message PolicyAcceptMessage {

src/sudo_intercept_common.c

@@ -63,7 +63,7 @@ static in_port_t intercept_port;
 
 /* Send entire request to sudo (blocking). */
 static bool
-send_req(int sock, const uint8_t *buf, size_t len)
+send_req(int sock, const void *buf, size_t len)
 {
     const uint8_t *cp = buf;
     ssize_t nwritten;
@@ -94,7 +94,7 @@ send_client_hello(int sock)
     bool ret = false;
     debug_decl(send_client_hello, SUDO_DEBUG_EXEC);
 
-    /* Setup policy check request. */
+    /* Setup client hello. */
     hello.pid = getpid();
     msg.type_case = INTERCEPT_REQUEST__TYPE_HELLO;;
     msg.u.hello = &hello;
@@ -272,8 +272,14 @@ send_policy_check_req(int sock, const char *cmnd, char * const argv[],
     size_t len;
     debug_decl(fmt_policy_check_req, SUDO_DEBUG_EXEC);
 
+    /* Send secret first (out of band) to initiate connection. */
+    if (!send_req(sock, &secret, sizeof(secret))) {
+	sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO,
+	    "unable to send secret back to sudo");
+	goto done;
+    }
+
     /* Setup policy check request. */
-    req.secret = secret;
     req.intercept_fd = sock;
     req.command = (char *)cmnd;
     req.argv = (char **)argv;