v4.5.0 relay_tx_check_decode_ack() does not calculate MIC according to TS011-1.0.0

[cjhdev]

I think there is a mistake in v4.5.0 relay_tx_check_decode_ack() where the WOR channel frequency and datarate is used to calculate the ACK MIC, instead of the frequency and datarate extracted from the WOR frame that is being acknowledged.

    const relay_tx_channel_config_t* conf =
        ( infos->last_ch_idx == 0 ) ? &infos->default_ch_config : &( infos->relay_tx_config.second_ch );

    wor_ack_mic_info_t ack_mic_info = {
        .dev_addr     = lorawan_api_devaddr_get( relay_stack_id ),
        .wfcnt        = infos->fcnt,
        .frequency_hz = conf->freq_hz,
        .datarate     = conf->dr,
    };

    // SMTC_MODEM_HAL_TRACE_PRINTF( "frequency_hz = %d, infos->last_ch_idx = %d ,  .dev_addr   = %x, datarate = %d\n",
    // conf->ack_freq_hz,infos->last_ch_idx,lorawan_api_devaddr_get( relay_stack_id ),conf->dr);
    //  Key is set to NULL because it is already save in the crypto element (soft or hard)
    const uint32_t mic_calc    = wor_compute_mic_ack( &ack_mic_info, infos->buffer, NULL );

The values are set in ack_mic_info which is then passed to wor_compute_mic_ack().

Line 793 of TS011-1.0.0 defines the MIC as:

CMAC = aes128_cmac(WorSIntKey, B0 | AckUplinkEnc | WOR | pad16)

Table 20 shows how to make WOR.

Line 797 immediately before table 20 says:

WOR contains the data received in the WOR Relay Class A Uplink and is defined as:

[opeyrard]

Thank you very much for you feedback.
After confirmation with LoRa Alliance members, the expected behavior is the one you implemented.
Similarly to the layer2 spec evolution, the intend is to add physical layer elements to the MIC. This reduces the risk of replay attack of the same frame on a different frequency and data rate.

[cjhdev]

One more thing to add to this ticket, I also notice that wor_compute_mic_ack() doesn't add the pad16 field when calculating the MIC.

SWL2001/lbm_lib/smtc_modem_core/lr1mac/src/relay/common/wake_on_radio.c

Line 279 in d05dad5

uint8_t tmp[13];

Perhaps this array should be 16B long and initialized to zero.

[opeyrard]

Hi,
This will be fixed in the next coming release.