Hi Simon, On Mon, 2021-07-26 at 09:49 +0000, THOBY Simon wrote: <snip> > > A new builtin policy could be defined based on the new "appraise_hash" > > option or simply a flag (e.g. ima_policy=). > > I have started to take a look at what I might do in that regard. I think your > idea to filter writes with the ima policy is definitely better than my secure > boot "hack". However I still wonder the form this might take to be correct. > > IMHO we cannot simply consider whether there is one rule in the policy that uses the > 'appraise_hash' option, and apply that hash algorithm policy everywhere: we do not > want to constrain files that rule doesn't impact. > e.g. if a rule constrains every file owned by root to be valid only if the IMA > signature was generated with sha256, another user shouldn't be constrained by that > rule. Consider this policy: > appraise func=MODULE_CHECK appraise_hash=sha256 > appraise func=BPRM_CHECK fowner=0 > > Here we do not want to constrain xattr writes to arbitrary files because we want > more insurances on the the kernel modules. > This would be a behavior hard to understand for users, and probably lead to > unexpected system breakage if someone were to upgrade their ima policy and change the > 'appraise_hash' value, because it would apply to files that the user didn't expect > to be impacted. > > For this reason, I believe there must be a way for the setxattr hook to determine if a > file should be affected by the hash policy or not. > > At first I thought about using 'ima_get_action' in the 'ima_inode_setxattr' hook > to extract the rule that matches the file, verify if there is a list of allowed > hash algorithms in that rule and apply the hash restriction to the xattr being > written. > But then I hit a significant setback: as I understand it, IMA cannot > detect if a given rule apply to a file *outside* of trying to executing that rule. > Let me explain what I mean with an example. Let us suppose we have the following > ima policy: > appraise func=BPRM_CHECK euid=0 appraise_hash=sha1,sha256 # (1) > appraise func=FILE_MMAP fowner=0 mask=MAY_EXEC appraise_hash=sha256,sha512 # (3) > appraise func=FILE_MMAP euid=0 mask=MAY_EXEC appraise_hash=sha384 # (3) > > (I agree that such a diversity of hashes is quite implausible on a single system > in practice, but I also think it best to try to think of degenerate usecases > before implementing that feature, as users will tend to rely on them) > > When a user try to update the ima hash (or ima signature) of a file, how can we > know the hash algorithms that the user can use ? How do we know if the users uses > a rule or another, and thus the algorithm that should apply ? > There is no one-to-one mapping between files and rules in IMA (I understand that is not > at all the philosophy of IMA), so the answer is "We cannot". > Worse, two rules could both apply to the same file (e.g. he could both mmap the dynamic > loader and run it directly, so rules (1) and (2) would both apply. > Except they do not use the same appraise_hash parameter! > So the step "extract the rule that matches a file" is not possible, and I need to get > back to the drawing board. > > Technically, we could try every possible combination of mask/func to determine which > would apply to the file whose xattr is being updated, but that would be absolutely > terrible performance wise, and it would still have bad semantics: > - either we would choose the first rule that match, and in that case the order of the > policy (and the order of our exhaustive search) would impact the resulting algorithms > allowed; > - or we could consider the intersection of hash algorithms allowed in each rule > (it might be null) or their union (it might be overly broad and we might choose > an algorithm not part of the intersection, thus the will will not be usable in > some situations). > > In short, I believe both situations would be a nightmare, for user experience, > performance, maintainability and probable the sanity of maintainers/code reviewers. Agreed. > > I think one possible way of getting out of this conundrum would be to extend the ima > policy further by adding a new value for the 'func' policy option (something like > WRITE_XATTR_HASH maybe ?). In that mode, the 'mask' option would have no effect, the > appraise_hash parameter would be mandatory, and any file matching this policy would > have the corresponding 'appraise_hash' policy enforced. > This might give policy rules of the following sort: > appraise func=BPRM_CHECK euid=0 appraise_hash=sha1,sha256 > appraise func=FILE_MMAP fowner=0 mask=MAY_EXEC appraise_hash=sha256,sha512 > appraise func=FILE_MMAP euid=0 mask=MAY_EXEC appraise_hash=sha384 > appraise func=WRITE_XATTR_HASH fowner=0 obj_type=bin_t appraise_hash=sha256 > > The first three rules would just impact executions/mmap()s, and the last one > would restrict xattr writes. > > I agree that would add quite a bit of complexity (and a performance hit to check > if a IMA policy matches) to the setxattr hook, that I don't see yet another way out > of this issue. > > Please let me know what you think, I certainly would prefer it if someone came up > with a much simpler option that I could then implement. Implementing complete setxattr policy rules, including LSM labels, would be the safest, but as you said, it would impact performance. Most systems could have a simpler rule to limit the hash algorithm(s). For example, appraise func=SETXATTR_CHECK appraise_hash=sha256 appraise func=SETXATTR_CHECK appraise_hash=sha256,sha384,sha512 Without a SETXATTR_CHECK rule, the default would be to limit it to the configured crypto algorithms. (The LSM hook is named security_inode_setxattr.) thanks, Mimi
