View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0011692Talermerchant backendpublic2026-07-27 22:082026-07-28 16:22
ReporterFlorian Dold Assigned ToChristian Grothoff  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Target Version1.6Fixed in Version1.6 
Summary0011692: merchant creates duplicate token signing keys for same validity period
Description# Duplicate token family keys in contract terms

## 1. Symptom

The `token_families` section of a v1 contract contains, for a single family, **two signing
keys with identical validity periods**. From the dump attached to the bug report (the
reporter replaced each `rsa_pub` value with a `"rrr"` / `"sss"` placeholder):

```json
"slugdiscount1-DWDZRW6T4MFX81NFK9AF6Y13AG": {
  "name": "discount1",
  "keys": [
    {"cipher":"RSA","rsa_pub":"rrr","signature_validity_start":{"t_s":1782484948},"signature_validity_end":{"t_s":1782657748}},
    {"cipher":"RSA","rsa_pub":"rrr","signature_validity_start":{"t_s":1782484948},"signature_validity_end":{"t_s":1782657748}}
  ],
  ...
}
```

Both referenced families in that order (`slugdiscount1-…` and `slugsubscription1-…`) showed
exactly two keys each.

The reproduction below shows the keys are **two genuinely distinct keys** that share one
validity period — the identical-looking `rsa_pub` values in the report are an artifact of
the reporter's redaction, not evidence that the same key was emitted twice.

## 2. Root cause

The backend uses **two different SQL queries** to find a family's keys, and they disagree on
whether the private key's retention deadline matters.

**Output path** — `src/backenddb/get_token_family_key.c:124`, used when an order *issues*
tokens ($2 = `valid_at`, $3 = `pay_deadline`):

```sql
FROM merchant_token_families mtf
LEFT JOIN merchant_token_family_keys mtfk
  ON ( (mtf.token_family_serial = mtfk.token_family_serial)
   AND ($2 >= mtfk.signature_validity_start)
   AND ($2 <= mtfk.signature_validity_end)
   AND ($3 <= mtfk.private_key_deleted_at) ) -- <=== retention condition
WHERE slug=$1
ORDER BY mtfk.signature_validity_start ASC
LIMIT 1
```

**Input path** — `src/backenddb/iterate_token_family_keys.c:203`, used when an order *accepts*
tokens ($2 = `now`, $3 = `pay_deadline`):

```sql
FROM merchant_token_families mtf
LEFT JOIN merchant_token_family_keys mtfk
  USING (token_family_serial)
WHERE slug=$1
  AND COALESCE ($2 <= mtfk.signature_validity_end, TRUE)
  AND COALESCE ($3 >= mtfk.signature_validity_start, TRUE);
                                                  -- no retention condition
```

`private_key_deleted_at` is the point from which the merchant may discard the private key.
It is written **once**, when the key is minted — `insert_token_family_key.c` binds the
`key_expires` argument to column `$6`, `private_key_deleted_at`. There is exactly one
minting site in the tree (`taler-merchant-httpd_post-private-orders.c:1971`), and **no code
anywhere updates the column afterwards**.

Its value is computed in `add_output_token_family()` as, in effect:

```
key_expires = max(pay_deadline_of_the_minting_order,
                  round_down(key.valid_before, validity_granularity))
```

So the stored retention deadline is pinned to *the pay deadline of whichever order happened
to mint the key*.

### The failure sequence

For an order whose pay deadline is later than an existing key's `private_key_deleted_at`:

1. The **output** path runs `get_token_family_key`. The existing key K1 satisfies both
   validity-period conditions but fails `$3 <= private_key_deleted_at`. The `LEFT JOIN`
   yields the family row with NULL key columns — i.e. "this family has no usable key".
2. The backend therefore mints **K2**. Because the validity window is derived
   deterministically from `valid_at`, the granularity and the family's `valid_after`, K2 gets
   the **exact same `signature_validity_start` / `signature_validity_end` as K1**.
3. The **input** path runs `iterate_token_family_keys`, which has no retention condition and
   returns **both** K1 and K2.
4. `add_family_key()` (`taler-merchant-httpd_post-private-orders.c:1588`) deduplicates only by
   public key, via `TALER_token_issue_pub_cmp`. K1 and K2 are different keys, so both are
   kept.

Result: two distinct keys with one validity period in the contract terms.

This only becomes visible when a family is referenced **both** as an output and as an input
within the same order — the output reference mints the surplus key, the input reference is
what surfaces the older one. That is exactly the shape of the
`taler-harness playground advanced-tokens-1` scenario the reporter ran.

## 3. Reproduction

Test: `taler-typescript-core` →
`packages/taler-harness/src/integrationtests/test-merchant-tokenfamily-keys.ts`
(committed on branch `dev/phoenix-bot/duplication` as `eeb717f6b`, marked
`.todo = true` since the backend is unfixed).

It mirrors the reporter's `playground advanced-tokens-1` order — five choices, each family
used as an output in one choice and as an input in another — and then creates **two** orders
against the same families:

| Parameter | Value |
| --- | --- |
| `duration` | 2 minutes |
| `validity_granularity` | 1 minute |
| family `valid_before` | now + 1 year |
| order 1 `pay_deadline` | now + 10 minutes |
| order 2 `pay_deadline` | now + 365 days |

Run:

```
taler-harness run-integrationtests merchant-tokenfamily-keys
```

Observed on current master:

```
[order-1] family slugdiscount1 has 1 key(s)
[order-1] family slugsubscription1 has 1 key(s)
[order-2] family slugdiscount1 has 2 key(s)
[order-2] family slugsubscription1 has 2 key(s)

  start=1785182238 end=1785182358 cipher=RSA pub=040000WKWZ9VPS3MRTG6B93G...
  start=1785182238 end=1785182358 cipher=RSA pub=040000ZQAWVR3R2FGBA5VM45...

windows identical: True
pubs identical : False
```

Order 2's pay deadline (+365 days) exceeds K1's `private_key_deleted_at` (pinned to order 1's
+10 minutes), so order 2 mints a second key over the same window — matching the reported
symptom.

### Conditions that do *not* reproduce it

The playground's **default** parameters do not trigger it: with `duration` = 2 days and
`validity_granularity` = 1 hour, `round_down(valid_before)` lands roughly two days out, far
beyond the +10 minute pay deadline, so the retention condition always holds. Neither a single
order nor two consecutive orders produced a duplicate under those settings. A short token
duration relative to a later order's pay deadline is what exposes it.

Note also that the backend enforces `duration >= validity_granularity + start_offset` at family
creation (HTTP 400, *"duration below validity_granularity plus start_offset"*), so a short
duration requires a correspondingly short granularity.

## 4. Relationship to commit `b1682233`

`b1682233` — *"fix first case of private token family key not being found"*, Christian
Grothoff, **2026-06-27**, the day after this bug was filed — added precisely the clamp that
keeps a freshly minted key alive to its own order's deadline:

```c
/* Make sure key never expires before the payment deadline */
key_expires = GNUNET_TIME_timestamp_max (oc->parse_order.order->pay_deadline,
                                         key_expires);
```

**That fix is incomplete.** It clamps to *the minting order's* pay deadline only. Any later
order with a longer pay deadline still fails the retention condition, so the defect recurs —
which is what the reproduction above demonstrates on current master.

The same commit touched `taler-merchant-httpd_post-orders-ORDER_ID-pay.c`, replacing a hard
assertion with an error path whose comment describes the other face of this same asymmetry:

```c
/* The key SHOULD have been created when we created the order (after all, the
   client was able to blind, and that requires us having had a public key).
   So how did we loose it? Bad bug, this should not be possible. */
```

and left a `FIXME: but it does! see merchant-tokenfamilies test!` on the neighbouring
`GNUNET_assert` in `sign_token_envelopes()`.

### A second, related hazard (reasoned, not yet reproduced)

The in-memory lookup `find_key_index()`
(`taler-merchant-httpd_post-private-orders.c:1679`) matches on the validity period **only** —
it does not consider `private_key_deleted_at` either. So if a contract's *first* reference to
a family is an input, the input path loads K1 into the in-memory family, and a subsequent
output reference will match K1 through `find_key_index()` and bypass the retention check
entirely. The order would then commit to a `key_index` whose private key may legitimately be
deleted before payment — producing exactly the 500 *"private token family key not found"* that
`b1682233` added handling for. This follows from reading the code; I did not construct a test
for it.

## 5. Impact

- **Contract terms** carry redundant keys — cosmetic, matching the "minor" severity on the
  ticket.
- **Key proliferation:** once the retention condition is failing for a family, *every*
  subsequent order mints another key over the same window. This is unbounded growth in
  `merchant_token_family_keys`, plus a fresh RSA keypair generation per order.
- **Payment failures:** the same asymmetry is the likely source of the
  *"private token family key not found"* internal error the pay handler now guards against
  (see §4).

## 6. Recommended fix

The duplication is a symptom; the defect is that a key's retention deadline is pinned to one
order and never revisited. Two options:

**Option A — extend the existing key (recommended).** When a key covers `valid_at` but its
`private_key_deleted_at` is earlier than the new order's pay deadline, extend it rather than
mint a replacement:

```sql
UPDATE merchant_token_family_keys
   SET private_key_deleted_at = GREATEST(private_key_deleted_at, $pay_deadline)
 WHERE ...
```

This needs a new `libtalermerchantdb` operation and a call in `add_output_token_family()` on
the path where the key is found by validity period but rejected by retention. It preserves the
invariant "one key per validity period" and removes the `find_key_index()` hazard in §4, since
any key reachable in memory is then guaranteed to outlive the deadline.

**Option B — decouple retention from the pay deadline.** Derive `private_key_deleted_at`
solely from the validity period (plus a grace margin) and *reject* orders whose pay deadline
outruns it. Simpler, no new DB operation, but it changes which orders are accepted and needs a
protocol/UX decision.

Independently of either, `add_family_key()` could reject a second key for a validity period
already present — but as a defence-in-depth assertion, not the fix: it would hide the
proliferation rather than stop it.

Once fixed, drop the `runMerchantTokenfamilyKeysTest.todo = true` marker in the test above.

## 7. Limits of this investigation

- The reporter's exact production parameters were not recovered. The reproduction establishes
  *a* mechanism that yields the reported symptom precisely, not proof that test/demo hit it
  by this same route.
- I could not build the pre-report tree (`3fb61e33`, 2026-06-26) to compare behaviour: it no
  longer compiles against the current exchange libraries (`TALER_BANK_registration` changed
  signature). Rolling the exchange back too was out of proportion to the question.
- The §4 "second hazard" is a code-reading conclusion, not a tested one.
TagsNo tags attached.

Activities

Christian Grothoff

2026-07-28 16:22

manager   ~0029251

Resolved by bumping the key expiration time as suggested. Originally I considered that creating additional keys was OK, but LLM is right that this would open the door to MANY keys, plus the wallets could not know which one to use without modifying the protocol, so this is cleaner.

Issue History

Date Modified Username Field Change
2026-07-27 22:08 Florian Dold New Issue
2026-07-27 22:08 Florian Dold Status new => assigned
2026-07-27 22:08 Florian Dold Assigned To => Christian Grothoff
2026-07-28 16:22 Christian Grothoff Status assigned => resolved
2026-07-28 16:22 Christian Grothoff Resolution open => fixed
2026-07-28 16:22 Christian Grothoff Fixed in Version => 1.6
2026-07-28 16:22 Christian Grothoff Note Added: 0029251