diam: don't close the connection on an unregistered application/command

[towi]

Summary

• readHeader's FindCommand lookup hard-fails for any (application, command) pair the loaded dictionary doesn't know about — e.g. an application whose XML dictionary isn't part of dict.Default at all, such as 3GPP Cx/Dx (16777216). ReadMessage then returns an error for an otherwise well-formed message.
• conn.serve() responds to any ReadMessage error by closing the whole connection (c.rwc.Close(); break) — so one message on an unregistered application takes down the entire peer link, including every other request in flight on it.
• Fix: fall back to a permissive Command instead of erroring, so decodeAVPs still runs. AVP-level lookups already degrade to datatype.Unknown for codes the dictionary doesn't recognize (dict.FindAVPByCode), so the message decodes without error — its AVPs just come back untyped when the application itself is unregistered.

Found this while load-testing a Diameter HSS: a client advertising both S6a/SWx (which dict.Default ships) and Cx/Dx (which it doesn't) in its CER would have its whole connection killed by a single Cx answer, even though the peer sent a perfectly valid message.

Test plan

• Added TestReadMessageUnregisteredApplication, reproducing the crash with a well-formed message on an unregistered application-id, asserting ReadMessage no longer errors.
• go test ./diam/... passes (existing suite untouched).
• go vet ./diam/... clean.
• Verified against a real Diameter peer via xk6-diameter load-testing a Cx MAR/MAA exchange against a live HSS — went from every request timing out (connection silently torn down) to 100% success.

[towi]

Minimal repro

Self-contained, no sipgate-specific anything: a stock go-diameter server + a k6/xk6-diameter client in one image. Confirmed on unpatched main — 2 of 3 checks fail, one message on an unregistered application (16777216, 3GPP Cx/Dx) kills the whole connection. With this PR's fix applied to both sides, all 3 pass.

Dockerfile

COMMENT_DOCKERFILE_EOF
# Minimal, self-contained repro for the go-diameter connection-close bug.
# Builds a stock (unpatched) go-diameter server and a k6+xk6-diameter
# client into one image; the entrypoint runs both and prints the result.
#
# Build & run:
#   docker build -t godiam-repro .
#   docker run --rm godiam-repro

FROM golang:1.25 AS server-build
WORKDIR /src
RUN go mod init repro && go get github.com/fiorix/go-diameter/v4@main
COPY server.go .
RUN go mod tidy && go build -o /out/server server.go

FROM grafana/xk6 AS client-build
RUN git clone https://github.com/lwlee2608/xk6-diameter.git /xk6/xk6-diameter && \
    git -C /xk6/xk6-diameter checkout 48db2efd4febbcb756a06218f74c5101f7eb37ed
RUN xk6 build latest \
    --with github.com/matrixxsoftware/xk6-diameter=/xk6/xk6-diameter \
    --output /xk6/k6

FROM debian:trixie-slim
RUN apt-get update && apt-get install -y --no-install-recommends ca-certificates && \
    rm -rf /var/lib/apt/lists/*
COPY --from=server-build /out/server /usr/bin/diam-server
COPY --from=client-build /xk6/k6 /usr/bin/k6
COPY repro.js /repro.js
COPY entrypoint.sh /entrypoint.sh
RUN chmod +x /entrypoint.sh
ENTRYPOINT ["/entrypoint.sh"]

server.go

// Minimal Diameter server, stock go-diameter, no custom dictionary. Answers
// any request it manages to decode with a generic Success. Demonstrates
// that receiving a single well-formed message on an application-id the
// dictionary doesn't know about (e.g. 16777216, 3GPP Cx/Dx) kills the whole
// connection -- not just that one request.
package main

import (
	"log"

	"github.com/fiorix/go-diameter/v4/diam"
	"github.com/fiorix/go-diameter/v4/diam/avp"
	"github.com/fiorix/go-diameter/v4/diam/datatype"
	"github.com/fiorix/go-diameter/v4/diam/sm"
)

func main() {
	settings := &sm.Settings{
		OriginHost:       datatype.DiameterIdentity("server"),
		OriginRealm:      datatype.DiameterIdentity("go-diameter"),
		VendorID:         13,
		ProductName:      "go-diameter-repro-server",
		FirmwareRevision: 1,
	}
	mux := sm.New(settings)
	mux.HandleFunc("ALL", func(c diam.Conn, m *diam.Message) {
		log.Printf("received:\n%s", m)
		a := m.Answer(diam.Success)
		a.NewAVP(avp.OriginHost, avp.Mbit, 0, settings.OriginHost)
		a.NewAVP(avp.OriginRealm, avp.Mbit, 0, settings.OriginRealm)
		if _, err := a.WriteTo(c); err != nil {
			log.Printf("write answer: %v", err)
			return
		}
		log.Printf("sent:\n%s", a)
	})
	go func() {
		for report := range mux.ErrorReports() {
			log.Printf("mux error report: %v", report)
		}
	}()
	log.Println("listening on :3868")
	log.Fatal(diam.ListenAndServe(":3868", mux, nil))
}

entrypoint.sh

#!/bin/sh
set -e
/usr/bin/diam-server &
SERVER_PID=$!
sleep 1
k6 run /repro.js
STATUS=$?
kill $SERVER_PID 2>/dev/null || true
exit $STATUS

repro.js

// Reproduces: a single well-formed message on an application-id the
// server's dictionary doesn't know about (e.g. 16777216, 3GPP Cx/Dx --
// not part of dict.Default) kills the WHOLE connection, not just that
// one request. Uses xk6-diameter (github.com/lwlee2608/xk6-diameter,
// built on fiorix/go-diameter) purely as a Diameter client; the bug
// itself lives in go-diameter, triggered here against a stock,
// unpatched go-diameter server (server.go, same repro bundle).
//
// Expected on unpatched go-diameter: step 1 succeeds, step 2 hangs/errors
// (the server's conn.serve() already died decoding the request header --
// FindCommand fails for the unregistered app-id), step 3 fails too, on
// the SAME connection that step 1 used successfully.
import diam from "k6/x/diameter";
import avpmod from "k6/x/diameter/avp";
import { check } from "k6";

export const options = {
  scenarios: {
    smoke: { executor: "per-vu-iterations", vus: 1, iterations: 1, maxDuration: "30s" },
  },
};

const TARGET = __ENV.TARGET || "127.0.0.1:3868";
const data = diam.DataType();

const CMD_R = 0x80; // Request bit
const FLAG_M = 0x40; // Mandatory
const APP_CREDIT_CONTROL = 4; // Charging Control -- common app both sides advertise
const APP_UNREGISTERED = 16777216; // 3GPP Cx/Dx -- not in dict.Default
const CMD_CCR = 272; // Credit-Control-Request, well-known under APP_CREDIT_CONTROL
const AVP_SESSION_ID = 263;
const AVP_ORIGIN_HOST = 264;
const AVP_RESULT_CODE = 268;
const AVP_ORIGIN_REALM = 296;
const AVP_DESTINATION_REALM = 283;

function newClient() {
  return diam.Client({
    requestTimeout: "5s",
    enableWatchdog: false,
    authApplicationId: [APP_CREDIT_CONTROL],
    capabilityExchange: {
      vendorId: 13,
      productName: "xk6-diameter-repro",
      originHost: "client.repro",
      originRealm: "repro.local",
    },
  });
}

// A minimal, well-formed request on the given (cmdCode, appId).
function request(cmdCode, appId, sessionSuffix) {
  const m = diam.newMessage(cmdCode, appId, CMD_R);
  m.add(avpmod.New(AVP_SESSION_ID, 0, FLAG_M, data.UTF8String("repro;1;" + sessionSuffix)));
  m.add(avpmod.New(AVP_ORIGIN_HOST, 0, FLAG_M, data.DiameterIdentity("client.repro")));
  m.add(avpmod.New(AVP_ORIGIN_REALM, 0, FLAG_M, data.DiameterIdentity("repro.local")));
  m.add(avpmod.New(AVP_DESTINATION_REALM, 0, FLAG_M, data.DiameterIdentity("repro.local")));
  return m;
}

// Sends msg and reports whether an answer with Result-Code 2001 came back.
function trySend(client, msg, label) {
  try {
    const answer = client.send(msg);
    const rc = answer.findAVP(AVP_RESULT_CODE, 0);
    console.log(`${label}: OK, result-code=${rc}`);
    return Number(rc) === 2001;
  } catch (e) {
    console.log(`${label}: FAILED -- ${e}`);
    return false;
  }
}

// Like trySend, but only asks "did an answer come back at all" -- used for
// the unregistered-app request, whose Result-Code decodes as untyped
// datatype.Unknown rather than 2001 (FindAVPByCode doesn't chain to the
// base app for an app-id it doesn't know), so a typed comparison doesn't
// apply here. Not throwing is exactly what this fix guarantees.
function trySendExpectNoThrow(client, msg, label) {
  try {
    const answer = client.send(msg);
    console.log(`${label}: OK, got an answer (${answer.findAVP(AVP_RESULT_CODE, 0)})`);
    return true;
  } catch (e) {
    console.log(`${label}: FAILED -- ${e}`);
    return false;
  }
}

export default function () {
  const client = newClient();
  try {
    client.connect(TARGET);
  } catch (e) {
    console.log(`connect: FAILED -- ${e}`);
    check(null, { "connected": () => false });
    return;
  }

  const step1 = trySend(client, request(CMD_CCR, APP_CREDIT_CONTROL, 1), "1. baseline request (known app)");
  check(null, { "1. baseline request succeeds": () => step1 });

  const step2 = trySendExpectNoThrow(client, request(303, APP_UNREGISTERED, 2), "2. request on unregistered app-id");
  check(null, { "2. unregistered-app request gets an answer": () => step2 });

  // Same client/connection as step 1 -- proves the bad message in step 2
  // took the whole connection down, not just its own request.
  const step3 = trySend(client, request(CMD_CCR, APP_CREDIT_CONTROL, 3), "3. baseline request again, same connection");
  check(null, { "3. connection survives the bad message": () => step3 });
}

Run:

docker build -t godiam-repro .
docker run --rm godiam-repro

Unpatched (main):

✓ 1. baseline request succeeds
✗ 2. unregistered-app request gets an answer
✗ 3. connection survives the bad message
checks..................: 33.33% ✓ 1  ✗ 2

With this fix (both server and client built against the fix branch):

✓ 1. baseline request succeeds
✓ 2. unregistered-app request gets an answer
✓ 3. connection survives the bad message
checks..................: 100.00% ✓ 3  ✗ 0