auth: integrate Catapult CAT token verification

[mondain]

Summary

This is stacked on feature/auth / PR #264 and isolates the Catapult integration requested in review.

• Adds Quicr/catapult as a submodule and wires it into the CMake and Docker builds.
• Replaces the local v1 token envelope/CBOR/HMAC parsing with Catapult CWT validation and CAT MOQT claim translation.
• Keeps the existing relay-facing auth API (AuthTokenVerifier, Grants, and allows) so relay call sites do not churn in this PR.
• Updates auth tests to issue Catapult CWTs through the local test helper and keeps config resolver coverage from the parent branch.

Notes

The existing config shape is preserved. Configured HMAC secrets are SHA-256 derived into the 32-byte key Catapult expects, so deployments do not need a config format change in this stacked PR.

Validation

Docker-first focused validation on this machine:

docker run --rm -v "$PWD":/src -w /src debian:bookworm bash -lc '
set -e
apt-get update
apt-get install -y --no-install-recommends build-essential ninja-build git ca-certificates python3-pip libssl-dev libunwind-dev libgoogle-glog-dev libgflags-dev libdouble-conversion-dev libevent-dev libsodium-dev libzstd-dev libboost-all-dev libfmt-dev libgtest-dev libgmock-dev zlib1g-dev libc-ares-dev libdwarf-dev
pip install --break-system-packages cmake
cmake -S . -B /tmp/moqx-test -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_PREFIX_PATH=/src/.docker-deps/moxygen -DCMAKE_MODULE_PATH=/src/cmake -DCMAKE_POLICY_VERSION_MINIMUM=3.5 -DCMAKE_FIND_LIBRARY_SUFFIXES=".so;.a" -DGFLAGS_SHARED=ON
cmake --build /tmp/moqx-test --target moqx_auth_test moqx_config_resolver_test -j$(nproc)
ctest --test-dir /tmp/moqx-test -R "Auth|ConfigResolver" --output-on-failure
'

Result: 29/29 tests passed.

---

This change is 

[suhasHere]

@mondain I will review the PR this week.

[suhasHere]

src/auth/Auth.cpp line 188 at r1 (raw file):

  }
  return folly::makeUnexpected(AuthError::BadSignature);
}

this seems fine, but it does add hmac validation per kid even though it is not a match. how about the below change ( based on the latest main)

const auto tokenBytes = toBytes(token.tokenValue);
const auto span = std::span(tokenBytes.data(), tokenBytes.size());

auto tryVerify = [&](const HmacKeyConfig& key)
-> folly::Expected<Grants, AuthError> {
try {
catapult::HmacSha256Algorithm hmac(deriveHmacKey(key.secret));
auto cwt = catapult::Cwt::validateCwt(span, hmac);
auto grants = grantsFromToken(cwt.payload);
if (grants.expiresAt <= std::chrono::system_clock::now()) {
return folly::makeUnexpected(AuthError::Expired);
}
return grants;
} catch (const catapult::CryptoError&) {
return folly::makeUnexpected(AuthError::BadSignature);
} catch (const catapult::CatError&) {
return folly::makeUnexpected(AuthError::Malformed);
}
};

// If kid present, verify only with the matching key
auto header = catapult::Cwt::decodeHeader(span);
if (header.kid.has_value()) {
auto it = std::find_if(config_.hmacKeys.begin(), config_.hmacKeys.end(),
[&](const auto& key) { return key.id == *header.kid; });
if (it == config_.hmacKeys.end()) {
return folly::makeUnexpected(AuthError::BadSignature);
}
return tryVerify(*it);
}

// No kid — trial verification across all keys
for (const auto& key : config_.hmacKeys) {
auto result = tryVerify(key);
if (result.hasValue() || result.error() != AuthError::BadSignature) {
return result;
}
}
return folly::makeUnexpected(AuthError::BadSignature);

[suhasHere]

src/auth/Auth.cpp line 58 at r1 (raw file):

}

catapult::MoqtBinaryMatch toCatapultMatch(const std::vector<MatchRule>& rules) {

The old allRulesMatch iterated all rules with std::all_of. If any code path
(config, future callers) builds a scope with multiple rules (Prefix + Suffix to express
a range), the serialization quietly drops the extras

[suhasHere]

src/auth/Auth.cpp line 188 at r1 (raw file):

  }
  return folly::makeUnexpected(AuthError::BadSignature);
}

