Repository navigation
feat!: audit rind's public API, and apply its decisions, before 0.2.0 - #383
Conversation
Quest-Claim: 5bc8fde9abfd8da4
Fixes the docs the audit found stale: agreement keys are no longer only ephemeral, ChaCha20::new and rind-rustls's Ticketer say what they do, and MIGRATING.md no longer claims every adapter function refuses an unsupported CPU (rind-rcgen's KeyPair returns rcgen's errors) and names agreement::Scalar. The quest now records what remains: the maintainer's picks on the PR's decisions, and the fixes they choose. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Audit inventory 1 of 2:
|
Audit inventory 2 of 2: the adapters, and the scriptrind-ring is generated with rind-rustlsrind-rcgenrind-dimplrind-jsonwebtokenrind-object-storerind-ringinventory.pyimport json, sys
def first_line(docs):
if not docs:
return "<NO DOC>"
for l in docs.strip().splitlines():
l = l.strip()
if l:
return l
return "<NO DOC>"
def fn_sig(it, idx):
f = it["inner"]["function"]
s = f["sig"]
args = []
for name, ty in s["inputs"]:
args.append(name + ": " + ty_str(ty))
out = ""
if s.get("output") is not None:
out = " -> " + ty_str(s["output"])
return "(" + ", ".join(args) + ")" + out
def ty_str(t):
if t is None: return "()"
if isinstance(t, str): return t
k, v = next(iter(t.items()))
if k == "primitive": return v
if k == "generic": return v
if k == "resolved_path":
name = v["path"]
args = v.get("args")
a = ""
if args and "angle_bracketed" in args:
parts = []
for x in args["angle_bracketed"]["args"]:
if "type" in x: parts.append(ty_str(x["type"]))
elif "lifetime" in x: parts.append(x["lifetime"])
elif "const" in x: parts.append(str(x["const"].get("expr")))
if parts: a = "<" + ", ".join(parts) + ">"
return name + a
if k == "borrowed_ref":
return "&" + (v.get("lifetime") + " " if v.get("lifetime") else "") + ("mut " if v["is_mutable"] else "") + ty_str(v["type"])
if k == "slice": return "[" + ty_str(v) + "]"
if k == "array": return "[" + ty_str(v["type"]) + "; " + v["len"] + "]"
if k == "tuple": return "(" + ", ".join(ty_str(x) for x in v) + ")"
if k == "impl_trait":
return "impl " + " + ".join(b.get("trait_bound", {}).get("trait", {}).get("path", "?") for b in v)
if k == "qualified_path": return "<..>::" + v["name"]
if k == "dyn_trait": return "dyn " + " + ".join(x["trait"]["path"] for x in v["traits"])
if k == "raw_pointer": return "*" + ("mut " if v["is_mutable"] else "const ") + ty_str(v["type"])
if k == "function_pointer": return "fn(..)"
return k
def main(path, crate):
d = json.load(open(path))
idx = d["index"]
root = d["root"]
seen = set()
out = []
def item_of(id_):
return idx.get(str(id_))
def walk(mid, prefix):
m = item_of(mid)
for cid in m["inner"]["module"]["items"]:
it = item_of(cid)
if it is None: continue
emit(it, prefix)
def emit(it, prefix, alias=None):
inner = it["inner"]
kind = next(iter(inner))
if kind == "use":
u = inner["use"]
target = item_of(u["id"]) if u.get("id") is not None else None
if target is None:
out.append(f"{prefix}::{u['name']} [external re-export of {u['source']}]")
return
if u["is_glob"]:
if "module" in target["inner"]:
walk(target["id"], prefix)
return
emit(target, prefix, alias=u["name"])
return
name = alias or it["name"]
if it.get("visibility") != "public" and kind != "use":
return
if kind == "impl":
return
p = f"{prefix}::{name}"
doc = first_line(it.get("docs"))
if kind == "module":
out.append(f"mod {p} -- {doc}")
walk(it["id"], p)
return
sig = ""
if kind == "function":
sig = fn_sig(it, idx)
if kind == "constant" or kind == "static":
t = inner[kind].get("type")
sig = ": " + ty_str(t)
out.append(f"{kind} {p}{sig} -- {doc}")
if kind in ("struct", "enum", "union", "trait"):
key = (it["id"])
if key in seen:
out.append(f" (see above)")
return
seen.add(key)
if kind == "enum":
for vid in inner["enum"]["variants"]:
v = item_of(vid)
out.append(f" variant {v['name']} -- {first_line(v.get('docs'))}")
if kind == "struct":
sk = inner["struct"]["kind"]
if isinstance(sk, dict) and "plain" in sk:
for fid in sk["plain"]["fields"]:
f = item_of(fid)
if f and f.get("visibility") == "public":
out.append(f" field {f['name']}: {ty_str(f['inner']['struct_field'])} -- {first_line(f.get('docs'))}")
elif isinstance(sk, dict) and "tuple" in sk:
for fid in sk["tuple"]:
if fid is None: continue
f = item_of(fid)
if f and f.get("visibility") == "public":
out.append(f" pub tuple field: {ty_str(f['inner']['struct_field'])}")
if kind == "trait":
for mid_ in inner["trait"]["items"]:
m = item_of(mid_)
mk = next(iter(m["inner"]))
s = fn_sig(m, idx) if mk == "function" else ""
out.append(f" trait-item {mk} {m['name']}{s} -- {first_line(m.get('docs'))}")
impls = inner[kind].get("impls", []) if kind != "trait" else []
traits = []
for iid in impls:
im = item_of(iid)
if im is None: continue
ii = im["inner"]["impl"]
if ii.get("is_synthetic") or ii.get("blanket_impl"):
continue
if ii["trait"] is None:
for mid_ in ii["items"]:
m = item_of(mid_)
if m.get("visibility") != "public": continue
mk = next(iter(m["inner"]))
s = fn_sig(m, idx) if mk == "function" else (": " + ty_str(m["inner"][mk].get("type")) if mk == "constant" else "")
out.append(f" {mk} {m['name']}{s} -- {first_line(m.get('docs'))}")
else:
tn = ty_str({"resolved_path": ii["trait"]})
traits.append(tn)
if traits:
out.append(" impls: " + ", ".join(sorted(traits)))
walk(root, crate)
print("\n".join(out))
main(sys.argv[1], sys.argv[2])(Written by Claude Opus 5.5) |
|
The maintainer picked the decisions on 2026-11-05:
All of them land in this PR. (Written by Claude Opus 5.5) |
The maintainer's picks on the audit (#383): - `signature::{Pkcs8, Spki}` move to a new `encoding` module. - `signature::RsaKeySize` is `rsa::KeySize`, and `signature::RsaPublicKeyComponents` is `rsa::PublicKeyComponents`. - `components.to_public_key(alg)` is `PublicKey::from_components(alg, &components)`. - `agreement::PrivateKey::from_private_key` is `from_scalar`, and `EcdsaKeyPair::from_private_key_and_public_key` is `from_scalar_and_public_key`. - The 16 symmetric key and keyed-state types gain a `Debug` that shows the algorithm only. - `kem::DecapsulationKey::decapsulate` borrows the key. - `aead::gcm::Key` takes a `gcm::Iv`, which refuses an empty IV and is not `Clone`. - `cipher::ChaCha20::new` takes `&[u8; 32]`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # proofs/Rind/Aarch64/P256Signature.lean
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Your OpenCode Go account is out of credit. Top it up, then re-trigger Pullfrog. OpenCode Go's response
|
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 59 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head: b2981a8
- Findings: no actionable defect found in the inspected changes. The RSA constructor move retains parsing and algorithm checks, the new GCM IV wrapper validates before dispatch, and the added Debug implementations expose algorithm metadata rather than key material.
- Direction: the changes match the recorded API decisions and are appropriately grouped before 0.2.0. No architectural revision requested. A small non-blocking test improvement would exercise multiple decapsulations, including malformed-then-valid input, on one retained ML-KEM key to pin the newly reusable contract.
- Verification limits: GitHub-only static review of the 58-file diff and relevant implementation/test context. I did not run tests, cross-target builds, benchmarks, or Lean proofs; the PR's reported local passes are not independently verified. The head's visible
proverun is still in progress and the returned Queue runs are skipped, so this is not a CI-pass claim.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head: 395db17
Incremental review since the previous review at b2981a8, focused on integrating #382 with the API audit.
- Findings: no actionable defect found in this integration delta.
rind/src/lib.rsmoves the newly mergedSec1export intoencodingalongsidePkcs8andSpki; its existing redactedDebugand zeroizingDropremain intact. The added agreement tests, crosschecks, and key-fuzzer call sites usefrom_scalar, while the merged key-format constructors and exporters retain their implementations. README and MIGRATING.md describe the new SEC1 location. - Direction: extending the recorded encoding-module decision to SEC1 is consistent with the audited API and belongs in this pre-0.2.0 break. No additional API or architectural revision identified.
- Verification limits: GitHub-only static comparison of the old and current PR diffs, merge history, and relevant key-format implementation/test context. This is not a fresh audit of the unrelated SHA changes merged from main. I did not run builds, tests, fuzzers, benchmarks, or Lean proofs, and the PR's reported local passes are not independently verified. The visible
proverun for this head is in progress and the returned Queue runs are skipped; this is not a CI-pass claim.

Works on quest/m1/api-audit.md: audit every public item of
rindand its published adapters against AGENTS.md's API rules before 0.2.0, which release-plz's moq-dev/rind#367 and the Bun PR wait on.Problem
0.2.0 commits rind to its API under semver. Every breaking fix has to land before it, and each finding needs either a fix or a recorded maintainer decision to keep it.
Approach
cargo doc's public surface"), walked every item, and read its source against AGENTS.md's API rules: misuse fails to compile, one way to do each job, names by role (aws-lc-rs's only when as clear), one-line docs, wiped secrets. I compared each against aws-lc-rs 1.19.0 and MIGRATING.md.mainat f0b7931, which includes the agreement keys from moq-dev/rind#372. I auditedagreementlast, after that PR merged.admin/provechecks, with their definitions and theorems renamed to match.Inventory
Crates audited:
rind0.1.13 (280 items: types, functions, methods, statics, variants, and fields),rind-ring0.1.12 (204),rind-rustls0.1.13 (51),rind-rcgen0.1.13 (4),rind-dimpl0.1.13,rind-jsonwebtoken0.1.13, andrind-object-store0.1.13 (1 each). Not audited:rind-instant-acme,rind-bench,rind-cpuid, andfuzz, which arepublish = false.The full per-item inventory, with signatures, trait impls, and doc lines, is in the PR comments, along with the script that regenerates it:
Mechanical checks over it:
rinditems are mapped. 40 appear only through a family row ("and so on", "their SHA-384 and SHA-512", "one ofTagLength's"), andagreement::Scalarwas named only as "to_scalar(), wiped on drop" (fixed below).aead::chacha20_poly1305_openssh,rsa::KeyPairComponentsandKeyPair::from_components,io,test_file!) and the addedcpu. Signatures match up to path spelling. Two derives differ harmlessly:error::UnspecifiedaddsEq, andhmac::Algorithmandpbkdf2::AlgorithmimplementPartialEqby hand. TheirDebuggaps are ring's own.Drop. That includescipher::Aesand the kem types, whoseDroprustdoc shows only on the inner type.tests/zeroing.rscovers them.Decisions
The maintainer picked each finding on 2026-11-05, and this PR implements every pick.
Across modules
Pkcs8andSpkilive (breaking): ✅ a newrind::encodingmodule holds them, not the crate root I recommended. Every key type that exports itself returns them, whatever its module.rsa::KeySizeandrsa::PublicKeyComponents, aws-lc-rs's names, whichsignatureuses from there.signaturekeeps no alias.signature::PublicKey::from_components(&alg, &components), a constructor on the type it builds, asrsa::EncryptingKey::from_componentsis;to_public_keyis gone.agreement::PrivateKey::from_scalarandEcdsaKeyPair::from_scalar_and_public_key, pairingto_scalaras Ed25519'sfrom_seed_and_public_keypairsto_seed. rind-ring keeps ring's names.Debugon the 16 symmetric key and state types (additive): ✅ each shows its algorithm only, never key bytes.Nonce,gcm::Iv,RandomizedNonceKey, andcipher::ChaCha20have one algorithm or none to show, so they print...kem
DecapsulationKey::decapsulateconsumes the key (nominally breaking): ✅ it borrows (&self), asagreeand aws-lc-rs's do. Still no loader.aead
gcm::Key'sivandaadare both&[u8](breaking): ✅ agcm::Iv<'a>fromIv::assume_unique_for_key(&[u8])?, which refuses an empty or over-long IV, is notClone, and is consumed per call, asNonceis.open_in_place_separate_tag(keep): ✅ kept. Its four types differ, so a misordering does not compile.cipher
cipher::ChaCha20::newtakes its key by value (breaking): ✅new(key: &[u8; 32]), asRandomizedNonceKey::newtakes it.rsa
rsa::OaepAlgorithm's name (keep): ✅ kept, aws-lc-rs's name;rsanow also holds key types, so the name saying OAEP is clearer.agreement
PrivateKey::from_sec1(&X25519, ..)compiles and always fails (keep): ✅ kept, as documented, which is aws-lc-rs's shape.rind-rcgen
KeyPair::to_pkcs8returnszeroize::Zeroizing<PrivatePkcs8KeyDer<'static>>(keep): ✅ kept; zeroize has been at 1.x since 2019.Checked and kept, with no decision needed
Each of these was checked against a rule and is kept, because the reason is recorded or upstream fixes the shape:
digest::Context::finish(&mut self)resets the context, unlike aws-lc-rs'sfinish(self): a recorded perf decision in moq-dev/rind#352.PublicKey::verify_digestandEcdsaKeyPair::sign_digestrefuse RSA, Ed25519, and non-SHA-2 digests at run time (Error::Unsupported): merged hours ago in moq-dev/rind#379.RsaKeyPair::signandpublic_keytake the algorithm per call, unlikeEcdsaKeyPair: an RSA key signs with several paddings (TLS uses PSS and PKCS#1 with one key). This is aws-lc-rs's shape, and the method docs say so.signature::sign::Xandverify::Xshare names, but their types differ, so a mix-up does not compile.hmac::Key::algorithm()is rind's onlyalgorithm()accessor; rind-rustls's HMAC needs it.key_wrap::AES_128andAES_256re-exportcipher::AES_128andAES_256: the same statics, under the module aws-lc-rs puts them in.rind_rustls::Ticketer::new()returnsArc<dyn ProducesTickets>from a unit struct, exactly as rustls's own providers do.DEFAULT_PROVIDERis a static, so it cannot check the CPU: jsonwebtoken requires a static. The docs say to callrind::cpu::check()first.Where rind differs from aws-lc-rs on purpose
MIGRATING.md maps each of these. In brief:
&'static, and no call takes a random source.legacysubmodules rather than_FOR_LEGACY_USE_ONLYnames.rind::Error, whose variant says which kind of call failed.encoding::Pkcs8,encoding::Spki,Signature, the OAEPPlaintext) have fixed capacity rather than being allocated.PublicKey::new, with noUnparsedPublicKey.SealingKey/OpeningKey/NonceSequencetypes: a singleaead::Keytakes aNonceper call.RandomizedNonceKeyis XChaCha20-Poly1305 with 192-bit nonces.hkdf::Prk::expandcannot fail, andKeyTypeis sealed.pbkdf2::Secretkeys HMAC once for every salt.digest::Context::finish(&mut self).Impact
Breaking, so the title takes
!; MIGRATING.md maps each aws-lc-rs name to the new one. Migrating from rind 0.1:rind::signature::{Pkcs8, Spki}becomerind::encoding::{Pkcs8, Spki}..as_ref()callers are unaffected.rind::signature::Sec1, returned byEcdsaKeyPair::to_sec1, becomesrind::encoding::Sec1. This applies D1's rule (every encoded key lives inencoding) to the new type; it is an addition toencoding, not a rename of anencodingitem.signature::PublicKey::{from_spki, to_spki},Ed25519KeyPair::{from_seed, to_pkcs8v1}, andagreement::PublicKey::{new, from_spki, to_spki}keep their names, which already follow the audit's rules (from_seedpairsto_seed), and returnencoding::Spkiandencoding::Pkcs8.agreement::PrivateKey::from_scalar, and its docs use the audited names. README'sencodingline now names SEC1.rind::signature::RsaKeySize::Rsa2048becomesrind::rsa::KeySize::Rsa2048, andrind::signature::RsaPublicKeyComponentsbecomesrind::rsa::PublicKeyComponents.components.to_public_key(&alg)?becomessignature::PublicKey::from_components(&alg, &components)?.agreement::PrivateKey::from_private_key(&alg, scalar)becomesfrom_scalar(&alg, scalar), andEcdsaKeyPair::from_private_key_and_public_key(&alg, scalar, point)becomesfrom_scalar_and_public_key(&alg, scalar, point).key.decapsulate(ct)compiles unchanged and now leaves the key usable; only UFCS and storedfnpointers change.gcm_key.seal_in_place_append_tag(&iv, aad, buf)becomesgcm_key.seal_in_place_append_tag(gcm::Iv::assume_unique_for_key(&iv)?, aad, buf), and the same for the other threegcm::Keymethods. An empty IV now fails atIv, with the sameError::WrongLength.cipher::ChaCha20::new(key)becomesChaCha20::new(&key).Debugondigest::Context,hmac::{Key, Context},hkdf::{Salt, Prk, Okm},pbkdf2::Secret,aead::{Key, Nonce, RandomizedNonceKey},aead::gcm::{Key, Iv},aead::quic::HeaderProtectionKey,cipher::{Aes, AesCtr, ChaCha20}, andkey_wrap::AesKek.In this workspace, rind-ring and rind-jsonwebtoken change internally, with no change to their own APIs. The docs fixes from the first commit:
rind::agreement's module summary no longer says "Ephemeral", since feat(agreement)!: store, load, and export agreement keys, agreeing by reference #372 made its keys storable and reusable.TicketerandTicketer::neware documented by what they do, not "The default ticketer".rind_rcgen::KeyPair's constructors return rcgen's errors, which cannot carry one. It also namesagreement::Scalar.Performance: these are API moves, and the codegen shows it. Built with one codegen unit at the base and at this head, every function in rind's release rlib is the same size except the renamed loaders (same sizes under new names), the new
Debugimpls (44 to 202 bytes each),decapsulate, 343 bytes smaller now that it no longer drops the key, and ChaCha20 QUIC header-protection key setup, 22 bytes larger.gcm::Key's seal and open,Ivincluded, are byte for byte the same size. Therind-benchML-KEM decapsulate bench now reuses one key, as aws-lc-rs's does, rather than timing a fresh key's drop.Alternatives
cargo docpublishes, re-exports included, so the inventory is reproducible.Follow-ups
agreement::PrivateKey's existingDebugprintsPrivateKey { .. }, without its algorithm as the D5 types show theirs. It is additive, so it can ship in 0.2.x.Validation
just checkon b2981a8: passes, 612 tests, including the armv7 and cross legs, which caught armv7's AES-GCM key missingkey_len.nix develop .#prove --command admin/proveon the merged head: every quote matches and the Lean build completes (2454 jobs).quest check --base origin/main: passes (68 documents).rind/tests/debug.rspins each D5 type'sDebugoutput and checks that no key byte shows.CARGO_BUILD_JOBS=3 just checkpasses (623 tests),nix develop .#prove --command admin/provepasses (2454 jobs, every quote matches), andadmin/bench-report render --checkreportsBENCHMARKS.mdcurrent.Debugalready showed the algorithm only; they now sharecipher::aesfor it instead of a copy of that helper.Review
Codex (gpt-6.1-sol, medium effort) reviewed head 655d0f3, the hand-resolved merge of #382, against origin/main 8747f62 with
quest review: round 1, no findings. 395db17 adds only a conflict-free merge of origin/main (#376, #387, #389) on top.🤖 Generated with Claude Code
(Written by Claude Opus 5.5)