Hi, 2017-05-18 23:49 GMT+02:00 Paul Moore <paul@xxxxxxxxxxxxxx>: > My apologies to you and Sebastien for not reviewing these patches sooner. It is ok, no problem. Thanks for all the advice from you and Stephen. I will try to take all this into account. As I understand it, I should not give the choice to allocate or not the string returned by security_policydb_brief(). The initial reason for this was that Lustre client code is expected to retrieve policy brief info hundreds or thousands of times per second, so saving on memory allocation would make sense. So if security_policydb_brief() necessarily allocates memory for the string returned, and I appreciate it helps maintenance and avoids complexity, it should not be called so often. One way to tackle this is to rely on the notification system: Lustre client code would call security_policydb_brief() only when it gets a change notification, and stores the current policy brief info internally. Another way could be to add another hook to check policy brief info validity. It would take a string as an input parameter, and return 0 if it matches the current policy. So Lustre client code would systematically call this hook, and only call security_policydb_brief() when the policy has changed, to store the current value internally. I have recently identified a new need from Lustre client code. We need to protect against the case where the policy is changed or set in permissive mode, and then set back to its previous state, to workaround policy check as carried out on server side based on policy brief info sent by client. In this scenario, the policy would only be the expected one by the time the client sends a request to the server (for instance a file open request), but not after that when SELinux actually checks the permissions on the client (via security_file_open() in this example). A solution to address this could be to add a new parameter to security_policydb_brief() hook, in the form of a pointer to an integer giving the current sequence number of the policy. That would complement the policy brief info, with the notion of change to the policy. I do not think it is desirable to include the sequence number in the policy brief info, as it is not the essence of the policy. Now with this sequence info in mind, the new hook to check policy brief info validity would only need to check the sequence, instead of the policy brief string. The current value of the sequence info should be stored by Lustre internally, and checked after SELinux permission checks. If a change is detected, Lustre client must stop normal processing and return an error for the current request. Thanks, Sebastien.
