-
Notifications
You must be signed in to change notification settings - Fork 279
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
Make a new portable simple standard tox save format #222
Comments
Yet another external dependency? Oh no! Please! |
I'll throw in #112 (comment) |
@isotoxin It seems wrong to try to implement a JSON parser in a library responsible for handling secure communications, as you're just widening attack/bug surface. If, for example, you wanted a JSON config, then better to defer that to a well-maintained library which was meant to handle JSON (and only handle JSON). https://github.com/akheron/jansson/ seems like an ideal candidate for the JSON case; it's just as portable as toxcore, and is even suited for embedded platforms. |
After reading the docs I agree that the current format is horrible and must die. To minimize the stuff needed to implement for a minimal client I think the file should be split into two parts:
The basic part would contain the minimum amount of data to transfer friends(to be decided) across clients in a simple format (maybe even binary) and must not need an external library to parse. The optional part would be appended to the basic part and is JSON (or whatever) encoded. Some keys needed for every client, like aliases, last time online,... Discuss. |
I like the idea of JSON format since it is easy to read (for human) and is cross platform safe by it self. Indeed it requires one additional and external library but atleast in some cases clients can use the same library to handle they own part of config handling (optional part of config). Also, i'm having hard time thinking case where handling of cross platform configuration file, possibly containing rich content, is done without external libraries (JSON, msgpack, protobuf, whatever). I like the sudden6's idea that tox save format could contain two parts. Optional part - Part which can contain client specific information or other information which is not required to get things working. This optional part would be something client can ask from toxcore. Clients could also ask toxcore to store something in this part of config. Any part of optional config should NOT be intepretted by toxcore but core gives it to the client as string. Clients can handle it as they want. With JSON format, encrypting save file is trivial. Toxcore can encrypt save file by using any commonly used symmetric encryption algorithm with user given password. At the same time, encryption is not needed if user does not want to encrypt his save file and in this case toxcore can skip encryption/decryption phase. About security of two part idea. If config would contain two separate parts the basic part would be the only one which is really intepretted by toxcore. From this part toxcore reads all attributes to it's internal structures and this part is constructed back when toxcore saves the save file. For optional part of save file toxcore would introduce ONLY get and put functions for client. Those functions are used by clients to get and give they optional config parts. Toxcore should never reveal any JSON library related functions to client side or try to intepret optional parts. However, toxcore should validate JSON syntax of optional part in order to detect case where client is trying to include invalid config section. If i'm right, any invalid section in JSON makes whole JSON structure invalid. This must be detected by toxcore and invalid inclusion must not be alloved. Syntax validation of JSON should be pretty simple, safe and most probably already available by libraries. Let's think about pros and cons of JSON config file with basic and optional sections:
CONS
After all, I like the idea of JSON format for tox save file. However, implementation of it must be done carefully! More discussion please... |
IIRC avatars aren't stored in the *.tox file right now and I think it would be a really bad idea to store that much data in the config file.
Why?
Yep, that could grow into a huge problem.
Also a possible solution would be to let the client handle JSON parsing with a library by it's choice and just init toxcore with an appropriat config struct/function. But I'm not quite sure this is a good approach, as it's not guaranteed that the output files will be compatible. Another idea is, to scrap the concept of a "Tox Save File" and just provide a "Tox Export File". Instead of saving/loading everything from the save file on every client start, the client would load the data needed for toxcore operation from it's own database/savefile and only provide a way to import/export the data in the specified format for exchange with other clients.
CONS
All in all I think before tackling the how to save stuff, we should first decide on what to save in this file. |
Encrypting/decrypting is trivial regardless of the data format.
Err, no, not a solution, but doing things the wrong way. |
I agree that saving avatars in tox save file might be bad idea but avatar was just an example. I can not think now any binary data we would like to save in save file but in future we might have. Future is a bit hard to predict. :) Basic and optional parts has some security concerns if optional part is something clients can freely add in. That opens few ways how bugy client can set tox core in danger by giving well crafted or bugy data. I belive that this possibility could be minimized by making sure that parts added by client are not intepreted by toxcore and toxcore syntacticaly validates the parts clients want to include.
If this kind basic and optional part separation is too risky at any matter, i still give +1 for JSON format. I have seen way too many b****t XML configs or piles of binary ****. |
See #215 for a related discussion.
The text was updated successfully, but these errors were encountered: