PD: Retransmit cached reply on sequence-repeatgc

Per OSDP specification, a PD must re-emit its previous reply verbatim
when the CP re-sends a command with the same sequence number.
Rebuilding the reply corrupts the SC MAC chain and tears down the
secure channel on every CP retry probe.

Cache the last sent reply and re-emit it on a matching seq-repeat.
Add unit tests covering the retransmit path and cache invalidation.

Signed-off-by: Siddharth Chandrasekaran <sidcha.dev@gmail.com>

Author: sidcha

src/osdp_common.h

@@ -437,6 +437,12 @@ struct osdp_pd {
 	uint32_t packet_scan_skip;
 	bool reply_prebuilt;
 
+	/* Retransmit cache: last successfully-sent reply bytes live in the
+	 * shared tx_buf as long as nothing overwrites them. On a sequence
+	 * repeat we re-emit those bytes verbatim (see phy_check_packet). */
+	uint16_t last_tx_len; /* 0 = cache empty */
+	uint8_t last_cmd_id;
+
 	int cmd_id;            /* Currently processing command ID */
 	int reply_id;          /* Currently processing reply ID */
 	union {
@@ -477,6 +483,7 @@ struct osdp {
 	struct osdp_pd *pd;    /* base of PD list (must be at lest one) */
 	struct osdp_channel channel; /* OSDP channel */
 	uint8_t tx_buf[OSDP_PACKET_BUF_SIZE];
+	uint8_t *rx_buf; /* RX landing buffer: aliased to tx_buf in CP; distinct in PD */
 
 	/* CP event callback to app with opaque arg pointer as passed by app */
 	void *event_callback_arg;
@@ -704,6 +711,8 @@ static inline void sc_deactivate(struct osdp_pd *pd)
 		osdp_sc_teardown(pd);
 	}
 	CLEAR_FLAG(pd, PD_FLAG_SC_ACTIVE);
+	/* Cached retransmit reply is no longer meaningful without SC. */
+	pd->last_tx_len = 0;
 }
 
 static inline void make_request(struct osdp_pd *pd, uint32_t req) {
@@ -791,13 +800,20 @@ static inline struct osdp_rb *cp_static_rx_rb_array_get(void)
 
 static inline struct osdp *cp_ctx_alloc(void)
 {
+	struct osdp *ctx;
 #ifdef OPT_OSDP_STATIC
-	struct osdp *ctx = cp_static_ctx_get();
+	ctx = cp_static_ctx_get();
 	memset(ctx, 0, sizeof(struct osdp));
-	return ctx;
 #else
-	return calloc(1, sizeof(struct osdp));
+	ctx = calloc(1, sizeof(struct osdp));
+	if (!ctx) {
+		return NULL;
+	}
 #endif /* OPT_OSDP_STATIC */
+	/* CP mode does not cache retransmit replies; alias rx_buf to tx_buf
+	 * to preserve the legacy shared-buffer behavior with zero cost. */
+	ctx->rx_buf = ctx->tx_buf;
+	return ctx;
 }
 
 static inline struct osdp_pd *cp_pd_array_alloc(int old_num_pd, int num_pd)
@@ -861,6 +877,12 @@ static inline struct osdp_pd *pd_static_ctx_pd_get(void)
 	return &g_osdp_pd_ctx;
 }
 
+static inline uint8_t *pd_static_rx_buf_get(void)
+{
+	static uint8_t g_osdp_pd_rx_buf[OSDP_PACKET_BUF_SIZE];
+	return g_osdp_pd_rx_buf;
+}
+
 #ifdef OPT_OSDP_RX_ZERO_COPY
 
 static inline struct osdp_rx_pkt *pd_static_rx_pkt_get(void)
@@ -883,13 +905,23 @@ static inline struct osdp_rb *pd_static_rx_rb_get(void)
 
 static inline struct osdp *pd_ctx_alloc(void)
 {
+	struct osdp *ctx;
 #ifdef OPT_OSDP_STATIC
-	struct osdp *ctx = pd_static_ctx_get();
+	ctx = pd_static_ctx_get();
 	memset(ctx, 0, sizeof(struct osdp));
-	return ctx;
+	ctx->rx_buf = pd_static_rx_buf_get();
 #else
-	return calloc(1, sizeof(struct osdp));
+	ctx = calloc(1, sizeof(struct osdp));
+	if (!ctx) {
+		return NULL;
+	}
+	ctx->rx_buf = calloc(1, OSDP_PACKET_BUF_SIZE);
+	if (!ctx->rx_buf) {
+		free(ctx);
+		return NULL;
+	}
 #endif /* OPT_OSDP_STATIC */
+	return ctx;
 }
 
 static inline struct osdp_pd *pd_instance_alloc(void)

src/osdp_pd.c

@@ -1007,6 +1007,8 @@ static int pd_send_reply(struct osdp_pd *pd)
 		if (ret < 0) {
 			return OSDP_PD_ERR_GENERIC;
 		}
+		pd->last_tx_len = (uint16_t)ret;
+		pd->last_cmd_id = (uint8_t)pd->cmd_id;
 		return OSDP_PD_ERR_NONE;
 	}
 
