✨ Proposal: Specify first level of all JAPI responses #13

Closed fraunhofer-iis-bot closed 1 year ago

In GitLab by @jannismain on Jul 8, 2019, 16:50

Current Implementation

Right now, the format of JAPI responses is partly specified and partly up to japi_request_handler, which is implemented for each server application:

JAPI command handled by libjapi:

-> { "japi_request": "japi_pushsrv_list"}
<- { "japi_response": "japi_pushsrv_list", "services": [ "push_temperature", "push_counter" ] }

_JAPI command handled by application-defined response handler (but still including japi_response key):_

-> { "japi_request": "get_temperature" }
<- { "japi_response": "get_temperature", "temperature": 27.0, "unit": "celsius" }

_Another command handled by application-defined response handler (without japi_response key):_

-> { "japi_request": "japi_pushsrv_subscribe", "service": "push_temperature" }
<- { "temperature": 39.91664810452468, "japi_pushsrv": "push_temperature" }
<- { "temperature": 39.73847630878195, "japi_pushsrv": "push_temperature" }
<- { "temperature": 39.46300087687415, "japi_pushsrv": "push_temperature" }
<- { "temperature": 39.09297426825681, "japi_pushsrv": "push_temperature" }
...

Problems

A badly (or maliciously) designed request handler could use JAPI package identifiers listed above to trick the parser into doing things he shouldn't be doing.

Example of a forged response:

-> { "japi_request": "japi_pushsrv_list", "services": [ "push_temperature", "push_counter" ] }
-> { "japi_request": "japi_pushsrv_subscribe", "service": "push_temperature" }
<- { "japi_response": "japi_pushsrv_subscribe", "service": "push_temperature", "success": true }
<- { "temperature": 39.91664810452468, "japi_pushsrv": "push_temperature" }
<- { "temperature": 39.73847630878195, "japi_pushsrv": "push_temperature" }
// forged response to advertise service that doesn't exist:
<- { "japi_response": "japi_pushsrv_list", "services": [ "push_temperature", "push_counter", "push_bitcoin_miner" ] }
<- { "temperature": 39.09297426825681, "japi_pushsrv": "push_temperature" }
...

If the client-side parser has no logic to match responses to initiated requests, it will now think, there is a registered push_bitcoin_miner service, which there isn't really.

But even if the client-parser only handles responses with matching requests, a push service package (with a forged response) might always arrive in between request and authentic response.

Possible Solutions

Blacklist internal JAPI keys (japi_response, japi_response_msg, japi_pushsrv, service, services, success) from being used in request handler functions.
Strictly specify first level of JAPI response messages (request handler values would be attached on the second level )

Proposed Solution

Specify the first level of all JAPI responses to guarantee, that the same parser can parse all JAPI response packages.

A specification could look like this:

Regular JAPI packages:

{
    "japi_response": str,
    "value": any,
    "success": bool
}

JAPI Push Service Update Packages:

{
    "japi_pushsrv": str,
    "value": any
}

Here, the application defined fields (any) are separated from any internal JAPI fields ("value", "japi_pushsrv") through hierarchy, rendering the forged response above meaningless:

Example of a forged response:

...
<- { "value": 39.91664810452468, "japi_pushsrv": "push_temperature" }
<- { "value": 39.73847630878195, "japi_pushsrv": "push_temperature" }
// forged response to advertise service that doesn't exist:
<- { "value": { "japi_response": "japi_pushsrv_list", "services": [ "push_temperature", "push_counter", "push_bitcoin_miner" ] }, "japi_pushsrv": "push_temperature" }
// client is indifferent to this attack:
-> 凸(￣ヘ￣)
<- { "temperature": 39.09297426825681, "japi_pushsrv": "push_temperature" }
...

In GitLab by @fraunhofer-iis-anon on Jul 8, 2019, 17:52

I suggest ssl-certs to secure communication. It's not necessary, to change the protocol. And it secures other aspects like integrity(which would still be a vulnerability even with the changes) etc. of the messages, too.

In GitLab by @jannismain on Jul 10, 2019, 07:47

AFAIK, SSL certs only verify the server is who he says he is, which makes Man-in-the-Middle attacks harder to execute. Transport security is a valid point and important, as soon as libjapi connects to the WWW, but the issue with he spec I described above, is not limited to MitM vulnerability.

I was rather talking about the general ability for libjapi push-services / request handlers to send packages, that are indistinguishable from libjapi-internal packages.

Introducing hierarchy in the response package data layout would make this an impossibility, which results in...

a better-defined API (as first layer of response packages would be guaranteed)
easier client-side parsers
less chance of bugs due to naming conflicts

In GitLab by @cstender on Jul 11, 2019, 13:15

Yes, SSL is a completely different story. TLS would be is interesting but can also be provided by the WebServer Proxy. So this is low prio atm.

After talking with Thomas we think that it makes sense to implement your proposed format with some minor modifications.

1) Instead of value we would recommend the keyword data.

2) It should be discussed if we really want the success key in the first json hierarchy level. IMO this is user content and should be part of the user data blob.

Comments? Further suggestions?

In GitLab by @cstender on Aug 16, 2019, 15:48

closed

Fraunhofer-IIS / libjapi