On Thu, Mar 7, 2013 at 11:56 PM, Vivek Goyal <vgo...@redhat.com> wrote: > On Thu, Mar 07, 2013 at 07:53:50PM +0200, Kasatkin, Dmitry wrote: > > [..] > > Hi Dmitry, > >> Sorry if missed something from this lengthy thread and I repeat something. >> >> I have not noticed what functions you propose to export. > > Actually I have not come up with functions yet. I have yet to write > the code. But I was thinking something along the lines of > verify_signature(). > >> >> But for your use case you need to know if file was signed and >> signature was fine, right? > > Right. > >> So you want to export a function which returns, for example >> "iint->flags & IMA_DIGSIG". > > Not sure about that but I think you are referring to your patch > of also exporting the iint->DIGSIG by setting a security flag > LSM_UNSAFE_DIGSIG in bprm->unsafe. That helps but then more > issues start cropping up. I will explain issues in detail below. > >> If it was no xattr or no signature, then this flag will not be set. > > - if iint->DIGSIG is not set then it could mean few things. > - There is no xattr or digital signature > - Or there is signature but ima is disabled or there is no > appraise rule configured. > > Now second point can create confusion. It means that a signed file will > be treated as unsigned and any functionality dependent on file being > signed will fail. I think it is uintutive. > > If there is no xattr or signature, IMA hook can return failure if appraise > policy is configured. I can't ignore the return code of security hook. So > I need a separate function just to tell me whether file is signed or > not. > >> If signature verification failed, then hook returns EPERM anyway. > > There are few issues here. >
I fully understand them. In the past I have implemented attacks to IMA that based on the fact that verification result is cached... > - iint->DIGSIG will be set only if ima is enabled and some appraise > rule/policy has been enabled. Otherwise it will not be set. It might > not be too huge a issue because it just means that a signed file will > be treated unsigned and any functionality dependent on file being > signed will fail. I think it is uintutive. > > - iint->DIGSIG could be set even if file is not signed. How? > - Assume system has booted with ima_appraise_tcb policy. > - A binary executes. bprm_check() is called and it will > set iint->DIGSIG. > - root does a direct write to disk blocks where file signature > are stored. > - File executes again. This time iint->DIGSIG is set but there > are no signature on the file. > > - File could have invalid signature still iint->DIGSIG could be set and > security hook will return success. > - Assume system has booted with ima_appraise_tcb policy. > - A binary executes. bprm_check() is called and it will > set iint->DIGSIG. > - User goes ahead and replaces appraise policy with some > other policy so no appraisal rule will match for same file. Policy can only be replaced once. So if policy has been initialized at early-user-space, then it cannot be replaced... > - User does a direct write to disk on file blocks. > - File executes again. This time iint->DIGSIG is set, and > IMA hook will return success (as there is no matching appraise > rule) and making caller believe file is validly signed. > > If we don't cache iint->DIGSIG, I think couple of above issues could be > solved. But then we also need to make sure digest of file and appraisal > results not cached either. Caching of everything is in general a issue > with IMA usage in my scenario. I am not sure why IMA did not address the > issue of somebody writing directly to disk bypassing file system. IMA protects against offline attacks. Runtime protection should be done by DAC/MAC. But anyway, I fully understand that you want also additional runtime protection. > > If we figure a way out to disable caching of everything, then we also > need to figure out a way to export iint->DIGSIG to callers. Current > security hooks don't allow returning anything other than success/fail > status, that means we probably need to create a new function. Seeting > it in bprm->unsafe alone is not sufficient as I might have to do file > verification in non executable file code also. bprm->unsafe was just a very-quick show-case. > > In summary, we can still solve the problem we can do few things. > > - Provide a reliable way to disable caching of iint->DIGSIG, digest > and appraisal results. > > - Provide functions to access iint->DIGSIG after every file execution. > > - Create a separate callable IMA function which tells whether file is > signed or not. > > - Provide a way to caller to ensure whether caching is disabled or not > in IMA. So that caller can interpret what does result mean. > > Thanks > Vivek I understand what you want to achieve :) - Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
