goToMain / libosdp

Implementation of IEC 60839-11-5 OSDP (Open Supervised Device Protocol); provides a C library with support for C++, Rust and Python3
https://libosdp.sidcha.dev
Apache License 2.0
134 stars 71 forks source link

Add support for sequence repeat (CP) and reply resend (PD) features #41

Closed a295656031 closed 1 year ago

a295656031 commented 3 years ago

According to the OSDP protocol the SQN which defines in message control byte, I think if CP receives PD response SQN error or No response from PD,the CP should repeat the command packet,the repeat command packet should be same with the last time command package.

sidcha commented 3 years ago

This can be helpful if there is no response from the PD (although the spec does say that the PD must reply with osdp_BUSY if it cannot respond to a command at the moment). But how does this help when the CP got an invalid sequence number? IMO, sending the same command should result in the same sequence mismatch error again.

a295656031 commented 3 years ago

You may have misunderstood。

PD has replied, but CP may accept the data incorrectly (it may be that the data is wrong in the RS485 transmission process, which causes CP not to accept the data correctly). PD thinks that the data has been successfully sent, but the data received by CP is wrong. In this case, CP should resend the last request.

In osdp_phy.c line 311, seq-repeat reply-resend feature not supported! When cp resend the last time request, the feature not supported.

sidcha commented 3 years ago

In osdp_phy.c line 311, seq-repeat reply-resend feature not supported! When cp resend the last time request, the feature not supported.

Yes. To close this issue both features will be supported.

PD thinks that the data has been successfully sent, but the data received by CP is wrong.

Transmission line errors will trigger the checksum error. Which is also a nice point to re-issue a command. Still not sure why an invalid sequence number needs a command level retry.

So far I can see 2 cases when resending a command with the same sequence number is helpful:

  1. On timeouts, and
  2. On checksum errors

Do you have any other scenarios?

schmida2 commented 1 year ago

@sidcha, I agree to the 2 cases you identified for resending a command with same sequence number. I consider this issue as important as it affects the OSDP built-in error recovery, and RS-485 transmission errors should not be neglected. I am not sure whether the existing command_complete_callback in PD side could be reused as confirmation that the ACU acknowledged the PD reply by correctly incrementing the sequence number, or whether a new callback would be required:

What are your thoughts?

IDmachines commented 1 year ago

The ACU (you should really stop calling these a CP as that has been dropped in the current specification) can resend a command previously sent w/ same sequence number (e.g. it did not process the response) and the PD either processes command, NAK, or BUSY. There are a set of test cases that are being updated to the OSDP Verified repository directly in the very near future that will outline these. BUSY lacks some still and not everone implments this.

sidcha commented 1 year ago

@schmida2 will take a another look at this. A lot of things have changed since this ticket was created so it might be easier to support this now.

The ACU (you should really stop calling these a CP as that has been dropped in the current specification) [..]

@IDmachines, this project will forever call it (the ACU) as a CP. That is how we started and it has gone into public headers and documentation here and in Zephyr RTOS upstream; besides it isn't technically wrong to call it a CP :)

IDmachines commented 1 year ago

Yes, fine as cp can understand the reply, sorry to distract, the coding of retry is a much better topic for discussion,

IDmachines commented 1 year ago

Wrt SC what commands/responses are ok in the clear?

sidcha commented 1 year ago

Wrt SC what commands/responses are ok in the clear?

None of them. The only exception is when a KEYSET command is ACK-ed in plain text because the PD has discarded the current session already in favour of the new key. When in SC all commands should have a secure control block in the header (with or without encrypted data).

IDmachines commented 1 year ago

COMSET, COM, CAP, ID, PDID should be supported both in and out of SC

sidcha commented 1 year ago

Yes, that is the case now. What I meant was you cannot mix modes: when SC is active, a PD will not allow non-SC messages and respond with NACK(6) for it. A CP is free to send all commands in plain text if a secure channel isn't active already.

IMHO, it is a miss in the specification to have a configureable, fully secure mode where PD will not respond to non-secure messages. Nevertheless, LibOSDP already supports such a feature using a build time flag ENFORCE_SECURE that completely locks down a PD.

IDmachines commented 1 year ago

Thanks

sidcha commented 1 year ago

@schmida2 I am looking into this now. Looks to me the re-send and re-response is an implementation detail that application doesn't need to know about. What is you idea behind callbacks to the application?

There are many ways to fix this, none of them seems elegant :). For now, I'm leaning towards an implementation that just processing the entire packet again (as if it was the first time the PD saw it). I cannot think of anything wrong in doing this at the moment and it is less invasive a change.

sidcha commented 1 year ago

Another option is to keep a full copy of the last reply that we constructed last. This makes the response to re-sent commands faster (without going back to the app in some cases) at the expense of keeping a copy in memory which we may never need.

The absolute over over-engineered solution would be to build a way to represent data in some compacted data structure which will be used to rebuild the packet when needed. As cool as it may sound, I'm not inclined to building such an infrastructure for a one-off case like this one.

schmida2 commented 1 year ago

Thanks for the quick solution, and sorry that I missed your question in the evening :-)

I am looking into this now. Looks to me the re-send and re-response is an implementation detail that application doesn't need to know about. What is you idea behind callbacks to the application?

I am thinking about situations like PD log entries being handed over to CP. For the unambiguous deletion of a log entry in PD I want to be sure that it got received by CP. Here I would use the callback to delete the PD log entry. A concrete application is the OSDP extension "Auto Addressing" we are working on; here we also need state information reliably sync'd between PD and CP.

For now, I'm leaning towards an implementation that just processing the entire packet again (as if it was the first time the PD saw it). I cannot think of anything wrong in doing this at the moment and it is less invasive a change.

That means the PD application can see a command twice, right? Can the application know that it was a re-send?

IDmachines commented 1 year ago

Glad you chose to support this, as the test lab for OSDP we expect the CP can resend the same command (sequence) and the PD will process that and CP maintains supervision. We are in the process of updating the test cases here https://github.com/Security-Industry-Association/osdp-verified/tree/master/test-descriptions/test-descriptions-2.5

sidcha commented 1 year ago

For the unambiguous deletion of a log entry in PD I want to be sure that it got received by CP.

I don't thing there is a reliable way to determine if a command was received by the CP other than the observation of sequence number incremented in the subsequent command.

That means the PD application can see a command twice, right? Can the application know that it was a re-send?

It does not. To make the app know it was resent, we have to compromise on a lot of simplicity which I feel isn't worth the effort. Do you have a use case in mind?

schmida2 commented 1 year ago

That means the PD application can see a command twice, right? Can the application know that it was a re-send?

It does not. To make the app know it was resent, we have to compromise on a lot of simplicity which I feel isn't worth the effort. Do you have a use case in mind?

Good question. I could surely construct a case where this matters, but with our current scenarios in the PD I think we can cope with commands coming in twice. If we run into problems I will let you know.