On 11/14/2024 8:36 AM, Dr. Greg wrote: > On Wed, Nov 13, 2024 at 10:57:05AM -0800, Song Liu wrote: > > Good morning, I hope the week is going well for everyone. > >> On Wed, Nov 13, 2024 at 10:06???AM Casey Schaufler <casey@xxxxxxxxxxxxxxxx> wrote: >>> On 11/12/2024 5:37 PM, Song Liu wrote: >> [...] >>>> Could you provide more information on the definition of "more >>>> consistent" LSM infrastructure? >>> We're doing several things. The management of security blobs >>> (e.g. inode->i_security) has been moved out of the individual >>> modules and into the infrastructure. The use of a u32 secid is >>> being replaced with a more general lsm_prop structure, except >>> where networking code won't allow it. A good deal of work has >>> gone into making the return values of LSM hooks consistent. >> Thanks for the information. Unifying per-object memory usage of >> different LSMs makes sense. However, I don't think we are limiting >> any LSM to only use memory from the lsm_blobs. The LSMs still >> have the freedom to use other memory allocators. BPF inode >> local storage, just like other BPF maps, is a way to manage >> memory. BPF LSM programs have full access to BPF maps. So >> I don't think it makes sense to say this BPF map is used by tracing, >> so we should not allow LSM to use it. >> >> Does this make sense? > As involved bystanders, some questions and thoughts that may help > further the discussion. > > With respect to inode specific storage, the currently accepted pattern > in the LSM world is roughly as follows: > > The LSM initialization code, at boot, computes the total amount of > storage needed by all of the LSM's that are requesting inode specific > storage. A single pointer to that 'blob' of storage is included in > the inode structure. > > In an include file, an inline function similar to the following is > declared, whose purpose is to return the location inside of the > allocated storage or 'LSM inode blob' where a particular LSM's inode > specific data structure is located: > > static inline struct tsem_inode *tsem_inode(struct inode *inode) > { > return inode->i_security + tsem_blob_sizes.lbs_inode; > } > > In an LSM's implementation code, the function gets used in something > like the following manner: > > static int tsem_inode_alloc_security(struct inode *inode) > { > struct tsem_inode *tsip = tsem_inode(inode); > > /* Do something with the structure pointed to by tsip. */ > } > > Christian appears to have already chimed in and indicated that there > is no appetite to add another pointer member to the inode structure. > > So, if this were to proceed forward, is it proposed that there will be > a 'flag day' requirement to have each LSM that uses inode specific > storage implement a security_inode_alloc() event handler that creates > an LSM specific BPF map key/value pair for that inode? > > Which, in turn, would require that the accessor functions be converted > to use a bpf key request to return the LSM specific information for > that inode? > > A flag day event is always somewhat of a concern, but the larger > concern may be the substitution of simple pointer arithmetic for a > body of more complex code. One would assume with something like this, > that there may be a need for a shake-out period to determine what type > of potential regressions the more complex implementation may generate, > with regressions in security sensitive code always a concern. > > In a larger context. Given that the current implementation works on > simple pointer arithmetic over a common block of storage, there is not > much of a safety guarantee that one LSM couldn't interfere with the > inode storage of another LSM. However, using a generic BPF construct > such as a map, would presumably open the level of influence over LSM > specific inode storage to a much larger audience, presumably any BPF > program that would be loaded. > > The LSM inode information is obviously security sensitive, which I > presume would be be the motivation for Casey's concern that a 'mistake > by a BPF programmer could cause the whole system to blow up', which in > full disclosure is only a rough approximation of his statement. > > We obviously can't speak directly to Casey's concerns. Casey, any > specific technical comments on the challenges of using a common inode > specific storage architecture? My objection to using a union for the BPF and LSM pointer is based on the observation that a lot of modern programmers don't know what a union does. The BPF programmer would see that there are two ways to accomplish their task, one for CONFIG_SECURITY=y and the other for when it isn't. The second is much simpler. Not understanding how kernel configuration works, nor being "real" C language savvy, the programmer installs code using the simpler interfaces on a Redhat system. The SELinux inode data is compromised by the BPF code, which thinks the data is its own. Hilarity ensues. > > Song, FWIW going forward. I don't know how closely you follow LSM > development, but we believe an unbiased observer would conclude that > there is some degree of reticence about BPF's involvement with the LSM > infrastructure by some of the core LSM maintainers, that in turn makes > these types of conversations technically sensitive. > >> Song > We will look forward to your thoughts on the above. > > Have a good week. > > As always, > Dr. Greg > > The Quixote Project - Flailing at the Travails of Cybersecurity > https://github.com/Quixote-Project