search

maidsafe / rust_sodium

This crate is no longer maintained. Looking for maintainers.
BSD 3-Clause "New" or "Revised" License
77 stars 34 forks source link

Disable PIE on systems without lsb_release #40

Closed maximbaz closed 7 years ago

maximbaz commented 7 years ago

See this comment for explanation and the context:

https://github.com/maidsafe/rust_sodium/pull/34#discussion_r123076486

/cc @ffflorian, @ustulation

ffflorian commented 7 years ago

Any news on this one? :)

Fraser999 commented 7 years ago

Hi there,

Thanks for the PR. We're against passing this flag in all cases except Ubuntu >= 15.04, since it's harder to attack a position-independent exe. So I've created a Jira task to change the script to use an env var to set the flag. This should offer the greatest flexibility from the end-user perspective.

I don't envisage that task taking us long to complete, but if you want to have a go at a PR taking that approach instead, please feel free! :-)

maximbaz commented 7 years ago

Hello, I don't know much about this project and I'll leave it to you guys to make the final decision, just want to point out that the project is not compiling on ArchLinux without specifying this flag (see https://github.com/wireapp/libsodium-neon/issues/15 for example with logs). If you don't want to use this flag by default, perhaps the compilation failure could be addressed in some other way?

/cc @ffflorian, as this is not merged and the flag will not be default anymore, many more people will suddenly hit issue https://github.com/wireapp/libsodium-neon/issues/15.

Viv-Rajkumar commented 7 years ago

@maximbaz

If you don't want to use this flag by default, perhaps the compilation failure could be addressed in some other way?

Think the idea here is to use an env variable so the user of this lib can specify whether they want to disable pie or not. That is the Jira task linked by @Fraser999 so hopefully we can get that implemented soon which should resolve your issue once the crate is published.

ustulation commented 7 years ago

@maximbaz

If you don't want to use this flag by default, perhaps the compilation failure could be addressed in some other way?

Did you check the JIRA task linked by @Fraser999 's above ? Does that not work for all the cases ?

maximbaz commented 7 years ago

In the particular case of Wire app, this library is used by the app as a dependency, and it is built as a part of building the app itself. I'm thinking as an end-user of that app:

Today (especially assuming this patch is merged) things work automagically, a user builds the main app and rust_libsodium dependency is compiled with or without the flag based on their system (if they have Ubuntu 15, the flag is not included, otherwise - included).

By adding the environment variable you either delegate making the decision on Wire developers (should they hardcode adding flag for each and every user? Will that work on Ubuntu 15, if the flag is present?), or Wire developers should delegate making this decision to end-users (probably make an extra build flag to the app "--build-app-dependencies-with-a-magical-flag") and then try to explain users when they should or should not use that flag 🙂

Everything is possible, just feels like this should be handled here, internally in this project - if it is known that project cannot be compiled on a given system without this flag, the flag should be enabled for that system.

Fraser999 commented 7 years ago

41 which uses the env var RUST_SODIUM_DISABLE_PIE to disable PIE (see the README) is now merged and published.

I do take your point about the inconvenience of having to use this env var in some cases, but as long as libsodium delegates setting that option (which does seem reasonable to me), I don't feel we should be trying to cater for all the possible environments where this needs to be set; this project is really just a Rust binding to libsodium after all.