telefonicaid / fiware-orion

Context Broker and CEF building block for context data management, providing NGSI interfaces.
https://github.com/telefonicaid/fiware-orion/blob/master/doc/manuals/orion-api.md
GNU Affero General Public License v3.0
210 stars 265 forks source link

[BUG ]in POST v2/subscriptions, in entities/idPattern field not plain ascii chars are allowed #1976

Open iariasleon opened 8 years ago

iariasleon commented 8 years ago

in POST v2/subscriptions, in entities/idPattern field not plain ascii chars are allowed.

Dataset

      | type       |
      |------------|
      | habitación |
      | españa     |
      | barça      |

subscription request

POST http://localhost:1026/v2/subscriptions
Content-Type: application/json
Fiware-Service: test_entities_id_pattern
Fiware-ServicePath: /test
 {"notification": {"callback": "http://localhost:1234", "attributes": ["temperature_0"]}, "expires": "2016-04-05T14:00:00.00Z", "subject": {"entities": [{"idPattern": "barça"}], "condition": {"attributes": ["temperature"]}}}

subscription response

http code: 201
date: Thu, 31 Mar 2016 13:05:45 GMT
connection: Keep-Alive
content-length: 0
location: /v2/subscriptions/56fd20a9a093059d431e55e1

doc in mongo

{ "_id" : ObjectId("56fd20a8a093059d431e55df"), "expiration" : NumberLong(1459864800), "reference" : "http://localhost:1234", "throttling" : NumberLong(0), "servicePath" : "/test", "entities" : [ { "id" : "habitación", "isPattern" : "true" } ], "attrs" : [ "temperature_0" ], "conditions" : [ { "type" : "ONCHANGE", "value" : [ "temperature" ] } ], "expression" : { "q" : "", "geometry" : "", "coords" : "", "georel" : "" }, "format" : "JSON" }
{ "_id" : ObjectId("56fd20a9a093059d431e55e0"), "expiration" : NumberLong(1459864800), "reference" : "http://localhost:1234", "throttling" : NumberLong(0), "servicePath" : "/test", "entities" : [ { "id" : "españa", "isPattern" : "true" } ], "attrs" : [ "temperature_0" ], "conditions" : [ { "type" : "ONCHANGE", "value" : [ "temperature" ] } ], "expression" : { "q" : "", "geometry" : "", "coords" : "", "georel" : "" }, "format" : "JSON" }
{ "_id" : ObjectId("56fd20a9a093059d431e55e1"), "expiration" : NumberLong(1459864800), "reference" : "http://localhost:1234", "throttling" : NumberLong(0), "servicePath" : "/test", "entities" : [ { "id" : "barça", "isPattern" : "true" } ], "attrs" : [ "temperature_0" ], "conditions" : [ { "type" : "ONCHANGE", "value" : [ "temperature" ] } ], "expression" : { "q" : "", "geometry" : "", "coords" : "", "georel" : "" }, "format" : "JSON" }

expected response

http code: 400
{"error":"BadRequest","description":"Invalid characters in entities idPattern"}
iariasleon commented 8 years ago

LGTM, don´t apply in idPattern field

fgalan commented 8 years ago

Sure? I mean, idPattern should allow characters such as ( that are part of the general forbidden chars in Orion (as these chars are needed by the regex syntax). However, I understand that "non ascii" susch as á or ö shouldn't be allowed.

(Reopened while we are still discussing this)

crbrox commented 8 years ago

Maybe this restriction about forbidden chars is a little too strong for idPattern, a regular expression. If a pattern like olé is used, it will never be matched, just the same that happens when an inexistent attribute with only plain ASCII characters is used. Generally, with a regular expressions, the user should check that it will be fired as she expects, and with non plain ASCII chars it will never do as we don't allow the creation of attributes with forbidden chars. Also as idPattern never occurs in URLs, table or database names, etc, the reasons for forbidding non-ASCII characters in this case are not so important and could be relaxed for this field

fgalan commented 8 years ago

Also as idPattern never occurs in URLs

Actually, it may occur in query entities operation, i.e. GET /v2/entities?idPattern=.... However, that's a different story (the present issue is about subscriptions).

kzangeli commented 8 years ago

For this issue to be implementable, we need to define the set of allowed characters for idPattern. Perhaps for ìd` as well. Do we really want to restrict by not allowing 'exotic chars'? I understand we should not allow 'dangerous' chars that make the broker vulnerable, but not allowing ñ, ç and é etc ... I don't see any reason for that.

So, in order for this issue to go forward, I believe we need to discuss this.

iariasleon commented 8 years ago

I agree with Ken, in this specific case (subscription in payload) but in another cases (url, etc), we can discuss in other issues.

Depend of the users create the regular expression correctly and I do not think that "not plain ascii chars" will be a security risk or maybe I'm wrong.

fgalan commented 8 years ago

We should define the "forbidden chars for id patterns" set (applicable at least to idPattern and typePattern) starting with the "Field syntax restrictions" set defined in the NGSIv2 spec, then adding/removing as needed taking into account the nature of the regex and the security considerations.