#60 Verbosely fail checksec.
Closed a year ago by vondruch. Opened a year ago by pvalena.
rpms/ pvalena/ruby rebase  into  master

file modified
+5 -1
@@ -768,7 +768,11 @@ 

  %if 0%{?with_hardening_test}

  # Check Ruby hardening.

  checksec --file=libruby.so.%{ruby_version} | \

-   grep "Full RELRO.*Canary found.*NX enabled.*DSO.*No RPATH.*No RUNPATH.*Yes.*\d*.*\d*.*libruby.so.%{ruby_version}"

+   grep "Full RELRO.*Canary found.*NX enabled.*DSO.*No RPATH.*No RUNPATH.*Yes.*\d*.*\d*.*libruby.so.%{ruby_version}" || \

+   {

+     checksec --file=libruby.so.%{ruby_version}

+     exit 1

+   }

  %endif

  

  # Check RubyGems version.

Will fail more spectacularly, like:

+ grep 'Full RELRO.*Canary found.*NX enabled.*DSO.*No RPATH.*No RUNPATH.*Yes.*\d*.*\d*.*libruby.so.2.6.6'
+ checksec -f libruby.so.2.6.6
+ checksec -f libruby.so.2.6.6
?[31mWARNING: 'openssl' not found! It's required for most checks.?[0m
Usage: checksec [--format={cli,csv,xml,json}] [OPTION]


Options:

 ## Checksec Options
  --file={file}
  --dir={directory}
  --proc={process name}
  --proc-all
  --proc-libs={process ID}
  --kernel[=kconfig]
  --fortify-file={executable-file}
  --fortify-proc={process ID}
  --version
  --help

 ## Modifiers
  --debug
  --verbose
  --format={cli,csv,xml,json}
  --output={cli,csv,xml,json}
  --extended

For more information, see:
  http://github.com/slimm609/checksec.sh

+ exit 1

https://download.copr.fedorainfracloud.org/results/pvalena/rubygems/fedora-rawhide-x86_64/01381770-ruby/builder-live.log.gz

I still wonder if there is not better way to do this, e.g. using tee or anything similar

How about like this with tee?

$ git diff
diff --git a/ruby.spec b/ruby.spec
index 4cd8c85..d85fcc5 100644
--- a/ruby.spec
+++ b/ruby.spec
@@ -767,8 +767,8 @@ rm -rf %{buildroot}%{gem_dir}/gems/rake-%{rake_version}/.github
 %check
 %if 0%{?with_hardening_test}
 # Check Ruby hardening.
-checksec --file=libruby.so.%{ruby_version} | \
-  grep "Full RELRO.*Canary found.*NX enabled.*DSO.*No RPATH.*No RUNPATH.*Yes.*\d*.*\d*.*libruby.so.%{ruby_version}"
+checksec --file=libruby.so.%{ruby_version} | tee %{_tmppath}/checksec.log
+grep "Full RELRO.*Canary found.*NX enabled.*DSO.*No RPATH.*No RUNPATH.*Yes.*\d*.*\d*.*libruby.so.%{ruby_version}" %{_tmppath}/checksec.log
 %endif

 # Check RubyGems version.

How about like this with tee?
$ git diff
diff --git a/ruby.spec b/ruby.spec
index 4cd8c85..d85fcc5 100644
--- a/ruby.spec
+++ b/ruby.spec
@@ -767,8 +767,8 @@ rm -rf %{buildroot}%{gem_dir}/gems/rake-%{rake_version}/.github
%check
%if 0%{?with_hardening_test}
# Check Ruby hardening.
-checksec --file=libruby.so.%{ruby_version} | \
- grep "Full RELRO.Canary found.NX enabled.DSO.No RPATH.No RUNPATH.Yes.\d.\d.libruby.so.%{ruby_version}"
+checksec --file=libruby.so.%{ruby_version} | tee %{_tmppath}/checksec.log
+grep "Full RELRO.
Canary found.NX enabled.DSO.No RPATH.No RUNPATH.Yes.\d.\d.libruby.so.%{ruby_version}" %{_tmppath}/checksec.log
%endif

# Check RubyGems version.

Well, you can do |tee -a /dev/stderr (or use a file), but that outputs even if it doesn't fail... Yes, it's an option.

Another option (example), using grep:

$ match="Correct"; value="Wrong"; echo -e "one$value\ntwo\nthree" | grep -e one | grep -ve "$match" && echo exit 1 || :
oneWrong
exit 1
$ match="Correct"; value="Correct"; echo -e "one$value\ntwo\nthree" | grep -e one | grep -ve "$match" && echo exit 1 || :

Where $match is what we want to check for (i.e. correct output), $value simulates output from checksec. There also has to be one additional grep to filter out only the line we want to do matching on (f.e. `grep '^Full RELRO').

This will output the invalid (non-matched) line in case the grep fails, otherwise it output nothing.

Another option (example), using grep:

I think it is not readable. At least using if, else is better.

Well, you can do |tee -a /dev/stderr (or use a file), but that outputs even if it doesn't fail... Yes, it's an option.

I did not know the way to use the device file as a argument of tee. How to do it with tee -a /dev/stderr like the following example?

+checksec --file=libruby.so.%{ruby_version} | tee %{_tmppath}/checksec.log
+grep "Full RELRO.*Canary found.*NX enabled.*DSO.*No RPATH.*No RUNPATH.*Yes.*\d*.*\d*.*libruby.so.%{ruby_version}" %{_tmppath}/checksec.log

I noticed My Fedora as file descriptor 3. We might be use this. But I still think the way with %{_tmppath}/checksec.log file is the most readable.

$ ls -l /dev/stderr 
lrwxrwxrwx 1 root root 15 May 12 12:05 /dev/stderr -> /proc/self/fd/2

$ ls /proc/self/fd
0@  1@  2@  3@

Frankly, I think the time discussing this would be better spent on something else. This issue pops out just once in a many moons. In that case, it is easy to manually duplicate the line to obtain the debug output. More time takes the subsequent analysis. I don't see this important enough for a change which is suboptimal from my POV. Thx for the PR but I'm going to reject it.

Pull-Request has been closed by vondruch

a year ago

You do not need to spend a time for the discussion if you do not have time.
As you did not have time, so, you rejected it.

It seems that currently the rpms/ruby is essentially managed by only one person's will and decision, even when it is the role of the main maintainer you.

How about adding additional active maintainers for rpms/ruby? Though I do not think I am a person to be the co-maintainer of rpm/ruby. I think Pavel is the right person to work with the tickets which Vit is not interested in.

I thought Mamoru was the good candidate for the co-maintainer of rpm/ruby. And he has been already added as the maintainer :-)

Vit, do you like managing and controling every source code in rpms/ruby by your decision? I think it's okay if you want to do it. It is your style.

I am contributing to rpms/ruby as it is my job at Red Hat, but otherwise, I do not do it.
Personally I would prefer the following project's maintainer's management style. He is more empathic.
https://github.com/nemequ/simde/issues

@jaruga, first of all, you have commit rights to the ruby, because there is @ruby-packagers-sig. I think I don't need to add you explicitly, do I?

Secondly, I have spent several hours investigating the issue and trying hard to find some satisfiable solution and I have not find any nor any proposals from this thread are satisfying. Moreover this was not the first time I have spent the time on this topic. Nevertheless, because the added value is so low, I decided after all to reject this and move on.

Just FTR, judging by the comment timestamps, it seems you have spent 1 hour writing the last 3 comments. I would be much happier if you spent that time on issues such as testing fixes for https://bugs.ruby-lang.org/issues/16904 I am quite sure that would be more beneficial to Fedora users and RH customers.

first of all, you have commit rights to the ruby, because there is @ruby-packagers-sig. I think I don't need to add you explicitly, do I?

You are right. Technically all the ruby packagers could have the rights, and reopen a pull-request when you declined it. You do not need to add me explicitly.

Just FTR, judging by the comment timestamps, it seems you have spent 1 hour writing the last 3 comments. I would be much happier if you spent that time on issues such as testing fixes for https://bugs.ruby-lang.org/issues/16904 I am quite sure that would be more beneficial to Fedora users and RH customers.

I was writing the comments during working and building Fedora module. Because Ruby module has the clear deadline. It's not 1 hour at all. Maybe 5 or 10 minutes. Your judgement is wrong. As a parallel job, writing the comments was good.

For the 16904 issue, it was not still clear for me which patch should be applied to the Fedora Ruby until Upstream Ruby would apply the patch. Which patch do you like to apply to Fedora Ruby? I expected you would work to apply the patch to rpms/ruby as usual, as you mostly have worked for rpms/ruby master branch.

I'm fine with closing the ticket at there's no optimal solution (all are either too complicated or "hackish"). It was meant as a discussion start (hence the naive implementation I submitted). It's a quite generic issue (many packages), but there's no need to solve it here, now and for this specific issue.

Maybe I'll submit some suggestion/proposal for an assert macro (to have such capability for all specs).
Please let me know if you know some better solution than the grep mentioned above (or equivalent).


I do not think there's need for me to address anything else in the discussion. Only suggestion would be to keep the content on-topic, as for general discussion I prefer Ruby SIG ML or other channels.