MIC calculation for MSG down

[tothero]

Hi,
the function "lw_mtype_msg_up" is used for parsing up & down messages. a little bit confusing but ok. But while calculating the MIC you use in BOTH cases the fcnt32 of up messages.

fcnt16 = ((uint32_t)msg[LW_DATA_OFF_FCNT+1]<<8) + msg[LW_DATA_OFF_FCNT];
fcntlsb = (uint16_t)cur->ufcnt;
fcntmsb = (uint16_t)(cur->ufcnt>>16);
if(fcnt16<fcntlsb){
    fcntmsb++;
}
lw_key.fcnt32 = ((uint32_t)fcntmsb<<16) + fcnt16;

lw_msg_mic(&mic, &lw_key);
if(mic.data != plmic.data){
    if(lw_key.fcnt32 == fcnt16){
        return LW_ERR_MIC;
    }
    lw_key.fcnt32 = fcnt16;
    lw_msg_mic(&mic, &lw_key);
    if(mic.data != plmic.data){
        return LW_ERR_MIC;
    }
}

Is this your intention or a bug?
I think the current implementation will give an error for framecounts > 16 bit. For down messages you should select the global 32bit down fcnt,

[JiapengLi]

Hi @tothero
Thank you for your feedback. Agree with you parse downlink message through lw_mtype_msg_up is messy, the idea is to reuse APIs, because down/up messages are very similar. I'll see if there is clean way to do it, maybe rename the api to a general one like lw_mtype_msg and call it in lw_mtype_msg_up and lw_mtype_msg_down functions.

I didn't do test again. The code is intended to designed like that. Design it in that way the parser can compatible with counter roll back (16 bits only, 32bits counter roll back not supported).

fcntmsb = (uint16_t)(cur->ufcnt>>16);
if(fcnt16<fcntlsb){
    fcntmsb++;
}

This piece of code is for counter overflow check. So it should support 32bits counter.

[tothero]

Thanks for your response. I will see if a can make a PR for you after doing some testing at this point. For LoRaWAN 1.1 there will be only 32 Bit Framecounter no 16 Bit are allowed anymore.

Btw i made very simple lib just for parsing based on parts of your code. check it out here:
https://github.com/Lobaro/util-lorawan-packets

Maybe or not it could make sense to integrate this into your tool to add a layer of separation between parsing and other stuff your tool does.

Greetings
Theo