osdp decrypt data : Tolerate null length data

[GaspardRig]

Hello, Thanks for this good library ! I think i found a small bug when the function osdp_decrypt_data is call with null length, the function return -1.

[sidcha]

@GaspardRig, a zero length block for decryption is not a valid packet as the encrypted block needs to be terminated by 0x80 which implies, at least one byte of data to be encrypted (and hence one block).

According to the spec, the CP/PD has no data to send for a command, it should use secure message types SCS_15/SCS_16 (instead of SCS_17/SCS_18 which leads to the osdp_decrypt_data call).

Which PD (make, model, firmware version, etc.,) are you using?

[GaspardRig]

Hi @sidcha, sorry for the late anwser, We develop on our existing reader and we integrate your library to support OSDP. We found this limitation on a CP that we use to test our reader. After authentification, the CP always use SCS_17 even if there is no data is the command. The specification is not totally clear about a command with SCS_17 but without data :

Note: this form shall be used with all commands that contain a DATA field.

Do you think that we should not support this type of command and report this default to the CP constructor ?

[IDmachines]

Hi @sidcha, sorry for the late anwser, We develop on our existing reader and we integrate your library to support OSDP. We found this limitation on a CP that we use to test our reader. After authentification, the CP always use SCS_17 even if there is no data is the command. The specification is not totally clear about a command with SCS_17 but without data :

Note: this form shall be used with all commands that contain a DATA field.

Do you think that we should not support this type of command and report this default to the CP constructor ?

Determine firmware version and check against OSDP Verified list if you could:

https://www.securityindustry.org/industry-standards/open-supervised-device-protocol/sia-osdp-verified/sia-osdp-verified-products/

[GaspardRig]

Hi @sidcha, sorry for the late anwser, We develop on our existing reader and we integrate your library to support OSDP. We found this limitation on a CP that we use to test our reader. After authentification, the CP always use SCS_17 even if there is no data is the command. The specification is not totally clear about a command with SCS_17 but without data :

Note: this form shall be used with all commands that contain a DATA field.

Do you think that we should not support this type of command and report this default to the CP constructor ?

Determine firmware version and check against OSDP Verified list if you could:

https://www.securityindustry.org/industry-standards/open-supervised-device-protocol/sia-osdp-verified/sia-osdp-verified-products/

The CP we use to test is not SIA verified

[sidcha]

@GaspardRig, allowing a zero length block may have more severe effect such as introducing a new attack vector that could compromise the session keys and render the whole thing pointless :). Just to be clear, I'm not saying that is the case (I'm no expert in that matter); just that such decisions cannot be made lightly and that I need to go and look at known attack against AES128 to be sure.

And, the spec was clear about not using SCS17/SCS18 for messages that don't have data (at least in prior versions, IIRC). In v2.2, reading the "Note" in SCS15/SCS16 makes it clear that it is intended exclusively for zero length data.

If speaking to the CP vendor is an option, I'd suggest you do that. After all, this might just be a trivial bug that no one has reported yet :).

[sidcha]

@IDmachines, thanks for the list of approved devices, I didn't know one existed. I wonder if we can get library implementations approved.

I'll write an email to the folks SEA.

[GaspardRig]

@GaspardRig, allowing a zero length block may have more severe effect such as introducing a new attack vector that could compromise the session keys and render the whole thing pointless :). Just to be clear, I'm not saying that is the case (I'm no expert in that matter); just that such decisions cannot be made lightly and that I need to go and look at known attack agains AES128 to be sure.

And, the spec was clear about not using SCS17/SCS18 for messages that don't have data (at least in prior versions, IIRC). In v2.2, reading the "Note" in SCS15/SCS16 makes it clear that it is intended excessively for zero length data.

If speaking to the CP vendor is an option, I'd suggest you do that. After all, this might just be a trivial bug that no one has reported yet :).

Hi @sidcha, I asked CP vendor and they will fix the wrong command, they hadn't paid attention before to that point. But good to know : they are using Stid reader presents in the SIA certified list and the reader work with there CP !

[sidcha]

But good to know : they are using Stid reader presents in the SIA certified list and the reader work with there CP !

Shared bugs always live harmony (until one day, they bite us)! Nevertheless, it is worth investigating if this can be safely allowed. I'll get to this when I get some free time.

[sidcha]

I don't have the bandwidth to look into this. Since this is not causing pain to anyone, closing this PR.