[PATCH] D66426: [lld] Enable a watermark of loadable sections to be generated and placed in a note section

Mon Dec 9 01:53:30 PST 2019

jhenderson added a comment.

In D66426#1773537 <https://reviews.llvm.org/D66426#1773537>, @MaskRay wrote:

> In D66426#1768793 <https://reviews.llvm.org/D66426#1768793>, @jhenderson wrote:
>
> > In D66426#1767580 <https://reviews.llvm.org/D66426#1767580>, @MaskRay wrote:
> >
> > > > The whole point of the watermark is to show that no post-link modifications have been made, and if the watermark itself is added post-link, it does not achieve this aim: someone could either deliberately or accidentally add a step prior to the watermarking happening.
> > >
> > > I am still confused. What I infer from the sentence is that strip/llvm-strip is still allowed. To make .note.llvm.watermark survive strip/llvm-strip, you place it into a PT_NOTE segment. So post-link modification is still possible, then why can't you use another tool to compute the watermark and append a section? In my comment, there are some other questions that are not answered. I have suggested an approach that will not slow down the whole build time.
> >
> >
> > (For clarity, @chrisjackson and I are on the same team, and I've been helping him with this). The problem with any post-link external tool used to create the watermark is that it doesn't prevent something happening between the link step and the watermarking step. For example, this would not be detected: 1) Do link; 2) Make a modification to the .data section; 3) Run the watermark tool.
> >
> > Stripping (and other things that don't affect the loadable image) is allowed, because it doesn't affect the loadable image. The aim of the watermark is to detect //loadable// data changes.
>
>
> As I understand it, the scenario is:
>
> 1. Do link; 2) Run the watermark tool to append .note.llvm.watermark; 3) Release SDK; 4) Downstream vendors modify .data and ship to end users; 5) End users verify that .note.llvm.watermark does not match computed watermark of loadable contents.
>
>   The build process before 3) are all controlled. The process should ensure there is no modification to .data between 1) and 2). How do you guarantee a linker side feature can prevent modification? How can you prevent the following:
> 2. Do link and generate .note.llvm.watermark in one step 1.5) Modify .data 2) Run the watermark tool to update .note.llvm.watermark

This is a motivation to not have an external tool on the watermarking. That being said, as @chrisjackson has said on more than one occasion, this isn't intended to be a security feature so we are not attempting to detect a malicious attacker. It could be possible for downstream LLD producers to add a local salt to the watermark to make it more secure, should they so choose, I suppose.

>> Build times should include the watermarking process, since that is part of creating a release build (just as e.g. llvm-strip etc should be too). Thus, saying that an external tool will not slow down build times is incorrect.
> 
> The watermark tool can append .note.llvm.watermark to the executable. It just has to rewrite the section header table at the end of the ELF (usually a few hundred bytes). This is not slower than a built-in linker feature.

It really doesn't matter which is slower: if an external watermarking tool is not part of the linker, things can be done post-link and therefore the thing being watermarked is not the output of the linker. The main goal of this is to show that there have been no changes to the loadable part of the //linker// output, whether controlled by the user of the linker or not.

I'm not sure if you've missed how the process is expected to go. Here's a recap:

1. User calls linker with --watermark specified, producing output.elf with a watermark.
2. Optionally a user might choose to do modifications that don't affect non-loadable data, e.g. `llvm-objcopy --strip-all output.elf` etc.
3. A user (probably a different user) runs a tool to validate that the watermark remains correct. If not, they can report it to the producer of `output.elf`.

Any attempt to modify e.g. .data between 1 and 3, either accidentally or deliberately, will be detected, unless the user explicitly tries to defeat the watermarking, which as mentioned is not something this feature on its own tries to detect. Having an external tool that does the watermarking post-link (i.e. after step 1) would allow users to make the modifications before watermarking, which is explicitly what the watermarking is trying to detect (and therefore wouldn't in this case).

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66426/new/

https://reviews.llvm.org/D66426