View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0011692Talermerchant backendpublic2026-07-27 22:082026-07-27 22:08
ReporterFlorian Dold Assigned ToChristian Grothoff  
PrioritynormalSeverityminorReproducibilityalways
Status assignedResolutionopen 
Target 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

There are no notes attached to this issue.

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