[GN][Chromecast] Add "-Wl,-rpath=\$ORIGIN" to executable and shlib defaults.

[GN][Chromecast] Add "-Wl,-rpath=\$ORIGIN" to executable and shlib defaults.

Chromecast executables depend on several shared libraries in $ORIGIN for
platform and OEM abstraction. The rpath for executables should be
updated so that these libraries can be found at runtime. This logic is
added here, instead of //build/config/gcc:executable_ldconfig, to keep
Chromecast-specific configs as concentrated as possible.

A note about security concerns: Chrome only enables this by default for
component and sanitizer builds, never shipped builds. This is to protect
users from arbitrary binaries being run in the shared lib directory.
Chromecast does not share these concerns, as it only runs trusted web
applications in practice.

BUG=516899
Committed: https://crrev.com/875b6f2a243ffbfb5bc3cd86f8ae97c3659c010d
Cr-Commit-Position: refs/heads/master@{#371886}

[slan, 2016-01-27 16:39:40]

slan@chromium.org changed reviewers:
+ alokp@chromium.org, dougsteed@chromium.org, dpranke@chromium.org,
halliwell@chromium.org

[slan, 2016-01-27 16:39:40]

Hey Dirk,

Per our discussion yesterday, I would like to add a default rpath here for Cast
GN executables. There's a number of ways that we can do this, and this seems
like the cleanest solution. PTAL!

+alokp, dougsteed, halliwell for FYI

[Dirk Pranke, 2016-01-27 17:59:15]

lgtm, though I'm not sure I would've bothered with the extra layer of
indirection.

[slan, 2016-01-27 18:06:57]

On 2016/01/27 17:59:15, Dirk Pranke wrote:
> lgtm, though I'm not sure I would've bothered with the extra layer of
> indirection.

Thanks for the feedback. This seemed like the best way to put the big fat
comment block in just one spot. That's the only reason for "ldconfig".

I'll wait for 1 Cast lgtm, then submit.

[alokp, 2016-01-27 18:17:21]

On 2016/01/27 18:06:57, slan wrote:
> On 2016/01/27 17:59:15, Dirk Pranke wrote:
> > lgtm, though I'm not sure I would've bothered with the extra layer of
> > indirection.
> 
> Thanks for the feedback. This seemed like the best way to put the big fat
> comment block in just one spot. That's the only reason for "ldconfig".
> 
> I'll wait for 1 Cast lgtm, then submit.

Stephen: I think Doug would be the best person to review the security concerns.
Note that this is only cumbersome for the developers, in which case, can we only
do this for eng and debug builds? I am not sure I understand the comment about
only allowing trusted web applications.

[slan, 2016-01-27 18:48:35]

On 2016/01/27 18:17:21, alokp wrote:
> On 2016/01/27 18:06:57, slan wrote:
> > On 2016/01/27 17:59:15, Dirk Pranke wrote:
> > > lgtm, though I'm not sure I would've bothered with the extra layer of
> > > indirection.
> > 
> > Thanks for the feedback. This seemed like the best way to put the big fat
> > comment block in just one spot. That's the only reason for "ldconfig".
> > 
> > I'll wait for 1 Cast lgtm, then submit.
> 
> Stephen: I think Doug would be the best person to review the security
concerns.

I spoke briefly with Doug about this this morning before uploading. I am not
super familiar with the setuid attack vector mentioned in the comments, but
since the functionality of Cast apps is limited to a finite set of APIs, and
there is no user access to this directory, I don't see this as a security
concern for us. But I am definitely out of my depth here, so I will let Doug
speak about the perceived risk :)

> Note that this is only cumbersome for the developers, in which case, can we
only
> do this for eng and debug builds? I am not sure I understand the comment about
> only allowing trusted web applications.

I think this is a concern for user builds as well. AFAIK, setting environment
vars is the only other way to solve this, which i see as strictly less
preferable. Perhaps I'm missing something.

[dougsteed, 2016-01-27 20:43:55]

On 2016/01/27 18:48:35, slan wrote:
> On 2016/01/27 18:17:21, alokp wrote:
> > On 2016/01/27 18:06:57, slan wrote:
> > > On 2016/01/27 17:59:15, Dirk Pranke wrote:
> > > > lgtm, though I'm not sure I would've bothered with the extra layer of
> > > > indirection.
> > > 
> > > Thanks for the feedback. This seemed like the best way to put the big fat
> > > comment block in just one spot. That's the only reason for "ldconfig".
> > > 
> > > I'll wait for 1 Cast lgtm, then submit.
> > 
> > Stephen: I think Doug would be the best person to review the security
> concerns.
> 
> I spoke briefly with Doug about this this morning before uploading. I am not
> super familiar with the setuid attack vector mentioned in the comments, but
> since the functionality of Cast apps is limited to a finite set of APIs, and
> there is no user access to this directory, I don't see this as a security
> concern for us. But I am definitely out of my depth here, so I will let Doug
> speak about the perceived risk :)
> 
> > Note that this is only cumbersome for the developers, in which case, can we
> only
> > do this for eng and debug builds? I am not sure I understand the comment
about
> > only allowing trusted web applications.
> 
> I think this is a concern for user builds as well. AFAIK, setting environment
> vars is the only other way to solve this, which i see as strictly less
> preferable. Perhaps I'm missing something.

I think this is OK on chromecast (happy to discuss further offline).

[slan, 2016-01-27 20:46:16]

The CQ bit was checked by slan@chromium.org

[commit-bot: I haz the power, 2016-01-27 20:47:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1639343002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1639343002/1

[commit-bot: I haz the power, 2016-01-27 22:10:30]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-01-27 22:11:40]

Description was changed from

==========
[GN][Chromecast] Add "-Wl,-rpath=\$ORIGIN" to executable and shlib defaults.

Chromecast executables depend on several shared libraries in $ORIGIN for
platform and OEM abstraction. The rpath for executables should be
updated so that these libraries can be found at runtime. This logic is
added here, instead of //build/config/gcc:executable_ldconfig, to keep
Chromecast-specific configs as concentrated as possible.

A note about security concerns: Chrome only enables this by default for
component and sanitizer builds, never shipped builds. This is to protect
users from arbitrary binaries being run in the shared lib directory.
Chromecast does not share these concerns, as it only runs trusted web
applications in practice.

BUG=516899
==========

to

==========
[GN][Chromecast] Add "-Wl,-rpath=\$ORIGIN" to executable and shlib defaults.

Chromecast executables depend on several shared libraries in $ORIGIN for
platform and OEM abstraction. The rpath for executables should be
updated so that these libraries can be found at runtime. This logic is
added here, instead of //build/config/gcc:executable_ldconfig, to keep
Chromecast-specific configs as concentrated as possible.

A note about security concerns: Chrome only enables this by default for
component and sanitizer builds, never shipped builds. This is to protect
users from arbitrary binaries being run in the shared lib directory.
Chromecast does not share these concerns, as it only runs trusted web
applications in practice.

BUG=516899

Committed: https://crrev.com/875b6f2a243ffbfb5bc3cd86f8ae97c3659c010d
Cr-Commit-Position: refs/heads/master@{#371886}
==========

[commit-bot: I haz the power, 2016-01-27 22:11:41]

Patchset 1 (id:??) landed as
https://crrev.com/875b6f2a243ffbfb5bc3cd86f8ae97c3659c010d
Cr-Commit-Position: refs/heads/master@{#371886}