On sexta-feira, 15 de julho de 2016 13:57:39 PDT Abhishek Sharma wrote:
> Hi All
>
> Request your feedback on CoAP->HTTP proxy feature that we developed and is
> under review here: (https://gerrit.iotivity.org/gerrit/#/c/7581/) The
> detailed information can be found on wiki page here:
> (https://wiki.iotivity.org/coap-http_proxy) And any issues or concerns can
> be raised on the JIRA issue here:
> (https://jira.iotivity.org/browse/IOT-1128)
Hello Abishek
I like the concept. It would allow constraind clients to be able to obtain
resources from the cloud without having HTTP clients and without having to
expose themselves to the outside world. This would allow the vendor of the
product offering the proxy to focus on hardening that particular node against
outside hacks, instead of distributing the need for hardening across all
devices.
In addition, the network administrator could configure via ACLs which devices
in the network are allowed to access the proxy in the first place. The
immediate conclusion from this is that this functionality should also support
an ACL for outgoing services: what web services is the paricular client
allowed to access? Using your example on the website, the thermostat would be
allowed to access a pre-configured weather service and possibly the
manufacturer's website to check for updates, but not "h4x0r.com". I would say
that your design should include this outgoing ACL functionality.
As a consequence of having configurable settings, the proxy itself needs to be
a resource that accepts configuration, similar to what we're doing to the
bridge device. Maybe this doesn't need to be an OCF resource, but could
instead simply be a webpage on the device itself.
About your proposed architecture:
I really, really don't like it.
1) CBOR-JSON mapping
First of all, this is extremely fragile. It depends on how we currently use
CBOR today, which may or may not be the way we'll do it in the future. We're
currently using CBOR to transport JSON data, so we can transform back to JSON
with no loss of data, but are we going to continue doing that in the future?
For example, if the CBOR data contains Byte Text (binary data), how is it
represented in JSON? Moreover, the implementation is fragile because it
depends on the limitations of OCRepPayload.
The second issue is the mapping at all. I would argue that the client of the
proxy should be able to get the data in any format that they want. For
example, many weather services provide their data in XML, not JSON. Limiting
the proxy support for JSON only is short-sighted. Client should be able to
download small binary blobs of data, images, XML, text, JSON (without
parsing!), CBOR, etc. Please pass the data through, unchanged.
That immediately leads to the conclusion that this feature needs to support
Block Transfer.
2) Manual HTTP client
If this code is going to be accessing arbitrary services on the Internet, we
should not deploy our own HTTP clients. Having done that myself for Qt, I can
tell you it's A LOT of work. It's not something we want to maintain long-term.
So I'm going to draw the line here and say:
This feature MUST use a third-party HTTP client library
That's non-negotiable.
Just look at how your current implementation does not support HTTPS properly
(or at all, since I couldn't find it). HTTPS support is MANDATORY. Do you
really want to write the code for it? TinyDTLS is not going to cut for it.
Don't forget checking the SSL certificates based on the OS's store, checking
CN, etc.
One more: where's your HTTP/2.0 support?
I pointed this out in the JIRA and you responded that size was a concern. No,
it's not. First of all, IoTivity does not run on constrained devices. Any
device where it can run will be able to accommodate libcurl. Second, the whole
point of a CoAP-HTTP Proxy is that a *bigger* device can provide a service for
a smaller one.
Implementation:
A) Location in the source code
This is a service. It should not be in "resource/".
It can be either in service/ or it could be in a separate repository. You can
develop it as a standalone application or as a loadable plugin to IoTivity.
B) Quality of the source code
Within two hours of reviewing your code, I found numerous memory leaks, buffer
overruns and possible out-of-bounds access, not to mention code that has
clearly never been tested. It has use of untrusted data in memory allocation,
which is a clear verctor for remote attacks.
Given this, I really recommend rewriting this in C++ or even in a higher-level
language (Python, Java, JavaScript, whatever). Given the amount of scrutiny
that the source code is going to need to go through, I recommend placing it
outside of iotivity.git, so that we can iterate it faster and quicker. It
would allow you to make your first commits soon, while we work on static code
analysis, security audits, etc., while others work on other features such as
the control resource.
--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
