keenlabs / keen-sdk-net

A .NET SDK for the Keen IO API
MIT License
37 stars 24 forks source link

Remove padding from scoped keys #53

Closed hex337 closed 7 years ago

hex337 commented 7 years ago

Scoped keys should no longer be padded.

Also, we should handle the case where the passed in IV is an empty string and throw an exception in that instance.

Example of a key that encodes with padding:

Key Body:

{
  "filters": [{
    "property_name": "ParsedUrl.domain",
    "operator": "in",
    "property_value": ["www.leons.ca","www.leons.com"]
  }],
  "allowed_operations": ["read"]
}

Currently, this ends up generating the following key:

8795B1E827396614F19CD3EE40700C15EE30DD039CE80E30F18CFF1A9F2CEA73232784CAE725C32E84EB184E84919C97C4A1BED5F22CFE6FBBE575E913EBF349E3793B60B032135A2BC6CA13D8DCD3FC695831F751243FC9256F632E4B8F1377C99624D98A59C67964EC798C04B6E2557AF811EBC984AD826FEFEEAA4B89C050C1CA82A541D8B3EA52B97BE05C7623FCB13F891F57E1029ACD2B186F2F877C0149FABF2C560E55C1227C66EB3910E8043368BC02E4BC8FA5E049FD7A24C74FCE031AC4CE88453CB299F26FE0E5C1E5A8AFD5D5F000ACBDC7F422B6A767F68A493CA38F0228AB322CC28D756261DA4E9D7D05C03197FB6A3BC316DC984F7F24C72357F0E9B0B8CFE3BD16D2A6E1EC1593

Which decrypts to (including special chars):

{\r\n "filters": [\r\n {\r\n "property_name": "ParsedUrl.domain",\r\n "operator": "in",\r\n "property_value": [\r\n "www.leons.ca",\r\n "www.leons.com"\r\n ]\r\n }\r\n ],\r\n "allowed_operations": [\r\n "read"\r\n ]\r\n}\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11

The \x11 chars are the padding, which breaks when using the key to run queries against Keen.

The generated key for the same key body should be:

f925bbfcc0afcd641f2808ca83ad474d5854efb6b5a6743cc0ebe9b3b070c0eaf1dee30a40c999bc6fae9fc07609e960f3d3ab7e38471e479144f54306cd746109cd6b382b347072f6d733098685e7669777d9fe63c201bc7a1e71577a1543485bacf3652374fcaf30fc075796b8877ece6a8dc3eedbdfc5820722f6183809c56d62b745984b4ccf0e5b17a17646ea67883c3220ef0ca3f7545caa1fd40544932080f04ce5a1a07fa330f4da5db197b7

masojus commented 7 years ago

Thanks for filing the issue and providing some investigation @hex337! I had some notes locally regarding Scoped Keys. I'm not sure if this is the root cause for Issue #34 or is new issue on top of that. Calling Encrypt() or EncryptString() should produce the same behavior, I believe. When I ran some tests a few weeks ago, I was seeing the same behavior in either case, unlike what Issue #34 seems to indicate.

Another thing we could do is use fewer bits when the JSON doesn't need to be readable (production vs debug mode) and compact the JSON before encrypting, but that won't fix this bug, specifically...just something to look at while we're in that code:

"I tried changing the code in Encrypt() to this so as to slightly compact the JSON string representation:"

var secOptionsJson = JObject.FromObject(secOptions ?? new object()).ToString(Newtonsoft.Json.Formatting.None);
masojus commented 7 years ago

Fixed in commit 0b08716.