Skip to content
This repository has been archived by the owner on Jan 15, 2025. It is now read-only.

(PA-6383) Enable PIE for Ubuntu and Debian #892

Merged
merged 1 commit into from
Aug 19, 2024

Conversation

@skyamgarp skyamgarp requested review from a team as code owners August 9, 2024 14:38
@skyamgarp skyamgarp marked this pull request as draft August 9, 2024 14:38
@joshcooper
Copy link
Contributor

joshcooper commented Aug 9, 2024

Thanks @skyamgarp I had a few comments:

  1. Could you add a _shared-compiler-settings.rb like @h0tw1r3 did in https://github.com/puppetlabs/puppet-runtime/pull/831/files#diff-c682e6498bfc9db6945b066a648384113b3f91de927edac5e67f3b31c6336b96 containing:
# Define default CFLAGS and LDFLAGS for most platforms, and then
# tweak or adjust them as needed.
proj.setting(:cppflags, "-I#{proj.includedir} -I/opt/pl-build-tools/include")
proj.setting(:cflags, "-frecord-gcc-switches #{proj.cppflags}")
proj.setting(:ldflags, "-L#{proj.libdir} -L/opt/pl-build-tools/lib -Wl,-rpath=#{proj.libdir}")

if (platform.is_sles? && platform.os_version.to_i >= 15) ||
      (platform.is_el? && platform.os_version.to_i >= 8 && platform.architecture !~ /ppc64/) ||
      (platform.is_debian? && platform.os_version.to_i >= 10) ||
      (platform.is_ubuntu? && platform.os_version.to_i >= 20) ||
      platform.is_fedora?
  proj.setting(:supports_pie, true)
end

Then refactor all of the projects to include the shared compiler settings. For example, in configs/projects/_shared-agent-settings.rb

-proj.setting(:cppflags, "-I#{proj.includedir} -I/opt/pl-build-tools/include")
-proj.setting(:cflags, "#{proj.cppflags}")
-proj.setting(:ldflags, "-L#{proj.libdir} -L/opt/pl-build-tools/lib -Wl,-rpath=#{proj.libdir}")
+# Load default compiler settings
+instance_eval File.read('configs/projects/_shared-compiler-settings.rb')

Then in each project/component, you can test for settings[:supports_pie] instead of copying the conditional. For example, in augeas.rb:

-  if((platform.is_sles? && platform.os_version.to_i >= 15) ||
-      (platform.is_el? && platform.os_version.to_i >= 8 && platform.architecture !~ /ppc64/) ||
-      (platform.is_debian? && platform.os_version.to_i >= 10) ||
-      (platform.is_ubuntu? && platform.os_version.to_i >= 20) ||
-      platform.is_fedora?
-    )
+ if proj.settings[:supports_pie]
  1. I think it's fine to skip power architecture for now, but please make a ticket so we come back to it.

  2. IIUC, your PR enables PIE for projects that include configs/projects/_shared-agent-settings.rb because that's where we define FORTIFY_SOURCE, etc More work is needed to enable PIE for other projects. For example, we'd need to move these settings into the shared-compiler-settings component and do this for each project. Could you add another commit to your PR or file a separate ticket?

  3. Could you verify that all of the executables built during agent-runtime-* have PIE enabled and can be executed/loaded? Gems with native extensions are likely to be problematic, for example ffi. Running /opt/puppetlabs/puppet/bin/ruby -e 'puts "require \"ffi\""' is a good test.

@skyamgarp skyamgarp force-pushed the PA-6383 branch 3 times, most recently from 23a54a3 to c3578b9 Compare August 13, 2024 11:21
(PA-6383) Exclude ppc64 architecture

(PA-6383) Fix Spaces

(PA-6383) Exclude ppc64 arch

(PA-6383) created compiler setting separately

(PA-6383) Changes to runtime-bolt

(PA-6383) Exclude EL platforms
@skyamgarp skyamgarp marked this pull request as ready for review August 19, 2024 05:55
@skyamgarp skyamgarp changed the title (PA-6383) Enable PIE for RHEL,Ubuntu and Debian (PA-6383) Enable PIE for Ubuntu and Debian Aug 19, 2024
@cthorn42 cthorn42 merged commit d9873e5 into puppetlabs-toy-chest:master Aug 19, 2024
3 checks passed
@skyamgarp skyamgarp deleted the PA-6383 branch August 19, 2024 15:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants