Conversation
session-deps is a new repo that consolidates how we handle loading and doing static bundle builds of various common external dependencies across Session projects.
Carrying a libsodium fork is too much of a nuissance as updating it to the latest version is non-trivial. This drops the libsodium-internal fork in favour of using tweenacl's implementation *just* for the X25519 -> Ed25519 pubkey conversion, and using a stock libsodium for everything else. This also bumps the libsodium requirement up to 1.0.21: that version will be required for SHAKE support in future commits on this branch.
- Blake2b hashing:
- Add nicer hash::blake2b functions for simpler has computations where
you can just pass a bunch of spannables and get the hash over them,
rather than needing to do a bunch of manual C API update calls.
- Add a ""_b2b_pers for a compile-time validated blake2b
personalisation string
- Drop make_blake2b32_hasher().
- TODO: convert these to make full use of hash::blake2b(...), as
above.
- Change cleared_array to take a Char type instead of forcing unsigned
char.
- Add cleared_uchars (and cleared_bytes) for the old force-unsigned char
typedef.
- Make `to_span` work for any input convertible to a span
- Tighten up various functions taking fixed length values (like session
ids, pubkeys) to take compile-time-fixed spans instead of dynamic
extent spans. This allows these generic functions to not have to
worry about length checking.
- Add a generic random::fill(s) function that fills some spannable type
s with random bytes.
This switches to the PR branches for session-router and libquic to use session-deps. (This was required under ninja builds, in particular, to get the deduplication handling for gmp and nettle via new session-deps code to deal with that).
- Refactor manual libsodium blake2b hash calls to use simpler hash::blake2b functions instead. - Unify usage of array_uc32/33/64 and uc32/33/64: now we have just uc32/33/64 and cleared_uc32/33/64 (i.e. the "array_" prefix is gone). - Add a unsigned char array literal, which is quite useful for static hash keys.
- Including raw integer bytes would break the hashes on a non-little endian arches; this adds a helper than ensures we are always hashing the little-endian value. - Make the remaining manual hash use blake2b_pers. This didn't get autoconverted before because the `if` around part of the hash, but that if is actually completely unnecessary: if the value is empty, a blake2b hash update does nothing, and so it can just be always included (and when empty, it is still the right thing).
oxenc has buggy "constexpr" overloads that just break if invoked, and they get invoked here with a uint8_t value + unsigned char array. The issue is fixed in oxenc dev, but switching to a byte here works around it for current and older oxenc versions.
The recent commit to unify the types wasn't applied to the test suite.
hash::blake2b (and related) now take integers directly, writing the integer bytes (with byte swapping applied, if necessary), which further simplies the hashing API.
Make way for account keys to be in here as well.
If two devices rotate account keys at approximately the same time, both might end up with inconsistent "active" keys. This commit adds deterministic tie breaking (always prefering the later, with fallback to seed ordering).
Adds a "needs push" concept, along with more tracking fields to let us distinguish between various possible states.
`session::AdjustedClock` now carries an adjustable static offset and when `now()` is called it returns standard system clock timepoints with the adjustment applied. The networking code had a similar adjustment already, although it was per-network-object: this change replaces that, and makes it now global across all uses of the clock anywhere, instead of per-Network instance.
pro_renewal_target returned nullopt ("never (re)fetch") whenever there was
no proof credential unless a prepaid purchase marker was set. But `s` (the
credential) and `E` (the access expiry) are independent config keys, so an
account can be genuinely entitled -- `E` still in the future -- while
holding no proof (e.g. `s` was dropped or merge-lost). That state should
fetch a proof, not sit idle forever. Return `now` when the access expiry is
still in the future and there's no proof.
Also rename the local `pro` -> `pro_config`: it is the full ProConfig
credential (rotating key + proof), not a boolean "are we pro", and the old
name misled at least one reader into misdiagnosing this very path.
Relies on the client keeping `E` synced to the backend's reported horizon
and clearing it on not_subscribed (E does not self-age); communicated
separately to the clients.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(PFS) pro_backend: throw on parse errors instead of an error vector
(PFS) user_profile: fetch a proof when entitled but holding none
Forward-port of #120 (dev commit b63b4b0) onto pfs; applies unmodified. GetProRevocationsCResponse / GetPaymentDetailsCResponse are aggregates built by the *_parse functions via std::make_unique from a parsed base response. make_unique initializes with parentheses, which only aggregate-initializes under P0960R3 (C++20): GCC implements it, the Apple Clang on the macOS CI runners does not, so it looks for a constructor and finds none. Give each an explicit base-slice constructor; call sites are unchanged. pfs has the same structs and the same make_unique call sites, so it has the same latent macOS break.
Forward-port of #114 (dev commit 89c6acb) onto pfs. Most of #114 is already here: the byte refactor (7662405) had independently given the binary API fixed-extent spans, so sign/verify/pubkey already reject wrong sizes at compile time, the string overloads already validate, and the blinding / group-keys call sites already pass sized views. Two things were missing. First, pfs's verify(string_view) returned false for a wrong-sized signature or pubkey where #114 throws std::invalid_argument, so it conflated "you passed a malformed argument" with "this signature does not verify". Align with dev: letting pfs keep return-false would silently revert #114's behaviour when pfs eventually lands on dev. Nothing in the tree calls the string overloads -- every call site uses the span overloads -- and the return-false came in incidentally with 7662405 rather than as a considered choice. Second, port #114's regression test for the rejected sizes. The three string overloads all validated a length and then narrowed to a fixed span; that is now one require_bytes<N> helper, which also gives the arguments #114's exception wording.
Records dev up to 2b27d27 as merged, keeping pfs's tree unchanged (-s ours). All four PRs in the range are now applied to pfs: - #116 (this-is-not-c) via #115 this-is-not-c-pfs - #118 (renewal-target-no-proof-fetch) via #119 renewal-target-no-proof-fetch-pfs - #120 (macOS C response holders) forward-ported in e682e6b - #114 (fixed-size XEd25519 spans) forward-ported in b03709d Verified for the two that were already applied: #119's user_profile.cpp change is line-identical to #118's, and for #116/#115 the `errs` error-vector is gone from both branches with identical parse_error usage.
Clients sometimes need to know whether a Pro subscription is terminal or auto-renewing (e.g. "renews on X" vs "expires on X"). Store the backend's `auto_renewing` (from get_pro_status) as a presence-only config flag `A`: 1 when auto-renewing, absent otherwise (terminal / unknown / not Pro). Deliberately not tri-state: unlike blinded_msgreqs `M`, this is backend- derived fact, not a defaulted client preference, so there's no upgrade- default edge case that a distinct "unset" would guard. And no t/T bump -- it's synced pro state like E/I/R, not a user-initiated profile edit. Exposes get_/set_pro_auto_renewing (C++ bool; C 0/1) with unit + C-API coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Forward-port of #125 (dev commit d9406eb) onto pfs, where the same code was present verbatim. encrypt_for_multiple_simple's decoy padding was sized from the plaintext rather than the ciphertext, so a padding entry was encrypt_multiple_message_overhead bytes shorter than every real entry and could be picked out by length alone -- defeating the point of padding the list to a fixed count. The `if (int pad_size = ...)` form also made an empty first message pad to zero, i.e. falsy, which skipped the padding loop entirely: a list of empty messages got no decoys at all. Test both, the sizes and the count.
pfs has broken a fair bit of the API (the std::byte span refactor, the crypto header reorganization, the C API changes), and together with core and the coming Client code this is a substantially different release, so it takes a major bump rather than following dev's 1.x line. This deliberately diverges from dev, which is at 1.8.0 (#128): pfs has been on its own version line since before 1.7.0, and the successive `-s ours` merges have been quietly masking dev's bumps. Setting 2.0.0 makes pfs's version unambiguously ahead, so a future merge into dev carries the version forwards rather than dragging it back.
I kept dev's line wrapping when porting #125, but pfs's encryption::XCHACHA20_ABYTES is shorter than dev's crypto_aead_xchacha20poly1305_ietf_ABYTES, so the CHECK now fits on one line and clang-format joins it. Missed because I did not run utils/format.sh before committing.
Forward-port of #126 (dev commit 4143882) onto pfs; the five files dev touches are line-for-line identical to dev's change. A 421 means the account we asked about is not in the queried node's swarm, so recovery requires re-resolving *that account's* swarm. The old code used failed_node.swarm_pubkey(), which was compute_x25519_pubkey(remote_pubkey) -- the X25519 pubkey of the node we had just been rejected by, not of the account -- so it re-resolved the wrong swarm entirely. Requests now carry the account they address in Request::swarm_pubkey (swarm_pubkey_hex in the C params), the 421 path uses that, and a request without one is failed rather than misdirected again. service_node::swarm_pubkey() goes with it: its only caller was that wrong line, and its name invited exactly this mistake. Beyond dev's diff: dev only populates the field in the C API, but pfs has three internal swarm-addressed request sites in core.cpp (the namespace batch poll, the PFS-key prefetch, and the DM store) which dev does not. Left unset those would now fail on their first 421 instead of recovering, so all three go through a swarm_request() helper that carries the pubkey alongside the node -- one place to get right, and a fourth site cannot silently omit it.
Forward-port of #127 (dev commits 724939a..4b582a5) onto pfs. A refresh can succeed and still yield too few nodes -- most easily when the multi-request intersection finds little in common -- and the result was written over the existing cache unconditionally, leaving nothing to route through until the next refresh landed. An empty result wiped it entirely. Treat an under-sized result as a failed refresh instead: keep the cache we have and retry after the usual backoff. The retry path already existed for an unparseable response, so it is lifted out of the catch block into a discard_and_retry lambda holding a weak self ref and reused for both cases. Also carried along from the same branch: seed_payment.py inlines the day-rounding it needs, since the backend moved base.round_datetime_to_next_day into its own test scaffolding. pfs adaptations: `_hexbytes` -> `_hex_b`; the new test helper is static, as pfs builds with -Werror=missing-declarations; and the byte cast when copying the remote key is dropped, view_remote_key() already returning std::byte here.
Forward-port of dev commit 0a23f70; applies unmodified. Aligns the revocationTag column and corrects expiryUnixTs's comment, which claimed milliseconds where the field carries seconds.
…-renewing flag Forward-port of #124 (dev commits 332b3e6..059e529) plus dev commit 7ef8385 onto pfs. pfs already had the auto-renewing half of this (config key `A`, ported as jagerman/pro-auto-renewing-config-pfs), so what lands here is the rest: - The grace period, config key `G`: how much longer the account is served past `E`, so any linked device can compute when coverage actually ends at `E + G`. Deliberately a plain duration rather than an optional -- the backend sends zero when the subscription is not auto-renewing, so "unset" and "zero" describe the same account. - Clearing `E` now clears `G` and `A` with it. A grace that outlived its expiry would silently pair with the *next* `E` write, and that write is typically a proof outcome, which carries no grace to correct it with. - The backend's grace and renewal flags are read off the proof response and off a `subscription_expired` failure. An absent field means "not applicable" and clears any stale cached value rather than preserving it. - The parse helpers move out of pro_backend.cpp: parse_error grows a hierarchy (parse_error_key/_missing/_type) in the new public session/parse_error.hpp, and the JSON readers move to session::detail in the new src/json_parser.hpp, gaining json_maybe and a wire_seconds concept so call sites stop writing sys_seconds{seconds{get<int64_t>()}}. json_require_hex moves there too, per 7ef8385. pfs adaptations: json_require_hex takes std::span<std::byte>; pro_backend.hpp keeps pfs's clock/ed25519 includes and does not gain dev's `using namespace oxenc::literals` (pfs uses session's `_hex_b` here); and the auto-renewing accessors dev's diff re-adds are dropped as pfs already has them.
pfs-only; not going upstream to dev. Every function in the newly-imported json_parser.hpp carried a `json_` prefix -- C-style namespacing by naming convention, in a codebase that has namespaces. Put them in `session::json` and drop the prefix, so call sites read `json::require<T>(obj, "key")`, `json::maybe<T>(...)`, `json::parse(...)`, `json::require_hex(...)`, and internally `json::is<T>` / `json::extract<T>`. `session::detail` is left to what it already held (ProFlags in config/internal.hpp); these helpers were only put there by dev because that is where the extraction landed, not because they belong beside it. One consequence: a parameter named `json` shadows the new namespace, breaking `json::` lookup inside such a function. That only affects definitions, so the four `parse_*` definitions and `read_envelope` take `json_in`, while the public declarations keep `json` -- there the name is doing real work, saying the value is JSON rather than generic input. The C wrappers keep `const char* json` throughout: they only forward it and never qualify with `json::`.
pfs-only, on top of the session::json rename. json_require_hex was misnamed and under-specified: what these fields hold is a binary value -- a pubkey, a signature, a tag -- and hex is merely one encoding the backend may send it in. Renamed to require_binary and taught to accept base64 as well, padded or unpadded. The encoding is chosen by the encoded length, not by inspecting the alphabet, which cannot work: every hex string is also valid base64. That makes the destination length load-bearing, so the buffer must be fixed-size and at least 5 bytes, both enforced by static_assert via CTAD on the span. Below 5 the encodings collide -- 1 byte is 2 chars either way, 2 and 4 bytes are 4 and 8 chars as hex or as padded base64 -- and from 5 up 2N exceeds both 4*ceil(N/3) and ceil(4N/3), so the lengths never coincide again. Dropped the 0x/0X prefix stripping. These are binary values rather than numbers, so the prefix means nothing here, and worse, `0`, `x` and `X` are all valid base64 characters: the strip would have silently corrupted any base64 value beginning that way. Sizes now come from oxenc::to_hex_size and to_base64_size rather than being recomputed inline. Tested through parse_pro_proof: hex, padded base64 and unpadded base64 all decode to the same bytes, and a length matching no encoding is rejected as a parse_error_key naming the field.
Records dev up to 7ef8385 as merged, keeping pfs's tree unchanged (-s ours). Everything in the range is now applied to pfs: - #125 (multi-encrypt padding size) -> 0763aff - #126 (re-resolve swarm after 421) -> 3699813 - #127 (undersized snode refresh) -> ec91a11 - 0a23f70 (proto formatting/comment) -> f8ffbd3 - #124 (pro grace period + tristate) and 7ef8385 (json_require_hex move) -> 85483fe #128 (dev's bump to 1.8.0) is deliberately NOT taken: pfs went to 2.0.0 in db19773 instead, for the API breaks it already carries. Taking dev's 1.8.0 would move pfs's version backwards. Two follow-ups on top are pfs-only and are not going to dev: 4003a13 moves the JSON helpers to session::json without the json_ prefix, and 3706900 reworks json_require_hex into require_binary, which accepts hex or base64.
…d field A ProProof is version 0 by its type, not by a value it carries. Scope ProProofVersion (enum class : uint8_t) and rename ProProof -> ProProof_v0 with `using ProProof = ProProof_v0` (a note explains the alias becomes a variant or virtual base when a v1 arrives). The version field is dropped from the C++ and C proof structs and from operator==, and config no longer sets it. fill_proof also stops reading a JSON `version` off the generate_pro_proof response: the endpoint fixes the format and the proof's version is bound into its signature via the personalisation, so there is nothing for the client to read or gate on -- a future format is a new endpoint returning a new ProProof_vN, not a version bump on this response. The wire version survives only where it must: the protobuf-embedded proof a peer decodes with no endpoint context, where it selects the layout. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…the message A received Pro proof whose wire version we don't recognize used to fail the parse, discarding the entire message -- the recipient silently never saw it. That's backwards for a forward-compat field: a newer proof format should cost the sender their Pro affordances on old clients, not the whole message. parse_pro_message now treats an unknown (or missing) version as a graceful degrade: it flags the proof ProStatus::UnsupportedVersion (C SESSION_PROTOCOL_PRO_STATUS_UNSUPPORTED_VERSION) and returns a non-pro message, so the caller skips signature evaluation and delivers it as an ordinary message. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
session::json::require<T>/maybe<T> now accept a scoped enum T, validated as its underlying integer (nlohmann converts through the underlying type in extract), so an enum-typed field can be requested directly rather than cast at the call site. No current caller needs it -- it is a zero-cost `if constexpr` branch that only instantiates when asked -- but it rounds out the generic helper. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Ported from the dev-side change (Audric Ackermann, f0b9ab2) that better aligns the proof with the protobuf. Rather than the proof carrying a version selector, its format is bound entirely by the signing domain prefix (the `_v0` in BUILD_PROOF_DOMAIN is part of the signed bytes), and a future format arrives as its own protobuf field/message -- not a version bump on this one. - Drop the `version` field from the protobuf ProProof (regenerated pb.cc/pb.h), renumbering so field 1 stays reserved. - Remove the ProProofVersion enum; rename ProProof_v0 back to ProProof and drop the alias (a new format is a new type, not a vN of this one). - Replace ProStatus::UnsupportedVersion with the general ProStatus::Invalid: a proof we cannot read (sender attached none, or it is in a format we don't know, so to us it simply isn't present) is unusable, with no reason-specific status for callers to learn. Such a message is still delivered as non-pro rather than dropped. - Detect that via `!pro_msg.has_proof()` instead of a version check. - The json scoped-enum helper support stays (now caller-less), noted as such. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(PFS) Version Pro Proofs on the outside instead of the inside
Forward-port of #132 (dev commit ba569b2) onto pfs; the base.cpp fix is line-for-line identical to dev's. When we are dirty and merge an incoming config making the identical change, we adopt that message as-is -- but new_conf was built as a MutableConfigMessage, which had already incremented the seqno past the adopted message's own value. The adoption therefore landed on a seqno no stored message occupies, so the peer's next ordinary change (correctly using the value we had burned) looked like a conflict and produced a pointless conflict-resolution push instead of a clean adoption. Test adaptations: std::byte spans rather than unsigned char, `_hex_b`, and Contacts takes the seed directly (pfs's PrivKeySpan). dev's ed_pk/ed_sk and its crypto_sign_ed25519_seed_keypair call are dropped -- they are computed but never used.
The test half of #129 (dev commit 8ee82fb). Its source half needs nothing here: both checks already exist on pfs, just further down. A seed shorter than 32 bytes throws from Encryptor's constructor, which from_file builds first, and the MAX_REGULAR_SIZE limit is enforced in start_encryption with the same message dev uses. This test passes against untouched src/, which is what establishes that rather than the code merely looking equivalent. It is real added coverage: pfs tested the too-large limit only for the in-memory overload, and had no short-seed test at all. The expected message differs from dev's by one word -- pfs throws "attachment::Encryptor requires..." because the check lives in the Encryptor constructor, reached from several entry points, rather than in encrypt().
Records dev up to ce82717 as merged, keeping pfs's tree unchanged (-s ours). All four PRs in the range are accounted for: - #130 (proof version as a scoped enum) was already applied via #131 proproofversion-scoped-enum-pfs: the scoped-enum branch is present in json::is<T> on both branches and the proof `version` field is gone from both. - #132 (spurious seqno increment on merge-adopt) forward-ported in e211602. This was the only one needing a real port; pfs's adopt branch was identical to dev's pre-fix code. - #129 (attachment file encryption input validation) needed no source change: both checks already exist on pfs, relocated into Encryptor's constructor and start_encryption. Its test is ported in ed14944 and passes against untouched src/, which is what demonstrates the behaviour is genuinely there. - #133 (apply disable_mtu_discovery) does not apply: pfs superseded that option with quic_max_udp_payload. The deprecated C flag is translated to opt::quic_max_udp_payload{1200} and reaches the endpoint as a populated make_optional, so the empty-optional no-op dev fixed cannot occur here -- nothing constructs that optional at all.
… Ed25519 Forward-port of #134 (dev 2aea9ed..838a22d) and #135 (39c05a8..021413a) onto pfs. Combined because they interleave: #135 rewrites the pubkey argument of the very ServerDestination initializers #134 gives a port to. #134: a url's authority is split into host and port, since `to_request` builds a ServerDestination from the two separately -- a port left inside the host reaches the network layer as a valid-looking hostname pointing nowhere. Generation writes the port back, omitting it when the scheme already implies it so urls for the default file server stay byte-identical to every other client's. The download destination uses the url's port rather than the local config's, because the file lives on the sender's server. #135: the default file server pubkey becomes its Ed25519 key, with the X25519 form derived per request. The old value was the X25519-only key, which cannot serve as `p=` in a url because it has no Ed private key and cannot be given one. The two unused legacy constants in session_network.cpp go with it. pfs adaptations: the derivation is `compute_x25519_pubkey(ed25519_pubkey::from_hex(...))` rather than dev's `to_span<unsigned char>(oxenc::from_hex(...))` -- pfs's compute_x25519_pubkey takes a fixed-extent byte span and ed25519_pubkey is a std::array<std::byte, 32>, which is the idiom onionreq/builder.cpp already uses. dev's extra session/util.hpp include is unnecessary here as a result. The two forms are pinned equal by the derived-key test. #135's bump to 1.9.0 is deliberately not taken: pfs is at 2.0.0.
Forward-port of #136 (dev commit 153351e). Test-only there and here: pfs compares the fragment against `d` for exact equality, so `d=` already fails it and needs no source change. These pass against untouched src/, which is what establishes that. Worth pinning because the value is easy to reintroduce: Session Desktop builds its fragment with URLSearchParams, which serialises a valueless key WITH the `=`, so a prefix match here would silently read every Desktop stream-encrypted upload as legacy.
Records dev up to cd2aca9 as merged, keeping pfs's tree unchanged (-s ours). All four PRs in the range are applied: - #134 (file server port in download urls) and #135 (file server pubkey as Ed25519) in 1dcea7a, combined because #135 rewrites the pubkey argument of the same ServerDestination initializers #134 gives a port to. - #136 (valueless `d=` is not the stream fragment) in f7ef6e5. Test-only: pfs already compares the fragment for exact equality against `d`, so the tests pass against untouched src/. - #137 (Pro access-not-found url) in 2fae54b, unmodified. #135's "chore: bump to 1.9.0" is deliberately NOT taken, as with #128 before it: pfs is at 2.0.0 for the API breaks it carries, and taking dev's 1.9.0 would move pfs's version backwards.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This branch (
pfs) is now actively maintained alongsidedevand will eventually replace thedevbranch.This adds perfect forward secrecy support with post-quantum encryption; a "Core" component with a database layer to track state; direct-QUIC file server support; and a host of code cleanups.
For extensive details, see the description of PR #90.