Bug 186332 - fix install path of the binary in sysutils/xmbmon
Summary: fix install path of the binary in sysutils/xmbmon
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: John Marino
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-01 07:30 UTC by TsurutaniNaoki
Modified: 2014-07-11 11:07 UTC (History)
3 users (show)

See Also:


Attachments
file.diff (3.04 KB, patch)
2014-02-01 07:30 UTC, TsurutaniNaoki
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description TsurutaniNaoki 2014-02-01 07:30:00 UTC
	sysutils/xmbmon installs an executable "xmbmon" to X resource directory.
	this is something strange.

Fix: here is a patch to ports tree.
	this also makes normal user can build the ports.

How-To-Repeat: 	always.
Comment 1 Edwin Groothuis freebsd_committer freebsd_triage 2014-02-01 07:30:07 UTC
Maintainer of sysutils/xmbmon,

Please note that PR ports/186332 has just been submitted.

If it contains a patch for an upgrade, an enhancement or a bug fix
you agree on, reply to this email stating that you approve the patch
and a committer will take care of it.

The full text of the PR can be found at:
    http://www.freebsd.org/cgi/query-pr.cgi?pr=ports/186332

-- 
Edwin Groothuis via the GNATS Auto Assign Tool
edwin@FreeBSD.org
Comment 2 Edwin Groothuis freebsd_committer freebsd_triage 2014-02-01 07:30:08 UTC
State Changed
From-To: open->feedback

