fix: SCP80/RAM re-read the preset before every operation; no counter bump on a rejected send (v3.6.3)

The SCP80/RAM forms hold a copy of the preset (counter, keys, TAR, SPI). Editing the preset on the Cards tab saved correctly, but the form kept the old copy: the operation sent the stale counter (the card answers cntr_low) and the post-send sync wrote the stale value back over the preset - the saved counter silently reverted. Reproduced in a real DOM: after select in SCP80: preset=0000000001 sp=0000000001 after Cards edit+save: preset=00000000AA sp=0000000001 after a sync: preset=0000000001 (edit lost) - `cardsApply()` split into `cardsApplyFields()` (field copy, no packet) and `cardsApply()` = fields + genSp; new `spRefreshFromPreset(selId)` re-reads the selected preset from `sp-card-sel` / `ram-card-sel` and re-applies it. - `pysimSendOta()` and `ramExecute()` call it before starting, so every SCP80/RAM operation uses the preset as it is now. - A rejected send no longer advances the counter: new `spPorAccepted(por)` gates the advance+write-back in `pysimSendOta`, the Explore pagination and its GET DATA step, and the server's RAM install (`_ram_next_cntr`: advance only for `por_ok`/`no_por` steps). A failed install still returns `final_cntr` (the accepted prefix) and the PWA persists it, so a retry never replays a counter the card already consumed. - tests: cards_counter.test.js (the stale-form regression, RAM selector, fields-without-genSp, spPorAccepted) and `_ram_next_cntr` cases; the cards_form/ram harnesses updated for the split. - docs/api.md counter semantics; AGENTS preset-source-of-truth rule. 607 frontend / 488 Python green; version 3.6.3; sw simple-v276.
2026-09-27 23:43:28 +03:00
parent 36d2f71bc7
commit 39c82f26f3
9 changed files with 182 additions and 24 deletions
@@ -477,7 +477,7 @@ Clears a finished run report (409 while a run is active).

 ### `POST /api/ram-install`

