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

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

Closed
fraunhofer-iis-bot opened this issue Jul 8, 2019 · 4 comments
Closed
Assignees
Labels

Comments

@fraunhofer-iis-bot
Copy link
Collaborator

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

  1. Blacklist internal JAPI keys (japi_response, japi_response_msg, japi_pushsrv, service, services, success) from being used in request handler functions.
  2. 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" }
...
@fraunhofer-iis-bot
Copy link
Collaborator Author

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.

@fraunhofer-iis-bot
Copy link
Collaborator Author

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

@fraunhofer-iis-bot
Copy link
Collaborator Author

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?

@fraunhofer-iis-bot
Copy link
Collaborator Author

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

closed

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

No branches or pull requests

2 participants