mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "H. Peter Anvin" <hpa@zytor.com>
To: Mimi Zohar <zohar@linux.vnet.ibm.com>, Vivek Goyal <vgoyal@redhat.com>
Cc: linux-kernel@vger.kernel.org, ebiederm@xmission.com,
	pjones@redhat.com, dhowells@redhat.com, jwboyer@redhat.com
Subject: Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary
Date: Sun, 20 Jan 2013 08:17:40 -0800	[thread overview]
Message-ID: <adaba402-2f13-4e0b-ac06-143adaf6101b@email.android.com> (raw)
In-Reply-To: <1358440365.2689.72.camel@falcor1>

You then get into issues like: do we have to ban prelink as a result?

Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

>On Thu, 2013-01-17 at 10:51 -0500, Vivek Goyal wrote:
>> On Thu, Jan 17, 2013 at 10:37:01AM -0500, Mimi Zohar wrote:
>> > On Tue, 2013-01-15 at 16:34 -0500, Vivek Goyal wrote:
>> > > If a binary is signed, verify its signature. If signature is not
>valid, do
>> > > not allow execution. If binary is not signed, execution is
>allowed
>> > > unconditionally.
>> > > 
>> > > CONFIG_BINFMT_ELF_SIGNATURE controls whether elf binary signature
>support
>> > > is compiled in or not.
>> > > 
>> > > Signature are expected to be present in elf section ".section".
>This code
>> > > is written along the lines of module signature verification code.
>Just
>> > > that I have removed the magic string. It is not needed as
>signature is
>> > > expected to be present in a specific section.
>> > 
>> > Right, it's written along the lines of the original module
>signature
>> > verification code, where the signature was in the ELF header, not
>the
>> > version that was upstreamed, where the signature was appended.
>> > 
>> > > I put the signature into a section, instead of appending it so
>that
>> > > strip operation works fine.
>> > 
>> > Wouldn't the original reasons for not embedding the signature in
>the ELF
>> > header for modules apply here too?  
>> 
>> I think rusty wanted to append signatures. He thought it is much
>easier.
>> Adding a .signature section makes life easier for user space tools.
>One
>> can strip files even after signing.
>> 
>> Not that I am married to the idea of putting signature in a section.
>Just
>> that it sounded reasonable enough to do for an RFC. So if appending 
>> signature proves to be better, it is easy to implement that.
>> > 
>> > > One signs and verifies  all the areas mapped by PT_LOAD segments
>of elf
>> > > binary. Typically Elf header is mapped in first PT_LOAD segment.
>As adding
>> > > .signature section can change three elf header fields (e_shoff,
>e_shnum
>> > > and e_shstrndx), these fields are excluded from digest
>calculation
>> > > 
>> > > Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
>> > 
>> > At this point, why would you want yet another method for signing
>files?
>> 
>> Are you saying that append signature instead of putting them in a
>section
>> or are you saying that just use IMA.
>> 
>> - For the first, I am fine with appending too if that works better.
>So
>>   what's wrong with current implementation. Just because we append
>the
>>   signatures in case of modules, we should follow the same thing for
>>   executables too?
>
>No, I was saying that if this patch set were to be upstreamed, then the
>signature verification, at least for ELF modules and ELF executables,
>should be the same.  The patch would then be a lot smaller.
>
>> - If above comment is w.r.t use of IMA, then I have no issues in
>using
>>   IMA as long as it can meet all the requirements. Looks like there
>is
>>   a long TODO list before we get there. In fact for some things its
>not
>>   even clear whether they fit in IMA or somehwere else.
>> 
>>   - Make sure IMA/EVM stuff chains into secureboot chain of trust.
>
>For sure.
>
>>   - Sort out all the memory locking related issues I mentioned in
>>     other mail. You seemed to be of opinion that it is out of scope
>>     for IMA, but I think it probably is nice extenstion.
>
>Yes, it would be, but I'm not sure that your method of mmaping a file
>with MAP_LOCKED is scalable, when all executables are signed.  If it
>is,
>then why not do it in general?  Otherwise there needs to be a more
>scalable solution.
>
>>     Or somehow a way needs to be found to make sure nobody can modify
>>     process address space without processe's knowledge.
>
>I'm not sure I understand.  Does your patch already address this?
>
>>   - Once all this works, then one needs to figure out all the RPM
>stuff
>>     and plugins to make sure files can be singed on build server and
>>     installed properly on target system.
>
>Yes, we need the distros involvment in this to sign all files.
>Immutable files should be signed with digital signatures.  Mutable
>files
>should have hashes.
>
>thanks,
>
>Mimi

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

  parent reply	other threads:[~2013-01-20 16:17 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-15 21:34 [PATCH 0/3] ELF executable signing and verification Vivek Goyal
2013-01-15 21:34 ` [PATCH 1/3] module: export couple of functions for use in process signature verification Vivek Goyal
2013-01-15 21:34 ` [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary Vivek Goyal
2013-01-16  4:30   ` Eric W. Biederman
2013-01-16  4:55     ` Mimi Zohar
2013-01-16  7:10       ` Eric W. Biederman
2013-01-16 14:00         ` Mimi Zohar
2013-01-16 14:48           ` Vivek Goyal
2013-01-16 15:33             ` Mimi Zohar
2013-01-16 15:54               ` Vivek Goyal
2013-01-16 17:24                 ` Mimi Zohar
2013-01-16 18:21                   ` Vivek Goyal
2013-01-16 18:45                     ` Mimi Zohar
2013-01-16 18:57                       ` Vivek Goyal
2013-01-16 19:37                         ` Mimi Zohar
2013-01-16 19:47                           ` Vivek Goyal
2013-01-16 20:25                             ` Mimi Zohar
2013-01-16 21:55                               ` Vivek Goyal
2013-01-17  8:37                             ` Elena Reshetova
2013-01-17 14:39                     ` Kasatkin, Dmitry
2013-01-17 14:35                 ` Kasatkin, Dmitry
2013-01-16 16:34               ` Vivek Goyal
2013-01-16 18:08                 ` Mimi Zohar
2013-01-16 18:28                   ` Vivek Goyal
2013-01-16 19:24                     ` Mimi Zohar
2013-01-16 21:53                       ` Vivek Goyal
2013-01-17 14:58                         ` Kasatkin, Dmitry
2013-01-17 15:06                           ` Kasatkin, Dmitry
2013-01-17 15:21                             ` Vivek Goyal
2013-01-17 15:18                           ` Vivek Goyal
2013-01-17 16:27                             ` Kasatkin, Dmitry
2013-01-17 20:33                             ` Frank Ch. Eigler
2013-01-17 20:55                               ` Vivek Goyal
2013-01-17 21:46                                 ` Kasatkin, Dmitry
2013-01-17 21:52                                   ` Vivek Goyal
2013-01-20 16:36                                     ` Mimi Zohar
2013-01-21 16:42       ` Vivek Goyal
2013-01-21 18:30         ` Mimi Zohar
2013-01-16 22:35   ` Mimi Zohar
2013-01-16 22:51     ` Vivek Goyal
2013-01-16 23:16       ` Eric W. Biederman
2013-01-17 15:37   ` Mimi Zohar
2013-01-17 15:51     ` Vivek Goyal
2013-01-17 16:32       ` Mimi Zohar
2013-01-17 17:01         ` Kasatkin, Dmitry
2013-01-17 17:03           ` Kasatkin, Dmitry
2013-01-17 17:42           ` Vivek Goyal
2013-01-17 17:36         ` Vivek Goyal
2013-01-20 17:20           ` Mimi Zohar
2013-01-21 15:45             ` Vivek Goyal
2013-01-21 18:44               ` Mimi Zohar
2013-01-20 16:17         ` H. Peter Anvin [this message]
2013-01-20 16:55           ` Mimi Zohar
2013-01-20 17:00             ` H. Peter Anvin
2013-01-15 21:34 ` [PATCH 3/3] binfmt_elf: Do not allow exec() if signed binary has intepreter Vivek Goyal
2013-01-15 21:37 ` [PATCH 4/3] User space utility "signelf" to sign elf executable Vivek Goyal
2013-01-15 22:27 ` [PATCH 0/3] ELF executable signing and verification richard -rw- weinberger
2013-01-15 23:15   ` Vivek Goyal
2013-01-15 23:17     ` richard -rw- weinberger
2013-01-17 16:22 ` Kasatkin, Dmitry
2013-01-17 17:25   ` Vivek Goyal
2013-01-22  4:22 ` Rusty Russell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=adaba402-2f13-4e0b-ac06-143adaf6101b@email.android.com \
    --to=hpa@zytor.com \
    --cc=dhowells@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=jwboyer@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pjones@redhat.com \
    --cc=vgoyal@redhat.com \
    --cc=zohar@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome