Correct core-ring reflect policy: bare reflect is fine, serialization isn't

Empirically verified crypto/rand (which transitively imports reflect)
builds and links under real TinyGo (0.41.1). The old archtest rule
banned "reflect" itself as a proxy for "TinyGo doesn't support this";
that proxy was wrong — TinyGo's reflect works, it's generic
reflection-based *serialization* (struct-tag walking to encode/decode
arbitrary types) that's unreliable there. Narrowed core/internal/archtest's
forbidden list accordingly: dropped the blanket "reflect" ban, kept
encoding/json, encoding/binary, and database/sql banned by name (with
corrected rationale), and documented why fmt still isn't explicitly
banned (crypto/rand itself transitively imports fmt, so banning it would
break the very fix this commit makes).

Two concrete consequences, found by grepping every "reflect" comment in
core/adapter/client for places functionality was pushed out of core
specifically because of the old (over-broad) rule:

- core/csnet.RandomMAC is now the canonical implementation (moved from
  client/link, which keeps a thin wrapper for its existing callers/API).
- core/auth gains NewCredential, generating its own random salt via
  crypto/rand instead of requiring every caller to generate one and pass
  it to DeriveCredential. adapter/auth/local's Store now calls it instead
  of hand-rolling salt generation; that package still lives in the
  adapter ring, but now correctly for its own reason (file I/O), not a
  stale crypto/rand rationale.

Also, per direct instruction: core/port.ParseMAC now delegates to
core/csnet.ParseMAC (net.ParseMAC) instead of its own hand-rolled parser,
so it matches every other MAC parser in the codebase exactly. This is a
deliberate behavior change: a single-nibble octet like "0:11:22:aa:bb:cc"
is no longer accepted (net.ParseMAC has always rejected it).

Surveyed the rest of the "reflection-free" comments across core/protocol/*
et al. (hand-rolled big-endian codecs, hex formatting) — these remain
correctly scoped: encoding/binary is still banned (Read/Write are the
actual generic-serialization concern), so core/binaryprimitives and
similar hand-rolled helpers are unaffected by this policy correction.

Verified: go build/vet/test -tags all, TestCoreImportGraph (archtest),
and a real TinyGo (0.41.1) build of cmd/cs-tinygo's linux/amd64 target.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Author: pgodwin

adapter/auth/local/store.go

@@ -3,13 +3,15 @@
 // colon-separated, salted PBKDF2-SHA256 hashes), separate from server.toml so
 // secrets never ride the main config or its numbered backups.
 //
-// It lives in the ADAPTER ring, not core, for one reason: generating a random
-// salt needs crypto/rand, which transitively imports reflect — banned in core by
-// the archtest gate (§1). The hashing/verification itself stays in core/auth
-// (reflection-free PBKDF2); this adapter supplies the randomness and the os file
-// I/O. It is the default store the way adapter/store/file and
-// adapter/metastore/sqlite are defaults — pure stdlib, no new dependency, no build
-// tag. A future PAM/Windows store is an additional adapter under adapter/auth/*.
+// It lives in the ADAPTER ring, not core, because it does file I/O (os,
+// path/filepath) — an adapter concern regardless of what core itself may
+// import. Credential generation/hashing/verification (including salt
+// generation via crypto/rand, which is fine in core — see
+// core/auth.NewCredential) stays in core/auth; this adapter supplies only the
+// on-disk format and file I/O. It is the default store the way
+// adapter/store/file and adapter/metastore/sqlite are defaults — pure stdlib,
+// no new dependency, no build tag. A future PAM/Windows store is an additional
+// adapter under adapter/auth/*.
 //
 // File format (one line per user; '#' comments and blank lines ignored):
 //
@@ -21,7 +23,6 @@
 package local
 
 import (
-	"crypto/rand"
 	"errors"
 	"os"
 	"path/filepath"
@@ -148,11 +149,10 @@ func (s *Store) SetUser(username, password string) error {
 	if password == "" {
 		return auth.ErrEmptyPassword
 	}
-	salt := make([]byte, auth.SaltLen)
-	if _, err := rand.Read(salt); err != nil {
+	cred, err := auth.NewCredential(password)
+	if err != nil {
 		return err
 	}
-	cred := auth.DeriveCredential(password, salt)
 
 	s.mu.Lock()
 	defer s.mu.Unlock()

client/link/random.go

@@ -1,6 +1,6 @@
 package link
 
-import "crypto/rand"
+import "github.com/ObsoleteMadness/ClassicStack/core/csnet"
 
 // RandomMAC returns a synthetic locally-administered unicast Ethernet address for
 // a virtual station: the first octet has the locally-administered bit set and the
@@ -10,13 +10,10 @@ import "crypto/rand"
 // borrow the host NIC's identity (which would collide, and on Windows cannot even
 // be resolved from an "\Device\NPF_{GUID}" name).
 //
-// This lives here rather than in core/csnet because it needs crypto/rand, which
-// transitively imports reflect — forbidden in the core ring (core/internal/archtest,
-// §1). Every current caller (client/etherdfs, client/ncp, client/netbios,
-// client/smb, cmd/internal/csconnect) already imports this package.
+// Delegates to core/csnet.RandomMAC, the canonical implementation (crypto/rand is
+// allowed in core — see core/csnet/random.go's doc comment). Kept as a wrapper
+// here since client/etherdfs.RandomMAC, client/ncp.RandomMAC,
+// client/netbios.RandomMAC, and client/smb.RandomMAC all call it by this name.
 func RandomMAC() [6]byte {
-	var mac [6]byte
-	_, _ = rand.Read(mac[:])
-	mac[0] = (mac[0] | 0x02) &^ 0x01 // locally-administered, unicast
-	return mac
+	return csnet.RandomMAC()
 }

client/link/random_test.go

@@ -2,6 +2,8 @@ package link
 
 import "testing"
 
+// Full RandomMAC coverage lives in core/csnet (csnet_test.go); this just proves
+// the wrapper actually delegates.
 func TestRandomMAC(t *testing.T) {
 	mac := RandomMAC()
 	if mac[0]&0x02 == 0 {
@@ -10,7 +12,4 @@ func TestRandomMAC(t *testing.T) {
 	if mac[0]&0x01 != 0 {
 		t.Errorf("RandomMAC() first octet %02x: unicast bit (0x01) should be clear", mac[0])
 	}
-	if other := RandomMAC(); mac == other {
-		t.Error("RandomMAC() returned the same address twice in a row (rand.Read broken?)")
-	}
 }

cmd/cs-tinygo/main.go

@@ -58,9 +58,10 @@ import (
 
 	// M8a: the authentication seam's CONTRACT + PBKDF2 credential codec must stay
 	// TinyGo-clean so an embedded build can gate share access. Only core/auth is
-	// blank-imported: it is reflection-free (hand-rolled hex, no crypto/rand). The
-	// file-backed store lives in adapter/auth/local — it uses crypto/rand (which
-	// pulls reflect) and os, so it is deliberately an adapter, not part of this gate.
+	// blank-imported (crypto/rand is fine here — see core/csnet/random.go — but
+	// hex coding stays hand-rolled regardless, matching core/binaryprimitives'
+	// style). The file-backed store lives in adapter/auth/local — it needs os for
+	// file I/O, so it is deliberately an adapter, not part of this gate.
 	_ "github.com/ObsoleteMadness/ClassicStack/core/auth"
 
 	// M1: the pure-Go pcapfile capture writer is required to be TinyGo-safe (§6f)

core/auth/auth.go

@@ -10,12 +10,15 @@
 // modern on our side of the bridge, faithful to the client's insecure dialect on
 // the wire.
 //
-// core discipline: this package imports only stdlib crypto (crypto/sha256,
-// crypto/rand, crypto/subtle, encoding/hex) — no net, no reflect, no
-// encoding/binary, no sqlite — so it compiles for embedded/TinyGo targets and
-// passes the archtest gate. A concrete file-backed store lives in the
-// core/auth/local subpackage (it needs os); a netless target that does not need
-// it simply does not import it.
+// core discipline: this package imports only stdlib crypto (crypto/hmac,
+// crypto/rand, crypto/sha256, crypto/subtle) plus errors — no net, no
+// encoding/binary, no encoding/json, no sqlite — so it compiles for
+// embedded/TinyGo targets and passes the archtest gate (hex coding is
+// hand-rolled in cred.go rather than encoding/hex, matching core/binaryprimitives'
+// style elsewhere in core). A concrete file-backed store lives in the
+// adapter/auth/local package (it needs os for file I/O — an adapter concern
+// regardless of what core itself may import); a netless target that does not
+// need it simply does not import it.
 package auth
 
 import (

core/auth/cred.go

@@ -2,6 +2,7 @@ package auth
 
 import (
 	"crypto/hmac"
+	"crypto/rand"
 	"crypto/sha256"
 	"crypto/subtle"
 	"errors"
@@ -12,14 +13,15 @@ import (
 // cost field. PBKDF2-HMAC-SHA256 is implemented here over crypto/hmac +
 // crypto/sha256 so the package needs no golang.org/x/crypto dependency.
 //
-// core discipline (§1 / archtest): this file imports only crypto/hmac,
-// crypto/sha256 and crypto/subtle — all reflection-free. It deliberately does NOT
-// import crypto/rand (which transitively pulls reflect) or encoding/hex (likewise):
-// SALT GENERATION is the caller's job (a store adapter, which may use crypto/rand
-// in the adapter ring), and hex coding is hand-rolled below. So the contract stays
-// TinyGo-clean while the randomness lives where reflect is allowed.
+// core discipline (§1 / archtest): crypto/hmac, crypto/sha256, crypto/subtle,
+// and crypto/rand are all fine in core — reflect itself builds and links under
+// TinyGo (see core/csnet/random.go); the archtest gate bans specific generic
+// reflection-based *serialization* packages (encoding/json, encoding/binary,
+// database/sql), not reflect itself. So salt generation (NewCredential) lives
+// here now rather than being the caller's job; hex coding stays hand-rolled
+// below regardless, matching core/binaryprimitives' style elsewhere in core.
 const (
-	SaltLen        = 16     // expected salt length in bytes (the adapter generates it)
+	SaltLen        = 16     // salt length in bytes NewCredential generates
 	credIterations = 100000 // PBKDF2 iteration count
 	credKeyLen     = 32     // derived key length (SHA-256 output size)
 )
@@ -36,17 +38,29 @@ type Credential struct {
 	Hash []byte
 }
 
-// DeriveCredential derives a Credential for password under the supplied salt. The
-// caller (a store adapter) provides the salt — generated with crypto/rand for a
-// new user, or decoded from storage when re-deriving. Keeping rand out of here is
-// what lets core/auth stay reflection-free.
+// DeriveCredential derives a Credential for password under the supplied salt.
+// Used to re-derive an existing user's credential (e.g. to re-verify against a
+// stored salt) or by NewCredential for a fresh one; a caller decoding a stored
+// record uses this directly with the salt from storage.
 func DeriveCredential(password string, salt []byte) Credential {
 	return Credential{
 		Salt: salt,
 		Hash: pbkdf2SHA256([]byte(password), salt, credIterations, credKeyLen),
 	}
 }
 
+// NewCredential generates a fresh random SaltLen-byte salt and derives a
+// Credential for password under it — the counterpart to DeriveCredential for
+// creating a new user (SetUser/change-password) rather than re-verifying one
+// already on disk.
+func NewCredential(password string) (Credential, error) {
+	salt := make([]byte, SaltLen)
+	if _, err := rand.Read(salt); err != nil {
+		return Credential{}, err
+	}
+	return DeriveCredential(password, salt), nil
+}
+
 // Verify reports whether password matches the credential, in constant time. A
 // zero-value (no Salt/Hash) credential never verifies.
 func (c Credential) Verify(password string) bool {

core/auth/cred_test.go

@@ -61,6 +61,26 @@ func TestCredentialRoundTrip(t *testing.T) {
 	}
 }
 
+func TestNewCredential(t *testing.T) {
+	c, err := NewCredential("hunter2")
+	if err != nil {
+		t.Fatalf("NewCredential: unexpected error: %v", err)
+	}
+	if len(c.Salt) != SaltLen || len(c.Hash) != credKeyLen {
+		t.Fatalf("credential sizes salt=%d hash=%d", len(c.Salt), len(c.Hash))
+	}
+	if !c.Verify("hunter2") {
+		t.Fatal("Verify rejected the correct password")
+	}
+	other, err := NewCredential("hunter2")
+	if err != nil {
+		t.Fatalf("NewCredential (second call): unexpected error: %v", err)
+	}
+	if string(c.Salt) == string(other.Salt) {
+		t.Fatal("NewCredential produced the same salt twice in a row (rand.Read broken?)")
+	}
+}
+
 func TestCredentialSaltMakesHashesDiffer(t *testing.T) {
 	saltA := []byte("aaaaaaaaaaaaaaaa")
 	saltB := []byte("bbbbbbbbbbbbbbbb")

core/csnet/csnet_test.go

@@ -66,6 +66,19 @@ func TestParseIPv4(t *testing.T) {
 	}
 }
 
+func TestRandomMAC(t *testing.T) {
+	mac := csnet.RandomMAC()
+	if mac[0]&0x02 == 0 {
+		t.Errorf("RandomMAC() first octet %02x: locally-administered bit (0x02) not set", mac[0])
+	}
+	if mac[0]&0x01 != 0 {
+		t.Errorf("RandomMAC() first octet %02x: unicast bit (0x01) should be clear", mac[0])
+	}
+	if other := csnet.RandomMAC(); mac == other {
+		t.Error("RandomMAC() returned the same address twice in a row (rand.Read broken?)")
+	}
+}
+
 func TestParseIPv4_Rejects(t *testing.T) {
 	cases := []string{"", "10.0.0", "10.0.0.256", "10.0.0.1.2", "not-an-ip", "::1"}
 	for _, in := range cases {

core/csnet/doc.go

@@ -1,16 +1,13 @@
 // Package csnet provides shared MAC and IPv4 address parsing/formatting for both
-// desktop and TinyGo/embedded builds: ParseMAC/FormatMAC and ParseIPv4/IPv4.
+// desktop and TinyGo/embedded builds: ParseMAC/FormatMAC, ParseIPv4/IPv4, and
+// RandomMAC.
 //
-// Two implementations exist per concern, selected by the tinygo build tag: the
-// default (!tinygo) build wraps the standard library's net package; the tinygo
-// build hand-rolls the same operation, since TinyGo's net package does not
-// reliably provide ParseMAC/ParseIP on baremetal targets. This mirrors the split
-// core/buf and core/hostinfo already use for target-specific behavior (§1) —
-// callers use the same API regardless of which build produced it.
-//
-// RandomMAC deliberately does NOT live here: it needs crypto/rand, which
-// transitively imports reflect and is therefore forbidden in the core ring
-// (core/internal/archtest, §1). See client/link.RandomMAC instead — every
-// current caller (client/etherdfs, client/ncp, client/netbios, client/smb,
-// cmd/internal/csconnect) already imports client/link.
+// ParseMAC and ParseIPv4 have two implementations each, selected by the tinygo
+// build tag: the default (!tinygo) build wraps the standard library's net
+// package; the tinygo build hand-rolls the same operation, since TinyGo's net
+// package does not reliably provide ParseMAC/ParseIP on baremetal targets. This
+// mirrors the split core/buf and core/hostinfo already use for target-specific
+// behavior (§1) — callers use the same API regardless of which build produced
+// it. RandomMAC needs no such split: crypto/rand builds and links fine under
+// TinyGo (see random.go's doc comment for why it's allowed in core at all).
 package csnet

core/csnet/random.go

@@ -0,0 +1,22 @@
+package csnet
+
+import "crypto/rand"
+
+// RandomMAC returns a synthetic locally-administered unicast Ethernet address: the
+// first octet has the locally-administered bit set and the group/multicast bit
+// clear, the rest are random. Used to give a virtual station (a probe tool, a
+// client-side transport) its own identity distinct from the host NIC's real MAC,
+// so it doesn't collide with the host on the wire.
+//
+// crypto/rand transitively imports reflect, but reflect itself builds and links
+// fine under TinyGo (verified with the real toolchain) — the core ring only bans
+// the specific packages that do generic reflection-based *serialization*
+// (encoding/json, encoding/binary, database/sql; see core/internal/archtest),
+// which crypto/rand is not. So, unlike an earlier version of this package,
+// RandomMAC lives here rather than in client/link.
+func RandomMAC() [6]byte {
+	var mac [6]byte
+	_, _ = rand.Read(mac[:])
+	mac[0] = (mac[0] | 0x02) &^ 0x01 // locally-administered, unicast
+	return mac
+}