-Install a Java Card `.cap` file on the card via GlobalPlatform commands (INSTALL[for load] → LOAD ×N → INSTALL[for install (+ make selectable)]) wrapped in SCP80 secured packets. Each step is sent via ENVELOPE; the PoR verdict and the remote command's own status word are both checked and the sequence aborts on the first failure (a non-`por_ok` PoR, a remote SW outside the success set, or an undecodable PoR). The `.cap` archive (a ZIP of nested components) is parsed server-side in `_cap_parse`; no external tooling is required.
+Install a Java Card `.cap` file on the card via GlobalPlatform commands (INSTALL[for load] → LOAD ×N → INSTALL[for install (+ make selectable)]) wrapped in SCP80 secured packets. Each step is sent via ENVELOPE; the PoR verdict and the remote command's own status word are both checked and the sequence aborts on the first failure (a non-`por_ok` PoR, a remote SW outside the success set, or an undecodable PoR).  The counter advances only for a packet the card accepted (PoR `por_ok`); `final_cntr` is returned on success **and** on failure, so the caller keeps the card's consumed counter (a rejected packet leaves it unchanged). The `.cap` archive (a ZIP of nested components) is parsed server-side in `_cap_parse`; no external tooling is required.

 **Request body:**
 ```json
@@ -1665,7 +1665,7 @@
 // ===== Version =====
 // Single source of truth for the PWA version: shown in the header and used
 // by the server version check in pysimConnect().
-const SIMPLE_VERSION = '3.6.2';
+const SIMPLE_VERSION = '3.6.3';
 document.getElementById('app-version').textContent = 'v' + SIMPLE_VERSION;

 // ===== Tab switching =====
@@ -7896,6 +7896,10 @@ async function pysimSendOta() {
 		alert('Card reader server is not connected');
 		return;
 	}
+	// Re-read the preset (counter and keys) before sending: the Cards tab may
+	// have changed it after the form was filled, and a stale counter is
+	// rejected by the card (cntr_low) and then written back over the preset.
+	spRefreshFromPreset('sp-card-sel');
 	const sp = document.getElementById('sp-result').value.replace(/[^0-9a-fA-F]/g, '').toUpperCase();
 	if (!sp) { alert('No secured packet to send. Generate a secure packet first.'); return; }
 	const sendResultEl = document.getElementById('sp-send-result');
@@ -7927,16 +7931,21 @@ async function pysimSendOta() {
 				porStatusEl.classList.remove('hidden', okPor ? 'text-red-600' : 'text-green-600');
 				porStatusEl.classList.add(okPor ? 'text-green-600' : 'text-red-600');
 			}
-			// The counter advances after every successful send, PoR or not: the
-			// card rejects a repeated counter (replay protection) and the same
-			// secured packet must never be sent twice (v1.9.6, revised 2.1.9).
-			const cntrEl = document.getElementById('sp-cntr');
-			cntrEl.value = spNextCntr(cntrEl.value);
-			msg += ' | CNTR -> ' + cntrEl.value;
-			document.getElementById('sp-result').value = '';
-			spShowSizeInfo();
-			// keep the selected card preset in sync with the new counter (v1.9.8)
-			spCntrSyncPreset();
+			// The counter advances only for a packet the card accepted: a
+			// rejected send (cntr_low, PoR error) must leave the preset value
+			// untouched - the card did not consume the packet (v3.6.3; the old
+			// "advance always" rule silently reverted preset edits).
+			if (spPorAccepted(por)) {
+				const cntrEl = document.getElementById('sp-cntr');
+				cntrEl.value = spNextCntr(cntrEl.value);
+				msg += ' | CNTR -> ' + cntrEl.value;
+				document.getElementById('sp-result').value = '';
+				spShowSizeInfo();
+				// keep the selected card preset in sync with the new counter
+				spCntrSyncPreset();
+			} else {
+				msg += ' | CNTR unchanged (' + ((por && por.response_status) || 'PoR error') + ')';
+			}
 			sendResultEl.textContent = msg;
 			if (por && por.raw) {
 				const rawLine = document.createElement('div');
@@ -8741,13 +8750,14 @@ async function ramExplore(sp) {
 				const apdu = '80F2' + p1 + p2 + dataField + 'C0000000';
 				ramShowProgress(label + ' P1=' + p1 + ' P2=' + p2 + '...');
 				const res = await ramSendOta(apdu, Object.assign({}, sp, { cntr, spi2 }));
-				cntr = ramIncrementCntr(cntr);
 				if (!res.success || !res.por || res.por.response_status !== 'por_ok') {
 					const errorMsg = res.por ? res.por.response_status : (res.error || t('no data'));
 					errors.push(label + ': ' + errorMsg);
 					tlvFailed = true;
 					break;
 				}
+				// the card consumed the packet: advance for the next step
+				cntr = ramIncrementCntr(cntr);
 			const data = res.por.decoded ? res.por.decoded.last_response_data : '';
 			const sw = (res.por.decoded ? res.por.decoded.last_status_word : '').toUpperCase();
 				// If first attempt with P2=02 gets an unsupported error, retry with P2=00.
@@ -8786,8 +8796,8 @@ async function ramExplore(sp) {
 	 ramShowProgress(t('Memory (GET DATA FF21)') + '...');
 	try {
 		const memRes = await ramSendOta('80CAFF2100', Object.assign({}, sp, { spi2: '01' }));
-		cntr = ramIncrementCntr(cntr);
 		if (memRes.success && memRes.por && memRes.por.response_status === 'por_ok') {
+			cntr = ramIncrementCntr(cntr);
 			const data = memRes.por.decoded ? memRes.por.decoded.last_response_data : '';
 			if (!data) {
 				errors.push(t('Memory') + ': ' + t('(no data)'));
@@ -8924,8 +8934,11 @@ async function ramInstallCap(sp) {
 	(data.steps || []).forEach((s, idx) => { txt += ramStepLine(s, idx) + '\n'; });
 	stepsEl.textContent = txt;

+	// Persist the counter the card actually consumed (accepted packets only):
+	// even a failed install leaves the accepted steps behind, and replaying
+	// their counter would make the card reject the next attempt.
+	if (data.final_cntr) ramSaveCntr(data.final_cntr);
 	if (data.success) {
-		ramSaveCntr(data.final_cntr);
 		let sizeInfo = '';
 		if (data.load_block_size) {
 			sizeInfo = ' — ' + t('LOAD blocks') + ': ' + data.load_block_size + ' B';
@@ -8948,6 +8961,9 @@ async function ramExecute() {
 	ramClearResults();
 	const cardIdx = parseInt(document.getElementById('ram-card-sel').value, 10);
 	if (!isNaN(cardIdx) && cards[cardIdx]) _ramCardIdx = cardIdx;
+	// Re-read the preset before the operation: its counter/keys may have
+	// changed in the Cards tab after the RAM form was filled.
+	spRefreshFromPreset('ram-card-sel');
 	const sp = getRamSpParams();
 	if (!sp.kicKey || !sp.kidKey) {
 		alert(t('Select a card preset with keys first (RAM subtab → Card preset)'));
@@ -9399,9 +9415,11 @@ function cardsAutoSelectByIccid(iccid) {
 	return idx;
 }

-function cardsApply(idx) {
+// Fill the SP form from the preset (no packet generation): the form is a
+// working copy, the preset stays the source of truth.
+function cardsApplyFields(idx) {
 	const c = cards[parseInt(idx)];
-	if (!c) return;
+	if (!c) return false;
 	document.getElementById('sp-spi1').value = c.spi1;
 	document.getElementById('sp-spi2-hex').value = c.spi2;
 	const kicByte = parseInt(c.kic, 16);
@@ -9418,9 +9436,35 @@ function cardsApply(idx) {
 	document.getElementById('sp-kid-key').value = c.kidKey;
 	spInvalidate();
 	updateSp();
+	return true;
+}
+
+function cardsApply(idx) {
+	if (!cardsApplyFields(idx)) return;
 	genSp();
 }

+// SCP80/RAM operations re-read the selected preset before starting: the
+// Cards tab (or a counter write-back) may have changed it after the form was
+// filled, and a stale counter is rejected by the card (cntr_low) and would
+// then be written back over the preset.  Returns the preset counter ('' when
+// no preset is selected).
+function spRefreshFromPreset(selId) {
+	const sel = document.getElementById(selId || 'sp-card-sel');
+	const idx = sel ? parseInt(sel.value, 10) : NaN;
+	if (isNaN(idx) || !cards[idx]) return '';
+	if (selId === 'ram-card-sel') _spTarKey = 'tar';
+	cardsApply(idx);
+	return cards[idx].cntr;
+}
+
+// A send counts as accepted when there is no PoR to check (the SPI requests
+// none) or the PoR is por_ok; anything else (cntr_low, Por error) must leave
+// the counter untouched.
+function spPorAccepted(por) {
+	return !por || por.response_status === 'por_ok';
+}
+
 function downloadJson(name, obj) {
 	const blob = new Blob([JSON.stringify(obj, null, 2)], {type: 'application/json'});
 	const a = document.createElement('a');
@@ -1,4 +1,4 @@
-const CACHE = 'simple-v275';
+const CACHE = 'simple-v276';
 const URLS = [
  'index.html',
  'help.html',
@@ -0,0 +1,89 @@
+const { test } = require('node:test');
+const assert = require('node:assert');
+const fs = require('node:fs');
+const path = require('node:path');
+
+const html = fs.readFileSync(path.join(__dirname, '..', 'index.html'), 'utf8');
+
+function extractFunc(src, name) {
+	const re = new RegExp('function\\s+' + name + '\\s*\\([^)]*\\)\\s*\\{');
+	const m = re.exec(src);
+	if (!m) throw new Error('function ' + name + ' not found');
+	let i = m.index + m[0].length - 1;
+	let depth = 0;
+	for (; i < src.length; i++) {
+		if (src[i] === '{') depth++;
+		else if (src[i] === '}') {
+			depth--;
+			if (depth === 0) break;
+		}
+	}
+	return src.slice(m.index, i + 1);
+}
+
+let code = '';
+for (const fn of ['cardsTarValue', 'cardsApplyFields', 'cardsApply',
+	'spRefreshFromPreset', 'spPorAccepted', 'spNextCntr']) {
+	code += extractFunc(html, fn) + '\n';
+}
+eval(code);
+
+// The SP form is a working copy: the preset is the source of truth, and
+// every SCP80/RAM operation re-reads it before starting (v3.6.3).
+function fakeEnv(selValue, selId, presetCntr) {
+	const els = {};
+	for (const id of ['sp-spi1', 'sp-spi2-hex', 'sp-kic-idx', 'sp-kic-alg',
+		'sp-kid-idx', 'sp-kid-alg', 'sp-tar', 'sp-cntr', 'sp-kic-key', 'sp-kid-key']) {
+		els[id] = { value: '' };
+	}
+	els[selId || 'sp-card-sel'] = { value: selValue };
+	globalThis.document = { getElementById: id => els[id] || null };
+	globalThis.cards = [{ name: 'C', cntr: presetCntr, spi1: '16', spi2: '01',
+		kic: '15', kid: '15', tar: '000000', uiccTar: 'B00000',
+		kicKey: 'AA', kidKey: 'BB' }];
+	globalThis._spTarKey = 'uiccTar';
+	globalThis.updateSpKic = () => {};
+	globalThis.updateSpKid = () => {};
+	globalThis.spInvalidate = () => {};
+	globalThis.updateSp = () => {};
+	let gen = 0;
+	globalThis.genSp = () => { gen++; };
+	return { els, gen: () => gen };
+}
+
+test('spRefreshFromPreset re-reads the edited preset before an operation', () => {
+	const env = fakeEnv('0', 'sp-card-sel', '00000000AA');
+	env.els['sp-cntr'].value = '0000000001';        // stale form copy
+	assert.strictEqual(spRefreshFromPreset('sp-card-sel'), '00000000AA');
+	assert.strictEqual(env.els['sp-cntr'].value, '00000000AA');
+	assert.ok(env.gen() > 0, 'the packet must be regenerated from the new counter');
+	// no preset selected: the manual form is left alone
+	globalThis.cards = [];
+	assert.strictEqual(spRefreshFromPreset('sp-card-sel'), '');
+	assert.strictEqual(env.els['sp-cntr'].value, '00000000AA');
+});
+
+test('spRefreshFromPreset for the RAM selector targets the ISD TAR', () => {
+	const env = fakeEnv('0', 'ram-card-sel', '0000000010');
+	assert.strictEqual(spRefreshFromPreset('ram-card-sel'), '0000000010');
+	assert.strictEqual(_spTarKey, 'tar');
+	assert.strictEqual(env.els['sp-tar'].value, '000000');
+});
+
+test('cardsApplyFields fills the form without generating a packet', () => {
+	const env = fakeEnv('0', 'sp-card-sel', '0000000007');
+	env.els['sp-cntr'].value = 'nonsense';
+	assert.strictEqual(cardsApplyFields(0), true);
+	assert.strictEqual(env.els['sp-cntr'].value, '0000000007');
+	assert.strictEqual(env.gen(), 0, 'a plain field refresh must not rebuild the packet');
+	cardsApply(0);
+	assert.strictEqual(env.gen(), 1);
+	assert.strictEqual(cardsApplyFields(3), false);
+});
+
+test('spPorAccepted treats a missing PoR and por_ok as accepted', () => {
+	assert.strictEqual(spPorAccepted(undefined), true);
+	assert.strictEqual(spPorAccepted({ response_status: 'por_ok' }), true);
+	assert.strictEqual(spPorAccepted({ response_status: 'cntr_low' }), false);
+	assert.strictEqual(spPorAccepted({ response_status: 'rc_cc_ds_failed' }), false);
+});
@@ -23,7 +23,7 @@ function extractFunc(src, name) {

 let code = '';
 for (const fn of ['cardsTarValue', 'cardsFormValues', 'cardsClearForm', 'cardsEdit',
-	'cardsAdd', 'cardsImport', 'cardsApply', 'ramApplyCard',
+	'cardsAdd', 'cardsImport', 'cardsApplyFields', 'cardsApply', 'ramApplyCard',
 	'cardsScp80Complete', 'cardsScp81Complete', 'cardsAdmPresent',
 	'spTarKeyForPack', 'spPresetTar', 'packToSp']) {
 	code += extractFunc(html, fn) + '\n';
@@ -45,6 +45,12 @@ const els = {};
 const doc = { getElementById: (id) => { if (!els[id]) els[id] = {value:''}; return els[id]; } };
 global.document = doc;

+// ramApplyCard/ramExecute delegate to the SP-form helpers; their real
+// behaviour (and the preset re-read before an operation) is covered by
+// cards_counter.test.js, so keep them as global stubs here.
+globalThis.cardsApply = () => {};
+globalThis.spRefreshFromPreset = () => '';
+
 function reset() { for (const id of Object.keys(els)) delete els[id]; }

 function genRamResult(fields) {
@@ -330,7 +336,8 @@ test('ramApplyCard remembers a valid picked preset', () => {
 	ramApplyCard('');
 	assert.strictEqual(_ramCardIdx, 1, 'invalid pick must not forget the preset');
 	delete globalThis.cards;
-	delete globalThis.cardsApply;
+	globalThis.cardsApply = () => {};
+	delete globalThis.cards;
 });

 test('ramExecute commits the dropdown selection before running', async () => {
@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"

 [project]
 name = "pysim-simple-server"
-version = "3.6.2"
+version = "3.6.3"
 description = "HTTP REST server wrapping pysim for the SIMple PWA"
 requires-python = ">=3.8"
 # pysim is a git-only dependency installed explicitly by setup.bat/setup.sh.
@@ -31,7 +31,7 @@ from osmocom.tlv import BER_TLV_IE
 from osmocom.utils import rpad


-VERSION = '3.6.2'
+VERSION = '3.6.3'

 MAX_ENVELOPE_SEGMENTS = 5  # max SMS segments for outgoing C-APDU in ENVELOPE

@@ -1121,6 +1121,16 @@ def _por_remote_sw(por):
    return ''


+def _ram_next_cntr(cntr, advance):
+    """Advance the SCP80 counter by one only when the card accepted the
+    packet (PoR ok / no PoR expected): a rejected packet (cntr_low,
+    rc_cc_ds_failed, ...) leaves the card's expectation and the preset
+    counter untouched."""
+    if not advance:
+        return cntr
+    return '%010X' % ((int(cntr, 16) + 1) % (2 ** 32))
+
+
 def _ram_step_result(step_name, last_sw, por, por_hex, bytes_, segments):
    """Assemble a RAM-install step record and its failure reason.

@@ -6105,8 +6115,9 @@ class PysimHandler(BaseHTTPRequestHandler):
                            step.get('por_sw', '-'),
                            (' error=%s' % step_error) if step_error else '',
                            result['bytes'], result['segments']))
-                        # Increment counter
-                        cntr = '%010X' % ((int(cntr, 16) + 1) % (2 ** 32))
+                        # Advance the counter only for an accepted packet
+                        cntr = _ram_next_cntr(
+                            cntr, step.get('por_status') in ('por_ok', 'no_por'))
                        if step_error:
                            failure['error'] = '%s: %s' % (step_name, step_error)
                            return False
@@ -6134,6 +6145,7 @@ class PysimHandler(BaseHTTPRequestHandler):
                        resp = {'success': False, 'steps': steps, 'failed_step': len(steps),
                                'error': (encode_error or failure.get('error')
                                          or ('%s failed' % step_name)),
+                                'final_cntr': cntr,
                                'load_file_aid': loadfile_aid, 'module_aid': module_aid,
                                'load_block_size': block_size,
                                'load_block_size_requested': block_size_req,
@@ -31,6 +31,7 @@ from pysim_simple_server.server import (
    _parse_select_item,
    _parse_setup_menu_items,
    _por_remote_sw,
+    _ram_next_cntr,
    _ram_remote_sw_ok,
    _ram_step_result,
    _record_tr,
@@ -1300,6 +1301,11 @@ class RamPorStepTest(unittest.TestCase):
        self.assertEqual(err, 'PoR undecodable')
        self.assertEqual(step['por_raw'], 'DEADBEEF')

+    def test_counter_advances_only_for_accepted_packets(self):
+        self.assertEqual(_ram_next_cntr('0000000010', True), '0000000011')
+        self.assertEqual(_ram_next_cntr('0000000010', False), '0000000010')
+        self.assertEqual(_ram_next_cntr('FFFFFFFF', True), '0000000000')
+
    def test_remote_sw_from_expanded_response(self):
        por = {'response_status': 'por_ok', 'decoded': {},
               'responses': [{'status_word': '9000'}, {'status_word': '6A82'}]}