This is bit inefficient in terms of number of HMAC operations done if KID is not a match , also Keys don;t change after AuthTokenVerifier Consutrctions. How about we do this

Code snippet:

   std::unordered_map<std::string, catapult::HmacSha256Algorithm> hmacKeyMap_;
  std::vector<catapult::HmacSha256Algorithm*> hmacKeyList_;  // for no-kid fallback

  // In constructor
  for (const auto& key : config.hmacKeys) {
    auto [it, _] = hmacKeyMap_.emplace(key.id,
        catapult::HmacSha256Algorithm(deriveHmacKey(key.secret)));
    hmacKeyList_.push_back(&it->second);
  }

  const auto tokenBytes = toBytes(token.tokenValue);
  const auto span = std::span<const uint8_t>(tokenBytes.data(), tokenBytes.size());

  auto tryVerify = [&](const catapult::HmacSha256Algorithm& hmac)
      -> folly::Expected<Grants, AuthError> {
    try {
      auto cwt = catapult::Cwt::validateCwt(span, hmac);
      auto grants = grantsFromToken(cwt.payload);
      if (grants.expiresAt <= std::chrono::system_clock::now()) {
        return folly::makeUnexpected(AuthError::Expired);
      }
      return grants;
    } catch (const catapult::CryptoError&) {
      return folly::makeUnexpected(AuthError::BadSignature);
    } catch (const catapult::CatError&) {
      return folly::makeUnexpected(AuthError::Malformed);
    }
  };

  auto header = catapult::Cwt::decodeHeader(span);
  if (header.kid.has_value()) {
    auto it = hmacKeyMap_.find(*header.kid);
    if (it == hmacKeyMap_.end()) {
      return folly::makeUnexpected(AuthError::BadSignature);
    }
    return tryVerify(it->second);
  }

  for (const auto* hmac : hmacKeyList_) {
    auto result = tryVerify(*hmac);
    if (result.hasValue() || result.error() != AuthError::BadSignature) {
      return result;
    }
  }
  return folly::makeUnexpected(AuthError::BadSignature);

[suhasHere]

src/auth/Auth.h line 75 at r1 (raw file):

  static std::string
  signForTest(std::string_view keyID, std::string_view secret, const Grants& grants);

The old API accepted raw CBOR bytes, enabling tests to construct deliberately
malformed payloads. The new one only produces well-formed CWTs. The tests
RejectsMalformedClaims and most of RejectsMalformedTokenEnvelope were deleted as a
result — this loses negative-path coverage.

how about we do the below

Code snippet:

 // For normal test usage:
    static std::string signForTest(std::string_view keyID, std::string_view secret, const
  Grants& grants);

    // For malformed-input tests (signs arbitrary bytes as CWT payload):
    static std::string signRawForTest(std::string_view keyID, std::string_view secret,
  std::span<const uint8_t> rawPayload);

  And alsoadd a test that verifies garbage COSE bytes return Malformed (not crash):

  // test/AuthTest.cpp — add after RejectsEmptyTokenAsMalformed
  TEST(AuthTest, RejectsGarbageBytesAsMalformedOrBadSig) {
    AuthTokenVerifier verifier(makeConfig());
    AuthToken token{.tokenType = 77, .tokenValue = "\xde\xad\xbe\xef", .alias =
  AuthToken::DontRegister};
    auto result = verifier.verify(token);
    ASSERT_TRUE(result.hasError());
    // Either Malformed (invalid COSE) or BadSignature (all keys failed) is acceptable
    EXPECT_THAT(result.error(), testing::AnyOf(AuthError::Malformed,
  AuthError::BadSignature));
  }

[suhasHere]

src/auth/Auth.cpp line 44 at r1 (raw file):

  std::vector<uint8_t> key(SHA256_DIGEST_LENGTH);
  SHA256(reinterpret_cast<const unsigned char*>(secret.data()), secret.size(), key.data());
  return key;

I have some concerns here

1. No domain separation — same secret reused elsewhere yields identical key material
2. No salt/info binding — not a proper KDF per NIST SP 800-108 or RFC 5869
3. If the input secret has less than 256 bits of entropy, SHA-256 doesn't stretch it (it's
   not a password-based KDF either)

We can consider 2 options

1. If we control key generation, then use something like
   // Best: generate a random key and store it directly — no derivation needed
   auto key = catapult::HmacSha256Algorithm::generateSecureKey();

2. My preferred wat though is to derive HKDF key properly

Code snippet:

// Fixed salt — could be all-zeros (per RFC 5869 §3.1) or a domain-specific constant.
  // Using a fixed non-secret value is fine; it just needs to be stable across
  deployments.
  constexpr std::string_view kHkdfSalt = "moqx-catapult-v1";

  // Info string binds the derived key to this specific usage.
  constexpr std::string_view kHkdfInfo = "moqx-catapult-hmac-token-verify";

  std::vector<uint8_t> deriveHmacKey(std::string_view secret) {
    std::vector<uint8_t> key(32);  // 256-bit output

    auto* ctx = EVP_PKEY_CTX_new_id(EVP_PKEY_HKDF, nullptr);
    if (!ctx) {
      throw std::runtime_error("EVP_PKEY_CTX_new_id failed");
    }
    auto guard = std::unique_ptr<EVP_PKEY_CTX, decltype(&EVP_PKEY_CTX_free)>(ctx,
  EVP_PKEY_CTX_free);

    size_t keyLen = key.size();
    if (EVP_PKEY_derive_init(ctx) <= 0 ||
        EVP_PKEY_CTX_set_hkdf_md(ctx, EVP_sha256()) <= 0 ||
        EVP_PKEY_CTX_set1_hkdf_salt(
            ctx,
            reinterpret_cast<const unsigned char*>(kHkdfSalt.data()),
            kHkdfSalt.size()) <= 0 ||
        EVP_PKEY_CTX_set1_hkdf_key(
            ctx,
            reinterpret_cast<const unsigned char*>(secret.data()),
            secret.size()) <= 0 ||
        EVP_PKEY_CTX_add1_hkdf_info(
            ctx,
            reinterpret_cast<const unsigned char*>(kHkdfInfo.data()),
            kHkdfInfo.size()) <= 0 ||
        EVP_PKEY_derive(ctx, key.data(), &keyLen) <= 0) {
      throw std::runtime_error("HKDF derivation failed");
    }
    key.resize(keyLen);
    return key;
  }

[suhasHere]

The catapult API usage is correct (CwtMode::MACed, exception hierarchy, MoqtClaims/BinaryMatch factories all look right). Main concern is the loss of key-ID-based lookup in the verify path — lets please restore that before merge since it's both a perf and security regression. Also the multi-rule truncation should at least be loudly documented. Otherwise looks good to land.

@suhasHere made 2 comments and resolved 1 discussion.
Reviewable status: 0 of 7 files reviewed, 4 unresolved discussions (waiting on afrind, akash-a-n, gmarzot, michalhosna, mondain, Oxyd, peterchave, and TimEvens).

---

src/auth/Auth.cpp line 58 at r1 (raw file):

Previously, suhasHere (Suhas Nandakumar) wrote…

The old allRulesMatch iterated all rules with std::all_of. If any code path
(config, future callers) builds a scope with multiple rules (Prefix + Suffix to express
a range), the serialization quietly drops the extras

how about

catapult::MoqtBinaryMatch toCatapultMatch(const std::vector& rules) {
if (rules.empty()) {
return catapult::MoqtBinaryMatch::any();
}
if (rules.size() > 1) {
XLOG(WARN, "CWT supports single match rule per scope; dropping {} extra rules",
rules.size() - 1);
}
const auto& rule = rules.front();
// ...

[mondain]

src/auth/Auth.cpp line 188 at r1

[AI] Restructured verify() along these lines in f724e37, with one constraint: this Catapult version has no Cwt::decodeHeader (or any other way to read the CWT kid without first validating against a key), so the if (header.kid) short-circuit isn't possible yet. What I did instead: derive each configured key once in the AuthTokenVerifier constructor and reuse it, so the per-key derivation no longer repeats on every verify(); verify() then trial-verifies against the cached keys (same shape -- continue on CryptoError, propagate the first non-BadSignature). That removes the per-call key-derivation cost. The remaining piece (verify only the matching key) needs a kid-peek API in Catapult -- raised that on the main review thread.

[mondain]

src/auth/Auth.cpp line 188 at r1

[AI] Agreed on hoisting key setup out of verify(); implemented in f724e37, slightly adapted. I cache the derived key bytes per configured key in the constructor (a small DerivedKey{id, key} vector) rather than an unordered_map<string, HmacSha256Algorithm>, because the kid-keyed map only pays off if we can look up by kid -- and Catapult currently has no way to read the token's kid without validating first (no Cwt::decodeHeader). So verify() trial-verifies over the cached keys; constructing HmacSha256Algorithm from cached bytes is cheap (no re-derivation). I deliberately kept the id on DerivedKey, so if/when Catapult grows a kid-peek API, switching to a kid-keyed map plus the matching-key short-circuit is a localized change here.

[mondain]

src/auth/Auth.cpp line 58 at r1

[AI] Done in f724e37 -- toCatapultMatch now XLOG(WARN)s when rules.size() > 1, reporting how many rules were dropped and that the serialized grant will be broader than the configured rules. (The read path fromCatapultMatch can only ever return a single rule since a Catapult MoqtBinaryMatch is one match per dimension, so this serialization step is the only place truncation happens.)

[mondain]

src/auth/Auth.cpp line 44 at r1

[AI] Went with your preferred option 2 (HKDF) in f724e37. deriveHmacKey now uses HKDF-SHA256 via EVP_PKEY_HKDF with salt "moqx-catapult-v1" and info "moqx-catapult-hmac-token-verify" for domain separation, deriving a 256-bit key. Sign (signForTest) and verify both go through it, so it's self-consistent and there's no token-compat break (pre-release). I kept your entropy caveat as a comment -- HKDF doesn't stretch a low-entropy secret, so operators still need a long random secret; I can add a minimum-secret-length check at config load as a follow-up if you'd like. (Didn't use generateSecureKey() since our keys come from operator-provided config secrets rather than being generated by us.)

[mondain]

src/auth/Auth.h line 75 at r1

[AI] Added the garbage-bytes test (RejectsGarbageBytesAsMalformedOrBadSig) in f724e37 -- it feeds \xde\xad\xbe\xef and asserts Malformed-or-BadSignature, restoring the "invalid COSE bytes don't crash" coverage.

On signRawForTest(span<const uint8_t> rawPayload): I couldn't implement it cleanly against this Catapult version. Cwt's only construction path is Cwt(alg, CatToken), and createCwt serializes that CatToken via encodePayload() -- there's no entry point to MAC arbitrary raw bytes as the payload, so deliberately-malformed-but-decodable payloads can't be produced through the public API. The existing structural negatives (BadSignature via bit-flip, empty, wrong type, expired) plus the new garbage-bytes test cover the reachable negative paths. If you want raw-payload signing for tests, it'd need a small Catapult-side helper (e.g. createCwtFromRawPayload); happy to file that.

[mondain]

Reviewable

[AI] Addressed the r1 comments in f724e37:

• Key setup hoisted into the AuthTokenVerifier constructor (each key derived once; verify() reuses cached key material), removing the per-call derivation cost.
• HKDF-SHA256 key derivation with domain-separation salt/info (the deriveHmacKey concern).
• Loud XLOG(WARN) on multi-rule scope truncation in toCatapultMatch.
• Restored a negative-path test (garbage bytes -> Malformed/BadSignature).

One flag on the key-ID-lookup must-fix: this Catapult version exposes no way to read the CWT kid without first validating against a key (no Cwt::decodeHeader / header-peek), so the "verify only the matching key" short-circuit isn't implementable yet. verify() still trial-verifies, which is cryptographically sound (the kid is advisory -- a token only validates if some configured key's HMAC matches). The actual perf regression (re-deriving keys on every call) is fixed; the remaining optimization needs a kid-peek on the Catapult side. Since you're close to catapult: would a small Cwt::decodeHeader(span) returning the unprotected header (alg/kid/typ) be reasonable to add? I've kept the key id alongside each cached key, so switching to a kid-keyed map + short-circuit would be localized here once that lands.

[mondain]

@copilot resolve the merge conflicts in this pull request

[Copilot]

@copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in d490534.

• CMakeLists.txt: kept both the catapult add_subdirectory block and the moqx_cache library target from feature/auth.
• src/auth/Auth.cpp: kept all catapult implementations (HKDF, fromCatapultMatch, grantsFromToken, etc.); discarded the old CBOR/HMAC helpers (hmacSha256, readU32BE, appendU32BE, parseAction, parseScope, parseClaims) that no longer apply.
• src/auth/Auth.h: kept catapult-branch comment for signForTest.
• test/AuthTest.cpp: removed RejectsDuplicateWellKnownClaimKeys (auto-merged in from feature/auth but references CBOR helper functions not present in this branch and calls makeToken with a std::string incompatible with the catapult Grants-based signature; duplicate-claim detection is now Catapult's responsibility).

[suhasHere]

src/auth/Auth.cpp line 231 at r3 (raw file):

  // yet -- see the PR discussion.
  const auto tokenBytes = toBytes(token.tokenValue);
  const auto span = std::span<const uint8_t>(tokenBytes.data(), tokenBytes.size());

Catapult latest main has following api

Code snippet:

// include/catapult/cwt.hpp
  static CwtHeader decodeHeader(std::span<const uint8_t> cwtBytes);

 // It returns:
  struct CwtHeader {
    int64_t alg;
    std::optional<std::string> kid; 
    std::optional<std::string> typ;
  };
  
  //and update the existing code to
  auto header = catapult::Cwt::decodeHeader(span);
  if (header.kid.has_value()) {
    // find matching key by kid, verify only with that key
  } else {
    // no kid — fall back to trial verification
  }

  //This eliminates the trial-verify loop for tokens that carry a kid, which is both a performance and security
  //improvement (this is similar to earlier psuedocode, but you get the idea).

[suhasHere]

src/auth/Auth.cpp line 58 at r1 (raw file):

Previously, mondain (Paul Gregoire) wrote…

[AI] Done in f724e37 -- toCatapultMatch now XLOG(WARN)s when rules.size() > 1, reporting how many rules were dropped and that the serialized grant will be broader than the configured rules. (The read path fromCatapultMatch can only ever return a single rule since a Catapult MoqtBinaryMatch is one match per dimension, so this serialization step is the only place truncation happens.)

Thanks @mondain for being patient on the review here. Since this is a good functionality to be added in catapult , i went ahead and add it over the weekend ( and merged this morning )
On the latest main , please see https://github.com/Quicr/catapult/blob/main/docs/moqt_scope_design.md for CompoundMatch support.

Given that, the above code will have some changes along the lines below. Please give it a try.

Code snippet:

catapult::MoqtCompoundMatch toCatapultMatch(const std::vector<MatchRule>& rules) {
    if (rules.empty()) return catapult::MoqtCompoundMatch::any();
    std::vector<catapult::MoqtBinaryMatch> conditions;
    conditions.reserve(rules.size());
    for (const auto& rule : rules) {
      switch (rule.type) {
        case MatchRule::Type::Exact:    conditions.push_back(catapult::MoqtBinaryMatch::exact(rule.value));
  break;
        case MatchRule::Type::Prefix:   conditions.push_back(catapult::MoqtBinaryMatch::prefix(rule.value));
  break;
        ///// so on
      }
    }
    return catapult::MoqtCompoundMatch::all(std::move(conditions));
  }
  
  // then
    std::vector<MatchRule> fromCatapultMatch(const catapult::MoqtCompoundMatch& match) {
    if (match.is_empty()) return {};
    std::vector<MatchRule> rules;
    rules.reserve(match.conditions().size());
    for (const auto& cond : match.conditions()) {
      rules.push_back(MatchRule{
        .type = fromCatapultMatchType(cond.match_type),
        .value = toString(cond.pattern),
      });
    }
    return rules;
  }
  
  

[suhasHere]

@mondain thanks for addressing the review feedback. the latest catapult ( from this morning) should have decodeHeader and CompoundMatch support. if you can integrate these 2 into your PR, I feel we are very close calling first version of cat support done.

I am sure we can improve library API later as needed, but these give us sufficient functionality to test it

@suhasHere made 2 comments.
Reviewable status: 0 of 7 files reviewed, 5 unresolved discussions (waiting on afrind, akash-a-n, gmarzot, michalhosna, mondain, Oxyd, peterchave, and TimEvens).

---

src/auth/Auth.cpp line 106 at r3 (raw file):

  // MatchRule vector can express several ANDed rules (e.g. Prefix + Suffix). We
  // can only serialize the first; warn loudly rather than silently dropping the
  // rest, since that would widen the grant beyond what was configured.

this is true since the idea was to use OR across scopes. But if we feel the functionality is important, i can add it

[mondain]

src/auth/Auth.cpp line 188 at r1

[AI] Done in f382dc9. Cwt::decodeHeader is now available in the updated catapult (7e8186e) and is used in verify().

The new flow:

1. Call Cwt::decodeHeader(span) to peek the COSE protected header without any crypto. If it throws CatError, return Malformed immediately.
2. If header.kid.has_value(), look up in keyIdIndex_ (built in the constructor: std::unordered_map<std::string, std::size_t> mapping kid -> index in derivedKeys_). Unknown kid -> BadSignature immediately; known kid -> verify with only that key.
3. No kid -- trial-verify against all configured keys.

keyIdIndex_ is populated in the constructor for any configured key with a non-empty id.

[mondain]

src/auth/Auth.h line 75 at r1

[AI] Attempted to file on Quicr/catapult but my token lacks write access to that repo. @suhasHere -- could you file it? The ask is a createCwtFromRawPayload(span<const uint8_t>) helper (possibly in a test_utils.hpp) so consumers can produce validly-signed COSE_Mac0 envelopes with deliberately malformed payloads for negative-path testing. The decodeHeader API added in the recent PR complements it nicely: together they allow tests to round-trip malformed payloads through a full COSE envelope.

[mondain]

src/auth/Auth.cpp line 231 at r3

[AI] Done in f382dc9. verify() now calls Cwt::decodeHeader(span) before any HMAC work, exactly as suggested. If header.kid is set, keyIdIndex_.find(*tokenKid) routes to the matching derivedKeys_ entry -- unknown kid returns BadSignature immediately without touching any other key. Tokens with no kid fall through to the trial-verify loop.

[mondain]

src/auth/Auth.cpp line 106 at r3

[AI] Updated in f382dc9. The warning comment about single-rule truncation is removed -- toCatapultMatch now iterates all rules into MoqtCompoundMatch::all(conditions), so all conditions are serialized. Scope semantics remain OR-across-scopes / AND-within-a-scope's-conditions, matching the catapult CompoundMatch design.

[mondain]

Reviewable

[AI] Done in f382dc9. Both items are integrated from catapult 7e8186e:

• CompoundMatch: toCatapultMatch returns MoqtCompoundMatch::all(conditions) iterating all MatchRules; fromCatapultMatch takes MoqtCompoundMatch and iterates all conditions. Full round-trip fidelity, no more truncation.
• decodeHeader: verify() peeks the COSE header first (no crypto) to extract kid, routes to the matching key via keyIdIndex_ in O(1), falls back to trial-verify only for tokens with no kid.

Added MultiRuleCompoundMatchRoundtripsViaCwt test for the compound match path. Only open item is the catapult-side createCwtFromRawPayload helper for malformed-payload tests -- flagged in the signRawForTest thread for @suhasHere to file.

[suhasHere]

@suhasHere made 1 comment.
Reviewable status: 0 of 7 files reviewed, 3 unresolved discussions (waiting on afrind, akash-a-n, gmarzot, michalhosna, Oxyd, peterchave, and TimEvens).

[afrind]

@afrind reviewed 7 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on akash-a-n, gmarzot, michalhosna, Oxyd, peterchave, suhasHere, and TimEvens).

---

src/auth/Auth.cpp line 37 at r5 (raw file):

  std::string out;
  for (const auto& field : ns.trackNamespace) {
    out.push_back(static_cast<char>((field.size() >> 24) & 0xff));

This seems gratuitous -- previous impl folly::Endian::big or ntohl would be preferable

[afrind]

Builds are broken in CI, and ASAN tests failed for me locally.

Reminder: devs should always use --profile san for daily development

[afrind]

Yes — strictly leaks (plus one UBSan finding), and all of them originate inside the
catapult/libcbor dependency, not in moqx's auth code.

Evidence from running AuthTest.VerifiesSignedTokenAndAllowsMatchingAction under ASan:

[ OK ] AuthTest.VerifiesSignedTokenAndAllowsMatchingAction (5 ms)
[ PASSED ] 1 test.

==462272==ERROR: LeakSanitizer: detected memory leaks
...
SUMMARY: AddressSanitizer: 2119 byte(s) leaked in 59 allocation(s).

So the test logic passes ([ PASSED ]) — every one of the 8 "failures" is the sanitizer tripping
at teardown, not an assertion failing.

Where the leaks are: every leak's allocation frame is inside deps/catapult and its bundled
external/libcbor, reached through catapult's CWT API:

• cbor_build_string / cbor_build_bytestring / cbor_new_int64 / cbor_new_definite_map →
  catapult::Cwt::createCwt / validateCwt / ClaimProcessor::addClaimRaw / SigStructureBuilder::build
• moqx only appears as the caller (Auth.cpp:284 signForTest, Auth.cpp:245/263 verify) — it's not
  allocating the leaked cbor_item_t objects itself. catapult creates them during token
  encode/decode and never frees them.

One extra, non-leak finding (UBSan, also in libcbor):
deps/catapult/external/libcbor/src/cbor/bytestrings.c:66:3: runtime error:
null pointer passed as argument 2, which is declared to never be null
That's libcbor passing nullptr into memcpy for a zero-length bytestring — benign in practice but
UBSan flags it.

Bottom line for the authors: the moqx auth integration is functionally correct under sanitizers;
the failures are leaked CBOR items owned by catapult's Cwt/MoqtClaims builders (likely missing
cbor_decref/ownership transfer in catapult itself, or an API-usage contract where the caller must
release something). The fix belongs in deps/catapult, not in src/auth/. Want me to drop a
summary comment on PR #286 capturing this so they don't have to re-derive it?

[afrind]

Also CC: @gmarzot -- do we need to setup autosync of the catapult repo once we take that dep?

[gmarzot]

Also CC: @gmarzot -- do we need to setup autosync of the catapult repo once we take that dep?

My impression is no catapult autosync automation in moqx ci is needed .. unless we think essential changes are flowing in all the time or that we may want local patches? @suhasHere we can do manual pr when it needs to be updated right

[suhasHere]

Also CC: @gmarzot -- do we need to setup autosync of the catapult repo once we take that dep?

My impression is no catapult autosync automation in moqx ci is needed .. unless we think essential changes are flowing in all the time or that we may want local patches? @suhasHere we can do manual pr when it needs to be updated right

@gmarzot Yes that is totally possible and I agree with your approach

[mondain]

@afrind @suhasHere @gmarzot so what is the next step, I thought this was good to go?

@mondain made 1 comment.
Reviewable status: 6 of 9 files reviewed, 3 unresolved discussions (waiting on afrind, akash-a-n, gmarzot, michalhosna, Oxyd, peterchave, suhasHere, and TimEvens).

[suhasHere]

+1 to land it

[mondain]

@suhasHere this appears to be blocked by 3 blocking replies tagged to your name

@mondain made 1 comment.
Reviewable status: 3 of 9 files reviewed, 3 unresolved discussions (waiting on afrind, akash-a-n, gmarzot, michalhosna, Oxyd, peterchave, suhasHere, and TimEvens).

[suhasHere]

@suhasHere reviewed 8 files and all commit messages, and resolved 2 discussions.
Reviewable status: 8 of 9 files reviewed, 1 unresolved discussion (waiting on afrind, akash-a-n, gmarzot, michalhosna, Oxyd, peterchave, and TimEvens).

[suhasHere]

@suhasHere reviewed 1 file.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on afrind, akash-a-n, gmarzot, michalhosna, Oxyd, peterchave, and TimEvens).

[afrind]

Pending discussion

@afrind made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on akash-a-n, gmarzot, michalhosna, Oxyd, peterchave, suhasHere, and TimEvens).

[suhasHere]

@afrind Per slack discussion, do you think we are good to move forward here ? Thanks