@@ -1032,6 +1034,12 @@ static int pd_send_reply(struct osdp_pd *pd)
 		return OSDP_PD_ERR_GENERIC;
 	}
 
+	/* Snapshot the just-sent reply for potential retransmit on seq
+	 * repeat. The bytes (including CRC/MAC/encrypted payload) live in
+	 * tx_buf and are preserved until the next reply is built. */
+	pd->last_tx_len = (uint16_t)ret;
+	pd->last_cmd_id = (uint8_t)pd->cmd_id;
+
 	return OSDP_PD_ERR_NONE;
 }
 
@@ -1355,6 +1363,7 @@ void osdp_pd_teardown(osdp_t *ctx)
 	}
 #endif /* OPT_OSDP_RX_ZERO_COPY */
 	safe_free(pd->file);
+	safe_free(pd_ctx->rx_buf);
 	safe_free(pd);
 	safe_free(ctx);
 #endif

src/osdp_phy.c

@@ -395,7 +395,9 @@ int osdp_phy_send_packet(struct osdp_pd *pd, uint8_t *buf,
 		return OSDP_ERR_PKT_BUILD;
 	}
 
-	return OSDP_ERR_PKT_NONE;
+	/* return the number of bytes actually put on the wire so that
+	 * callers can cache the full finalized packet for retransmit */
+	return len;
 }
 
 static int phy_validate_header(struct osdp_pd *pd, uint8_t *buf,
@@ -578,16 +580,38 @@ static int phy_check_packet(struct osdp_pd *pd, uint8_t *buf, int pkt_len)
 			 * secure channels.
 			 */
 			phy_reset_seq_number(pd);
+			pd->last_tx_len = 0;
 			sc_deactivate(pd);
 		}
 		else if (comp == pd->seq_number) {
 			/**
-			 * Sometimes, a CP re-sends the same command without
-			 * incrementing the sequence number. To handle such cases,
-			 * we will move the sequence back one step (with do_inc
-			 * set to -1) and then process the packet all over again
-			 * as if it was the first time we are seeing it.
+			 * CP re-sent the same command without incrementing the
+			 * sequence number. Per OSDP spec, we must re-emit the
+			 * previous reply bit-identically. Rebuilding it here would
+			 * corrupt the SC MAC chain (r_mac/c_mac have already
+			 * advanced) and re-consume state (event/card/file data).
+			 *
+			 * If we have a cached reply and the command id matches,
+			 * re-send the bytes still sitting in tx_buf (RX goes to a
+			 * separate rx_buf in PD mode, so tx_buf is preserved).
 			 */
+			if (pd->last_tx_len > 0) {
+				int scb_len = (pkt->control & PKT_CONTROL_SCB) ?
+					pkt->data[0] : 0;
+				uint8_t rx_cmd_id = buf[sizeof(*pkt) + scb_len];
+
+				if (rx_cmd_id == pd->last_cmd_id) {
+					LOG_INF("Seq repeat: retransmitting cached reply");
+					osdp_channel_send(pd,
+						osdp_tx_staging_buf(pd),
+						pd->last_tx_len);
+					return OSDP_ERR_PKT_SKIP;
+				}
+				/* seq matches but cmd id doesn't: drop the cache
+				 * and fall through to the normal path which will
+				 * MAC-verify and NAK if the packet is bogus. */
+				pd->last_tx_len = 0;
+			}
 			phy_rollback_seq_number(pd);
 			LOG_INF("Received a sequence repeat packet!");
 		}
@@ -897,10 +921,11 @@ void osdp_phy_state_reset(struct osdp_pd *pd, bool is_error)
 	pd->packet_buf_len = 0;
 	pd->packet_len = 0;
 	pd->phy_state = 0;
-	pd->packet_buf = osdp_tx_staging_buf(pd);
+	pd->packet_buf = pd_to_osdp(pd)->rx_buf;
 	if (is_error) {
 		pd->phy_retry_count = 0;
 		pd->phy_tx_seq = 0;
+		pd->last_tx_len = 0;
 		phy_reset_seq_number(pd);
 		if (channel->flush) {
 			channel->flush(channel->data);

src/osdp_sc.c

@@ -242,6 +242,10 @@ void osdp_sc_setup(struct osdp_pd *pd)
 
 	osdp_crypt_setup();
 
+	/* Any cached retransmit reply belongs to the previous session and
+	 * must not be re-emitted after a handshake. */
+	pd->last_tx_len = 0;
+
 	memcpy(scbk, pd->sc.scbk, 16);
 	memset(&pd->sc, 0, sizeof(struct osdp_secure_channel));
 	memcpy(pd->sc.scbk, scbk, 16);

tests/unit-tests/CMakeLists.txt

@@ -19,6 +19,7 @@ add_library(${LIB_OSDP_TEST} STATIC EXCLUDE_FROM_ALL
 list(APPEND OSDP_UNIT_TEST_SRC
 	test.c
 	test-cp-phy.c
+	test-pd-phy.c
 	test-cp-fsm.c
 	test-file.c
 	test-commands.c