iscsi-scstd: Reject CHAP challenges when entropy reads fail

Manual review found that chap_rand ignores read errors and short reads,
then returns a partially or wholly uninitialized integer. No saved
analyzer diagnostic directly reports this defect. A negative signed
value can also reduce the intended 25-to-49-byte challenge length.

Read a complete unsigned value, retry EINTR and return failures to the
caller. Reject challenge creation on entropy failure and free a partial
challenge before returning, without publishing CHAP_C.

Add an isolated Ginkgo suite using the production CHAP implementation
with deterministic entropy I/O. Cover length bounds, interrupted and
short reads, open/read failures, EOF and partial-challenge cleanup.
Prepare interface headers privately so probes need no preceding build.

iscsi-scst/usr/chap.c

@@ -411,17 +411,33 @@ chap_calc_digest_sha3_256(char chap_id, const char *secret, int secret_len,
  * To generate challenge for CHAP, use stronger random number generator as
  * opposed to simple rand().
  */
-static int chap_rand(void)
+static int chap_rand(unsigned int *value)
 {
-	int fd;
-	int r;
+	unsigned char *p = (unsigned char *)value;
+	size_t left = sizeof(*value);
+	int fd, err = 0;
 
 	fd = open("/dev/urandom", O_RDONLY);
-	assert(fd != -1);
-	if (read(fd, &r, sizeof(r)) < sizeof(r)) {
+	if (fd < 0)
+		return -errno;
+	while (left) {
+		ssize_t count = read(fd, p, left);
+
+		if (count < 0) {
+			if (errno == EINTR)
+				continue;
+			err = -errno;
+			break;
+		}
+		if (count == 0) {
+			err = -EIO;
+			break;
+		}
+		p += count;
+		left -= count;
 	}
 	close(fd);
-	return r;
+	return err;
 }
 
 static int chap_initiator_auth_create_challenge(struct connection *conn)
@@ -429,6 +445,7 @@ static int chap_initiator_auth_create_challenge(struct connection *conn)
 	char *value, *p;
 	char text[CHAP_CHALLENGE_MAX * 2 + 8];
 	static int chap_id;
+	unsigned int random;
 	int i;
 
 	value = text_key_find(conn, "CHAP_A");
@@ -467,7 +484,10 @@ static int chap_initiator_auth_create_challenge(struct connection *conn)
 	 * wise, or should we rather always use the max. allowed length of
 	 * 1024 for the (unencoded) challenge?
 	 */
-	conn->auth.chap.challenge_size = (chap_rand() % (CHAP_CHALLENGE_MAX / 2)) + CHAP_CHALLENGE_MAX / 2;
+	if (chap_rand(&random) < 0)
+		return CHAP_TARGET_ERROR;
+	conn->auth.chap.challenge_size =
+		random % (CHAP_CHALLENGE_MAX / 2) + CHAP_CHALLENGE_MAX / 2;
 
 	conn->auth.chap.challenge = malloc(conn->auth.chap.challenge_size);
 	if (!conn->auth.chap.challenge)
@@ -477,7 +497,13 @@ static int chap_initiator_auth_create_challenge(struct connection *conn)
 	strcpy(p, "0x");
 	p += 2;
 	for (i = 0; i < conn->auth.chap.challenge_size; i++) {
-		conn->auth.chap.challenge[i] = chap_rand();
+		if (chap_rand(&random) < 0) {
+			free(conn->auth.chap.challenge);
+			conn->auth.chap.challenge = NULL;
+			conn->auth.chap.challenge_size = 0;
+			return CHAP_TARGET_ERROR;
+		}
+		conn->auth.chap.challenge[i] = random;
 		sprintf(p, "%.2hhx", conn->auth.chap.challenge[i]);
 		p += 2;
 	}

tests/ginkgo/README.md

@@ -121,6 +121,12 @@ It preserves existing records and leaves initiator services and modules loaded.
 CHAP configuration tests require Python 3 on the target and the ability to
 run file-access checks under an unprivileged UID/GID.
 
+The isolated `internal/chap` suite tests the production CHAP implementation
+with deterministic entropy I/O: length bounds, short reads, interruptions and
+failure cleanup. It runs through `ginkgo-check` and requires `cc`. Interface
+headers are prepared in a temporary directory; no daemon or storage is used.
+Run it separately with `go test ./internal/chap` from this directory.
+
 ## SCSI and I/O
 
 Install Python 3, util-linux and sg3_utils on the command host: the target for

tests/ginkgo/internal/chap/chap_test.go

@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0-only
+package chap
+
+import (
+	_ "embed"
+	"os"
+	"path/filepath"
+	"runtime"
+	"testing"
+	"time"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
+	"scst-component-tests/internal/admin"
+	"scst-component-tests/internal/cprobe"
+)
+
+//go:embed testdata/random_probe.c
+var randomProbe string
+
+func TestCHAP(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "SCST CHAP randomness")
+}
+
+var _ = Describe("CHAP entropy failures", Label("unit", "userspace", "chap"), func() {
+	var probe string
+	var client *admin.Client
+
+	BeforeEach(func(ctx SpecContext) {
+		client = &admin.Client{Log: GinkgoWriter}
+		dir, err := os.MkdirTemp("", "scst-chap-probe-")
+		Expect(err).NotTo(HaveOccurred())
+		DeferCleanup(func() {
+			if client.Blocked() != nil {
+				AddReportEntry("preserved CHAP probe", dir)
+				return
+			}
+			Expect(os.RemoveAll(dir)).To(Succeed())
+		})
+		source := os.Getenv("SCST_TEST_SOURCE_DIR")
+		if source == "" {
+			_, file, _, ok := runtime.Caller(0)
+			Expect(ok).To(BeTrue())
+			source = filepath.Clean(filepath.Join(filepath.Dir(file), "../../../.."))
+		}
+		Expect(cprobe.PrepareISCSIHeaders(source, dir)).To(Succeed())
+		input := filepath.Join(dir, "random_probe.c")
+		Expect(os.WriteFile(input, []byte(randomProbe), 0600)).To(Succeed())
+		probe = filepath.Join(dir, "random-probe")
+		// Dead-section removal keeps this unit probe independent of the
+		// daemon's unrelated authentication and transport dependencies.
+		r, err := client.RunProgram(ctx, 30*time.Second,
+			"cc", "-D_GNU_SOURCE", "-O2", "-ffunction-sections", "-fdata-sections",
+			"-I"+dir,
+			"-I"+filepath.Join(source, "iscsi-scst/usr"),
+			"-I"+filepath.Join(source, "iscsi-scst/include"),
+			"-I"+filepath.Join(source, "scst/include"),
+			input, "-Wl,--gc-sections", "-o", probe)
+		Expect(err).NotTo(HaveOccurred(), r.String())
+		Expect(r.ExitCode).To(BeZero(), r.String())
+	})
+
+	DescribeTable("publishes only complete challenges from successful reads", func(ctx SpecContext, scenario string) {
+		r, err := client.RunProgram(ctx, 10*time.Second, probe, scenario)
+		Expect(err).NotTo(HaveOccurred(), r.String())
+		Expect(r.ExitCode).To(BeZero(), r.String())
+		Expect(r.Stdout).To(Equal("ok\n"), r.String())
+	},
+		Entry("zero entropy value retains the lower length bound", "zero"),
+		Entry("a set sign bit cannot shorten the challenge", "high-bit"),
+		Entry("interrupted and short reads complete the value", "interrupted-short"),
+		Entry("open failure rejects authentication", "open-error"),
+		Entry("read failure rejects authentication", "read-error"),
+		Entry("EOF rejects authentication", "eof"),
+		Entry("EOF after a short read rejects authentication", "partial-eof"),
+		Entry("payload failure discards the partial challenge", "payload-error"),
+	)
+})

tests/ginkgo/internal/chap/testdata/random_probe.c

@@ -0,0 +1,166 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#include <errno.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+static int probe_open(const char *path, int flags, ...);
+static ssize_t probe_read(int fd, void *buf, size_t size);
+static int probe_close(int fd);
+
+/* Compile the production implementation; only the entropy I/O is replaced. */
+#define open probe_open
+#define read probe_read
+#define close probe_close
+#include "chap.c"
+#undef open
+#undef read
+#undef close
+
+static void check(bool condition, int line, const char *expression)
+{
+	if (!condition) {
+		fprintf(stderr, "line %d: %s\n", line, expression);
+		exit(EXIT_FAILURE);
+	}
+}
+
+#define CHECK(condition) check(!!(condition), __LINE__, #condition)
+
+static const char *scenario;
+static unsigned int opens, closes, reads, current_reads;
+static unsigned int challenge_keys;
+static char encoded[CHAP_CHALLENGE_MAX * 2 + 8];
+static unsigned char fill;
+
+static bool is_case(const char *name)
+{
+	return strcmp(scenario, name) == 0;
+}
+
+static int probe_open(const char *path, int flags, ...)
+{
+	if (strcmp(path, "/dev/urandom") || flags != O_RDONLY) {
+		fprintf(stderr, "unexpected entropy open\n");
+		exit(2);
+	}
+	opens++;
+	current_reads = 0;
+	if (is_case("open-error")) {
+		errno = EACCES;
+		return -1;
+	}
+	return 123;
+}
+
+static ssize_t probe_read(int fd, void *buf, size_t size)
+{
+	if (fd != 123 || !size || size > sizeof(unsigned int)) {
+		fprintf(stderr, "unexpected entropy read\n");
+		exit(2);
+	}
+	reads++;
+	current_reads++;
+	if (is_case("interrupted-short") && reads == 1) {
+		errno = EINTR;
+		return -1;
+	}
+	if (is_case("read-error") ||
+	    (is_case("payload-error") && opens == 2 && current_reads == 2)) {
+		errno = EIO;
+		return -1;
+	}
+	if (is_case("eof") || (is_case("partial-eof") && current_reads == 2))
+		return 0;
+	if ((is_case("interrupted-short") && reads == 2) ||
+	    (is_case("partial-eof") && current_reads == 1) ||
+	    (is_case("payload-error") && opens == 2 && current_reads == 1))
+		size = 1;
+	memset(buf, fill, size);
+	return size;
+}
+
+static int probe_close(int fd)
+{
+	if (fd != 123) {
+		fprintf(stderr, "unexpected entropy close\n");
+		exit(2);
+	}
+	closes++;
+	return 0;
+}
+
+char *text_key_find(struct connection *conn, const char *key)
+{
+	static char algorithm[2];
+
+	/* CHAP negotiation parses a writable value with strsep(). */
+	memcpy(algorithm, "5", sizeof(algorithm));
+
+	return strcmp(key, "CHAP_A") == 0 ? algorithm : NULL;
+}
+
+void text_key_add(struct connection *conn, const char *key, const char *value)
+{
+	if (strcmp(key, "CHAP_C") == 0) {
+		challenge_keys++;
+		snprintf(encoded, sizeof(encoded), "%s", value);
+	}
+}
+
+bool af_alg_supported(char *algorithm)
+{
+	return false;
+}
+
+int main(int argc, char **argv)
+{
+	struct connection conn = { 0 };
+	bool success;
+	unsigned int value, i;
+	int result;
+
+	CHECK(argc == 2);
+	scenario = argv[1];
+	success = is_case("zero") || is_case("high-bit") || is_case("interrupted-short");
+	CHECK(success || is_case("open-error") || is_case("read-error") ||
+	      is_case("eof") || is_case("partial-eof") || is_case("payload-error"));
+	fill = is_case("zero") ? 0 : 0xff;
+	value = fill ? UINT_MAX : 0;
+	result = chap_initiator_auth_create_challenge(&conn);
+	if (success) {
+		CHECK(result == 0);
+		CHECK(conn.auth.chap.challenge_size >= CHAP_CHALLENGE_MAX / 2);
+		CHECK(conn.auth.chap.challenge_size < CHAP_CHALLENGE_MAX);
+		CHECK(conn.auth.chap.challenge_size == value % (CHAP_CHALLENGE_MAX / 2) +
+		      CHAP_CHALLENGE_MAX / 2);
+		CHECK(challenge_keys == 1);
+		CHECK(strlen(encoded) == 2 + 2 * conn.auth.chap.challenge_size);
+		CHECK(strncmp(encoded, "0x", 2) == 0);
+		for (i = 0; i < conn.auth.chap.challenge_size; i++) {
+			CHECK((unsigned char)conn.auth.chap.challenge[i] == fill);
+			CHECK(encoded[2 + 2 * i] == (fill ? 'f' : '0'));
+			CHECK(encoded[3 + 2 * i] == (fill ? 'f' : '0'));
+		}
+		CHECK(opens == 1 + conn.auth.chap.challenge_size);
+	} else {
+		CHECK(result == CHAP_TARGET_ERROR);
+		CHECK(challenge_keys == 0);
+		CHECK(!conn.auth.chap.challenge);
+		CHECK(conn.auth.chap.challenge_size == 0);
+	}
+	CHECK(closes == opens - (is_case("open-error") ? 1 : 0));
+	if (is_case("interrupted-short"))
+		CHECK(reads == opens + 2);
+	if (is_case("partial-eof"))
+		CHECK(reads == 2);
+	if (is_case("payload-error"))
+		CHECK(opens == 2 && reads == 3);
+	free(conn.auth.chap.challenge);
+	puts("ok");
+	return 0;
+}

tests/ginkgo/internal/cprobe/headers.go

@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Package cprobe prepares private build inputs for isolated daemon probes.
+package cprobe
+
+import (
+	"crypto/sha1"
+	"fmt"
+	"os"
+	"path/filepath"
+)
+
+// PrepareISCSIHeaders copies the current interface and generates its version
+// header using the same SHA-1 input as iscsi-scst/Makefile. The caller puts dir
+// first in the compiler include path; the source checkout remains untouched.
+func PrepareISCSIHeaders(source, dir string) error {
+	data, err := os.ReadFile(filepath.Join(source, "iscsi-scst/include/iscsi_scst.h"))
+	if err != nil {
+		return err
+	}
+	if err := os.WriteFile(filepath.Join(dir, "iscsi_scst.h"), data, 0600); err != nil {
+		return err
+	}
+	version := fmt.Sprintf("/* Generated for the isolated probe. */\n#define ISCSI_SCST_INTERFACE_VERSION ISCSI_VERSION_STRING \"_%x\"\n", sha1.Sum(data))
+	return os.WriteFile(filepath.Join(dir, "iscsi_scst_itf_ver.h"), []byte(version), 0600)
+}