Awaiting maintainers feedback (via the GNATS Auto Assign Tool)
Comment 3 John Marino freebsd_committer freebsd_triage 2014-06-19 11:32:24 UTC
I think this PR is obsolete after the recent changes.
Comment 4 TsurutaniNaoki 2014-06-20 07:35:56 UTC
(In reply to John Marino from comment #3)
> I think this PR is obsolete after the recent changes.

No, install path is still wrong; xmbmon should be in ${LOCALBASE}/bin.
Comment 5 John Marino freebsd_committer freebsd_triage 2014-06-20 09:53:04 UTC
I will assume that it was placed in the X resource directory intentionally by somebody in the past.  They must have had a reason.

This is the maintainer's call, but personally I'd like some rationale why it should be in bin instead other than the obvious "path" argument.  Something that explains why it was in X resource before and why that rationale isn't valid anymore.  Does anyone know that?
Comment 6 TsurutaniNaoki 2014-07-04 23:43:44 UTC
In additon, mbmon and xmbmon binary is not setuid root any more since rev.358319.
Comment 7 John Marino freebsd_committer freebsd_triage 2014-07-05 06:38:08 UTC
It uses the standard (and correct) INSTALL_PROGRAM macro to install the binaries.

> make -v INSTALL_PROGRAM
install -s -m 555

see http://www.freebsd.org/doc/en/books/porters-handbook/book.html
section 5.15.1

It should be setting ownership to root:wheel by default.
Comment 8 Muhammad Moinur Rahman freebsd_committer freebsd_triage 2014-07-05 08:47:30 UTC
When I took over it there were two different applications: mbmon and xmbmon(the GUI of mbmon). I merged it into a single port. And hence when you are chosing X11 as an option it will install the xmbmon app in X11 resource directory. I think it is perfectly alright where it is now. On the other hand the CLI app is installed in ${LOCALBASE}/bin.
Comment 9 John Marino freebsd_committer freebsd_triage 2014-07-05 09:05:20 UTC
muhammad, if you address the setuid comment I think we can close this again.  At least, I don't see any action to take.
Comment 10 Muhammad Moinur Rahman freebsd_committer freebsd_triage 2014-07-05 09:22:53 UTC
I can see that both are installed as root:wheel. But if someone disagrees I need snippets and versions of OS.

(In reply to John Marino from comment #9)
> muhammad, if you address the setuid comment I think we can close this again.
> At least, I don't see any action to take.
Comment 11 TsurutaniNaoki 2014-07-06 06:11:44 UTC
Thank you for your work of merging 2 ports.
However, both mbmon and xmbmon binary should be in ${LOCALBASE}/bin and they should be setuid root; for both of them are used by normal users.
Applications and commands for users should be put in normal directory.
Both mbmon and xmbmon are put in ${LOCALBASE}/bin originally and setuid root
in order to work with normal user.  Is there any reasons why changing this ?
Comment 12 John Marino freebsd_committer freebsd_triage 2014-07-06 07:08:53 UTC
(In reply to turutani from comment #11)
> Thank you for your work of merging 2 ports.
> However, both mbmon and xmbmon binary [snip]
> should be setuid root; for both of them are used by normal users.

Both of us said we see these files as owned by root.  From context, you seem to think some other user id owns it.  Or we don't understand what you mean when you say "setuid root".  Speaking for myself, I know I need clarification.


> Applications and commands for users should be put in normal directory.
> Both mbmon and xmbmon are put in ${LOCALBASE}/bin originally and setuid root
> in order to work with normal user.  Is there any reasons why changing this ?


Why can't a normal user execute it now?  regardless if it's in x11 resource directory or /usr/local/bin ?  There's no permission restriction on execution.

Maybe this is clear to Muhammad, but I'm just not understanding the problem.
Comment 13 TsurutaniNaoki 2014-07-06 08:36:35 UTC
I meant the both binary should be installed with setuid bit on.
Original Makefile installs them with "install -o root -g wheel -m 4555".
Otherwise, normal user may not be able to access to the system information.
I cannot find the reason why this behaviour should be changed.
Is there any discussion about this point before you drop the suid bit ?

Concerning th install path, I think the binary which normal user can run should
be put where popular users add to the PATH environmental variable.
In fact, I could not find it at the first time after upgrade.
Resource direcroty should only contain the resource files, and should not
contain the binary for users.  Is there any misunderstandings ?
Comment 14 John Marino freebsd_committer freebsd_triage 2014-07-06 08:58:12 UTC
(In reply to turutani from comment #13)
> I meant the both binary should be installed with setuid bit on.
> Original Makefile installs them with "install -o root -g wheel -m 4555".
> Otherwise, normal user may not be able to access to the system information.
> I cannot find the reason why this behaviour should be changed.
> Is there any discussion about this point before you drop the suid bit ?

We are using the standard installation macros which are ultimately using the defaults defined in /usr/share/mk, and they are all 3 digit permissions.  It is best practice to use standard installation macros instead of vendor makefile installation commands. (e.g. use ${BSD_INSTALL_PROGRAM} in the vendor makefile to force it to install according to ports standards.

The macro can be affected by setting BINMODE=4555 in the vendor makefile though.  I'll leave it to Muhammad to determine if that needs to be defined.

 
> Concerning th install path, I think the binary which normal user can run
> should
> be put where popular users add to the PATH environmental variable.
> In fact, I could not find it at the first time after upgrade.
> Resource direcroty should only contain the resource files, and should not
> contain the binary for users.  Is there any misunderstandings ?

OK, if I understand correctly, the main complaint is that x11 resource directory is not on a standard path.  It can be run, but only with a full path to the executable.
Comment 15 Muhammad Moinur Rahman freebsd_committer freebsd_triage 2014-07-06 09:28:29 UTC
I don't think so. So far I believe when X is installed these paths are believed to be added to to ENV. Cause these same paths are used at least for other 88 ports.

find /usr/ports/ -name Makefile | xargs grep "lib/X11/app-defaults" 

(In reply to John Marino from comment #14)
> (In reply to turutani from comment #13)
> > I meant the both binary should be installed with setuid bit on.
> > Original Makefile installs them with "install -o root -g wheel -m 4555".
> > Otherwise, normal user may not be able to access to the system information.
> > I cannot find the reason why this behaviour should be changed.
> > Is there any discussion about this point before you drop the suid bit ?
> 
> We are using the standard installation macros which are ultimately using the
> defaults defined in /usr/share/mk, and they are all 3 digit permissions.  It
> is best practice to use standard installation macros instead of vendor
> makefile installation commands. (e.g. use ${BSD_INSTALL_PROGRAM} in the
> vendor makefile to force it to install according to ports standards.
> 
> The macro can be affected by setting BINMODE=4555 in the vendor makefile
> though.  I'll leave it to Muhammad to determine if that needs to be defined.
> 
>  
> > Concerning th install path, I think the binary which normal user can run
> > should
> > be put where popular users add to the PATH environmental variable.
> > In fact, I could not find it at the first time after upgrade.
> > Resource direcroty should only contain the resource files, and should not
> > contain the binary for users.  Is there any misunderstandings ?
> 
> OK, if I understand correctly, the main complaint is that x11 resource
> directory is not on a standard path.  It can be run, but only with a full
> path to the executable.
Comment 16 TsurutaniNaoki 2014-07-06 09:34:44 UTC
(In reply to Muhammad Moinur Rahman from comment #15)
> I don't think so. So far I believe when X is installed these paths are
> believed to be added to to ENV. Cause these same paths are used at least for
> other 88 ports.
> 
> find /usr/ports/ -name Makefile | xargs grep "lib/X11/app-defaults" 

I have no objection to use app-defaults directory: how many ports installs
binary for users to app-defaults ?
Comment 17 John Marino freebsd_committer freebsd_triage 2014-07-08 13:16:03 UTC
Muhammad, is it worth setting BINMODE to 4555?
I don't actually understand what isn't possible with BINMODE 555 but turutani seems to think it's essential for correct operation, or at least to match what it did before...
Comment 18 John Marino freebsd_committer freebsd_triage 2014-07-10 15:54:45 UTC
Muhammad, reading this back quickly, I'm seeing two recommendations:

1) set BINMODE to 4555
2) move binary location lib/X11/app-defaults

Can you comment on these?
If you agree, can you provide a patch so we can (finally) close out this PR?
Comment 19 Muhammad Moinur Rahman freebsd_committer freebsd_triage 2014-07-10 16:12:00 UTC
John
1. We can go ahead with it.
2. It's already set as that and submitter wants to change it to generic $PATH.


(In reply to John Marino from comment #18)
> Muhammad, reading this back quickly, I'm seeing two recommendations:
> 
> 1) set BINMODE to 4555
> 2) move binary location lib/X11/app-defaults
> 
> Can you comment on these?
> If you agree, can you provide a patch so we can (finally) close out this PR?
Comment 20 commit-hook freebsd_committer freebsd_triage 2014-07-10 20:31:12 UTC
A commit references this bug:

Author: marino
Date: Thu Jul 10 20:30:30 UTC 2014
New revision: 361488
URL: http://svnweb.freebsd.org/changeset/ports/361488

Log:
  sysutils/xmbmon: Install programs with mode 4555 instead of 555

  Mode 4555 is how the programs were installed before staging, and allows
  normal users to access the system information.  For now, xmbmon is still
  installed in the X resource directory.

  PR:		186332
  Reported by:	turutani (scphys.kyoto-u.ac.jp)
  Approved by:	maintainer (Muhammad Rahman)

Changes:
  head/sysutils/xmbmon/Makefile
Comment 21 John Marino freebsd_committer freebsd_triage 2014-07-11 05:55:00 UTC
(In reply to turutani from comment #16)
> (In reply to Muhammad Moinur Rahman from comment #15)
> > find /usr/ports/ -name Makefile | xargs grep "lib/X11/app-defaults" 
> 
> I have no objection to use app-defaults directory: how many ports installs
> binary for users to app-defaults ?

I ran that command with " | wc -l" and it returned 43.

I am inclined to leave xmbmon path as it is and I think Muhammad is as well.  Can we close the PR out now that the executables are once again installed with mode 4555?
Comment 22 TsurutaniNaoki 2014-07-11 09:09:42 UTC
(In reply to John Marino from comment #21)
> I am inclined to leave xmbmon path as it is and I think Muhammad is as well.
> Can we close the PR out now that the executables are once again installed
> with mode 4555?

Please do as you like.