-
Notifications
You must be signed in to change notification settings - Fork 72
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
[RFC] Add a way to say that certain data is stored in system state only and not in the clixon database #534
base: master
Are you sure you want to change the base?
[RFC] Add a way to say that certain data is stored in system state only and not in the clixon database #534
Conversation
Another note on this. It would be more general, perhaps, to add callbacks in the plugin that would tie in at the same points (where the data is added from the system state and where the data is removed) and pass in the database name and cxobj tree. You would end up with the same code for the most part, just in different places, but that would let you do other things, perhaps. I don't know what other things, though. |
Actually, this really isn't feasible. You need to know, in the database, if the system state data is loaded into the XML tree. You don't want an operation to come in and operate on a database and then remove the system state data if the system state data was already in the tree for some other reason. |
The docker-alpine-test-2 failure should be fixed by 9ee5544 please rebase |
Some input/thoughts in no specific order:
In this way the data could be marked in a declarative way, otherwise it seems to me a more cumbersome function calls (eg in the start plugin call) needs to be made to make to xmldb_add_stateonly().
|
This is more than just sensitive data. This is data where the system state is the only database. For instance, the list of users is held in /etc/passwd, and /etc/shadow. You don't store any of that in the clixon database, every time you need it you load it from the system state. I want this for a number of reasons:
I agree that stateonly is not the optimal name. I just haven't come up with one. Maybe systemstateonly, but that's too long.
I have this working in the python interface now with the ietf-system model. That's a bit complicated, though, as you have the python interface, the transaction framework, and then the implementation on top of the transaction framework. I agree that a simple use case and example would be ideal. I'll see what I can come up with.
The idea here is the system state is the only database for the portions of the tree that are stateonly. Except for the temporary time when a candidate is being worked on, when the system state is in the candidate, and an even shorter time when the system state is in the running database when doing the validate and commit processing. The sort-of-pseudo code for this is:
This is exactly what I want. This way, the system state data only gets loaded right when it's needed, and gets removed after it is no longer needed. So if it changes between transactions, everything is still good. Again, the main philosophy of clixon is that it is the master database and it make the system conform to its database. That's best for most data. However, for some cases, it's better to have the system data be the master database One problem with the current implementation is the running database is loaded from the system state later than the candidate, so if something happened to change the system state between them, that change would only be in the running database. It might be better to save the system state data and apply it later to the running database. It may not be that important, tough.
It's mostly orthogonal, yes, and may be useful for other purposes. It just keeps the candidate database, which could hold keys and such while a client is working on it, off the disk. This is purely a security thing from my point of view.
That was my original thought and what you suggested. I had some issues, though:
That was my reasoning, obviously from limited understanding.
Yes, the data in transit will be protected with TLS/SSH, and the data will be in memory (though not on disk) temporarily. That's normal. It would be better if the memory was scrubbed after use. That would be relatively easy to add to the removal of the subtree, but then there's the buffers holding the transport data, both in the client interfaces and in the backend. I'm ok with not scrubbing at this point, and it's something that can be fixed later. |
Two questions here.
As you say, one still needs to do a (functional) registration and write code to read/write stateonly data. But in my view it is easier to maintain across updates etc, if the dependencies are stated in direct association with the YANG. Suppose the YANG revision changes and with it some of the stateonly fields. In functional code one needs to keep track of revisions with conditionals. In YANG-annotated code this is done inside the new YANG (or an upgraded server YANG).
|
Ok, system-only-config is better, I agree. Would sysonly_config be good?
I'll see what I can do on this. All my work has been in python, so I'll have to do something in C.
Correct, that is expected.
Yes. You wouldn't want to use this for data that is very large. Essentially, you are polling it every time you do a transaction. And there may be surprising side effects.
I considered this, and I think I tried it, but I ran into issues. In edit-config, you need to know if the current system config is loaded into the candidate database, so you need some flag for that. Since you want to re-read the system config every time you start a transaction, it was easier to remove it when the transaction is done and when the next edit-config happens for a new transaction to read it back in.
Ah, good.
If you implemented it in db_dump, then it would be efficient on the write side. But if not, then the remove operation would have to scan the entire database every time to find all these items and remove them. And in either case to read the new values back in it has to scan the entire database to find them. On the read side, it has to then locate and call the registered code to read that piece of data in. I don't see another way.
I am not seeing how annotating it in the YANG makes maintenance any easier. If the YANG version changes the system config only data, you would still need conditionals to know how to handle the data based on the version, or keep the code in sync with the version. If you can somehow change things in the YANG and don't have to change things in the functional side, that would be cool, but I don't see how. And if there's no efficiency issues, that would be fine. |
7dc8ee8
to
e9aa452
Compare
Add plugin_rpc_err(), which works something like clixon_err, but it saves error information from a plugin for reporting rpc-errors that need to be returned. Signed-off-by: Corey Minyard <corey@minyard.net>
Signed-off-by: Corey Minyard <corey@minyard.net>
Don't store "candidate" or any other database except "running" on disk, only keep it in memory, if CLIXON_TMPDB_VOLATILE is true. This, along with the stateonly handling, can be used to avoid storing sensitive data in the clixon databases. Signed-off-by: Corey Minyard <corey@minyard.net>
Signed-off-by: Corey Minyard <corey@minyard.net>
Signed-off-by: Corey Minyard <corey@minyard.net>
Using a string for a path didn't work, when adding path elements to the tree using it you need to add a namespace in certain places. So take an XML path instead and use the namespaces from that. Signed-off-by: Corey Minyard <corey@minyard.net>
It can get there from the user or from defaults, but we don't want it. Also add some documentation on some functions. Signed-off-by: Corey Minyard <corey@minyard.net>
If passing in an XML tree to the stateonly functions, completely ignore the database, don't do anything to it. Signed-off-by: Corey Minyard <corey@minyard.net>
The database must be sorted afer reading in the data from xmldb or other operations won't work correctly. Add the stateonly data to the running database during a commit operation so the compare will work correclty. Signed-off-by: Corey Minyard <corey@minyard.net>
The data is expected to still be in the candidate database unless it is discarded, so don't delete it. Signed-off-by: Corey Minyard <corey@minyard.net>
e9aa452
to
a3daabe
Compare
The candidate commit function need to always clean the candidate database at the end. Signed-off-by: Corey Minyard <corey@minyard.net>
This adds two things:
An option to not store any database on disk except the runtime database and startup database.
A way to tell clixon that certain data is not to be stored in the database, and instead will be fetched from the system state when required, and removed before storage to disk.
This should make handling security-sensitive data and data that can change dynamically under clixon easier.
This is just an RFC. I'm not too enamored with the "stateonly" name, but I haven't worked much on a better name. Also, I'm sure some of the things here could be done more efficiently and/or cleanly.