Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protocol Error checking #762

Open
redboltz opened this issue Dec 13, 2020 · 3 comments
Open

Protocol Error checking #762

redboltz opened this issue Dec 13, 2020 · 3 comments

Comments

@redboltz
Copy link
Owner

There are many protocol errors in MQTT spec.
mqtt_cpp doesn't check them enough.
Some of check requires extra cost. For example, some of properties cannot appear in some MQTT control packet.
In order to check it, I need to count them.

My idea is separate receiving and sending. And sending is relatively lower priority because It is application error.
I think that mqtt_cpp should provide sending protocol error checker as optional feature.

Receiving is more important. Because the connection partner could send protocol error MQTT control packet.
I think that protocol error can be checked at process_* functions. It doesn't require extra parse loop because it is only place to parse MQTT control packet.
Maybe adding some protocol_checker class and passed it to process_* functions as a parameter.
protocol_checker has state and it is updated by step by step for each element processed.
When error condition is satisfied, then the connection partner is disconnected and the endpoint got on_error handler call.

@kleunen
Copy link
Contributor

kleunen commented Dec 13, 2020

Maybe pass a list of protocol checkers, so you can have multiple protocol checks running ? Otherwise you create a protocol_checker which takes a list of protocol_checkers ..

And I think resending publish/pubrec/pubcomp can be a protocol check as well? I would not mind disconnecting these rogue clients.

@redboltz
Copy link
Owner Author

redboltz commented Dec 13, 2020

My image is as follows:

MQTT control packet
+----------+----------+----------+----------+----------+
|          |          |          |          |          |
+----------+----------+----------+----------+----------+
process_a   process_b  process_c  process_d  process_e

protocol_checker checker { publish }; // set MQTT control packet type.

void process_a(..., protocol_checker& checker) {
    if (!checker.some_flag(...)) {
        // protocol_error. DISCONNECT and call on_error()
    }
    ....
    process_b(checker); // pass the checker to the next process

}

When the control packet type is decided, then just create for the checker for that type.

I didn't consider yet about abstraction.

@redboltz
Copy link
Owner Author

I think that protocol checking on individual MQTT control packet and a series of send/receive of MQTT control packet should be separated. My first target is former. The lifetime of the checker ends on individual packet parsing is finished.

The latter is difficult to implement. Maybe endpoint needs to have state machine. And need to check the combination of send/receive packet. Especially the client uses async_ APIs, API call timing and actual sending timing is different.
If we implement it invalidly, the connection is disconnected by false positive check.
So I postpone it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants