Bug 215989 - shells/lshell: Update to 0.9.18, take MAINTAINER'ship
Summary: shells/lshell: Update to 0.9.18, take MAINTAINER'ship
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Ruslan Makhmatkhanov
URL:
Keywords: patch, security
Depends on:
Blocks: 215988
  Show dependency treegraph
 
Reported: 2017-01-12 10:27 UTC by Damien Fleuriot
Modified: 2017-02-06 17:16 UTC (History)
2 users (show)

See Also:


Attachments
New Makefile (733 bytes, patch)
2017-01-12 10:28 UTC, Damien Fleuriot
no flags Details | Diff
New distinfo (389 bytes, patch)
2017-01-12 10:28 UTC, Damien Fleuriot
no flags Details | Diff
New pkg-descr (281 bytes, patch)
2017-01-12 10:28 UTC, Damien Fleuriot
no flags Details | Diff
New pkg-plist (1.05 KB, patch)
2017-01-12 10:29 UTC, Damien Fleuriot
no flags Details | Diff
New files/patch-lshell_checkconfig.py (586 bytes, patch)
2017-01-12 10:29 UTC, Damien Fleuriot
no flags Details | Diff
0.9.16 to 0.9.18 latest commit (3.26 KB, patch)
2017-01-13 12:13 UTC, Damien Fleuriot
no flags Details | Diff
bump lshell to latest GH tag (4.33 KB, patch)
2017-01-26 13:53 UTC, Damien Fleuriot
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Damien Fleuriot 2017-01-12 10:27:09 UTC
The current 0.9.16 version of lshell was released in 2013 and is subject to a shell escape (which, to be fair, kinda defeats the purpose of the package entirely).


This set of diff files :
- brings shells/lshell up to the upstream's version 0.9.18.
- changes the master site to github.
- handles the new file naming (lshell_$version instead of lshell-$version).
- sets me as maintainer, we use lshell regularly at work and will keep an eye on the upstream.
- adjusts my initial patch to replace /dev/log with /var/run/log, seeing the original file's offsets have changed.


Please note that files/patch-lshell_shellcmd.py needs to be removed as the upstream has been refactored since.


The new package works just fine for me on 10-STABLE.

Requesting peer review.



Closes:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=215988
Comment 1 Damien Fleuriot 2017-01-12 10:28:03 UTC
Created attachment 178779 [details]
New Makefile

Bumps version to 0.9.18
Changes master site
Adds maintainer
Handles distfile naming
Comment 2 Damien Fleuriot 2017-01-12 10:28:25 UTC
Created attachment 178780 [details]
New distinfo
Comment 3 Damien Fleuriot 2017-01-12 10:28:46 UTC
Created attachment 178782 [details]
New pkg-descr
Comment 4 Damien Fleuriot 2017-01-12 10:29:10 UTC
Created attachment 178783 [details]
New pkg-plist
Comment 5 Damien Fleuriot 2017-01-12 10:29:50 UTC
Created attachment 178784 [details]
New files/patch-lshell_checkconfig.py

Offsets in the upstream file have changed.
Comment 6 VK freebsd_triage 2017-01-12 10:44:54 UTC
Thanks for the patch. However, it would be preferable if you submitted a single svn diff patch instead of one patch per file, for example run `svn diff --ignore-properties shells/lshell > patchfile` in the svn tree root (eg. in /usr/ports).

It would also be preferable if you ran Poudriere test for all currently supported FreeBSD versions, and confirmed they passed (no need to attach build logs).

Also please sync up the e-mail address you use for MAINTAINER with the one you use here on Bugzilla as otherwise we cannot track maintainer notifications and approvals.
Comment 7 Damien Fleuriot 2017-01-12 11:04:41 UTC
Thank you for the feedback !


I'll correct those and post a more appropriate diff.

I'll see what I can do about Poudriere, my company does not have 11.x and HEAD machines at the current time.
Comment 8 VK freebsd_triage 2017-01-12 12:27:26 UTC
Just a note, it appears there are more vulnerabilities, as I've listed them in the related bug #215988. Perhaps you could ask the upstream to tag 0.9.19 and we upgrade to that, or else it might be wise to temporarily upgrade the port to latest commit hash. I don't know how stable that is, and it would require run tests.
Comment 9 Damien Fleuriot 2017-01-12 16:04:04 UTC
I've asked upstream for a new release incorporating the fixes which were published post 0.9.18 release :
https://github.com/ghantoos/lshell/issues/166


I've fixed the Makefile with regards to portlint's warnings, now working on "make makepatch", then Poudriere.

Will report back when there's progress :)
Comment 10 Damien Fleuriot 2017-01-13 12:13:39 UTC
Created attachment 178857 [details]
0.9.16 to 0.9.18 latest commit
Comment 11 Damien Fleuriot 2017-01-13 12:14:31 UTC
Kindly find attached the svn diff to bring lshell up to the latest commit hash 279d7ab [1]



* The port passes both 'portlint -C' and 'port test'.
* The offsets for files/patch-lshell_checkconfig.py have been adjusted to this latest commit hash.
* The patch files/patch-lshell_shellcmd.py needs to be removed since a lot of code was refactored in 0.9.17.

I am unclear as to whether I should handle the extraneous patch file's deletion in post-extract ?
Quote [2] : If a file must be deleted, do it in the post-extract target rather than as part of the patch.



Seeing as we're fetching against a commit hash, I've taken the liberty of setting PORTREVISION to the commit's date.

I believe this also elegantly solves the problem of having no 0.9.19 release.


I'll gladly heed any advice if you think there is yet room for improvement.




Will submit confirmation of poudriere runs as soon as I get a HEAD host up with the proper jails.
The port does pass "manual checks" on 10-STABLE , as in managing to run it and failing to exploit previous shell escape vulnerabilities.




[1] https://github.com/ghantoos/lshell/commit/279d7ab7980ed88c6628e6621918f6299a2e2e32
[2] https://www.freebsd.org/doc/en/books/porters-handbook/slow-patch.html
Comment 12 VK freebsd_triage 2017-01-13 12:44:12 UTC
PORTREVISION should generally increment monotonically, so in this case it's enough to bump it to 3, it must reset the next time PORTVERSION is bumped. Alternatively you could push PORTVERSION to 0.9.18.20160916, which would be more correct IMHO, and also 0.9.19 > 0.9.18.<something>. It's okay as long as you don't decrease the version, as that would require PORTEPOCH bump on next update.

You should remove patch files from shells/lshell/files/ with 'svn delete', the change will be recorded when you generate the diff with 'svn diff', if the patched code is now available in the updated upstream source. The quoted handbook entry applies to files that are in the unpackaged source ($WRKSRC) that have to be removed before building/packaging.
Comment 13 VK freebsd_triage 2017-01-13 12:44:44 UTC
Remove 'easy' keyword as the change is a bit more involved now, requiring run tests.
Comment 14 Damien Fleuriot 2017-01-13 13:21:50 UTC
Thanks for the input, I shall adjust PORTVERSION in stead of PORTREVISION for better clarity.


Just to be clear, the Poudriere tests need to be run against :
- 10.3R
- 10S
- 11.0R
- 11S

as per https://www.freebsd.org/security/security.html#sup ?
Comment 15 Damien Fleuriot 2017-01-26 13:53:39 UTC
Created attachment 179332 [details]
bump lshell to latest GH tag

Bump lshell to latest github tag.

Passes portlink -C, port test, and poudriere.
Comment 16 Damien Fleuriot 2017-01-26 13:54:32 UTC
Attached patch passes poudriere runs on the following amd64 jails :

10.3-RELEASE-p16
10.3-STABLE r312286
11.0-RELEASE-p7
11.0-STABLE r312279


The patch leaves in place .orig files which need to be removed.
The patch leaves in place files/patch-lshell_shellcmd.py (which I svn delete'd) with 0 bytes, which needs to be removed. (I assume that is expected)

The patch leverages PORTVERSION instead of PORTREVISION to include the tag's date.
Comment 17 commit-hook freebsd_committer 2017-02-06 16:31:14 UTC
A commit references this bug:

Author: rm
Date: Mon Feb  6 16:30:17 UTC 2017
New revision: 433496
URL: https://svnweb.freebsd.org/changeset/ports/433496

Log:
  shells/lshell: update to 0.9.18.20160916

  - pass maintainership to submitter
  - switch to github
  - add NO_ARCH, update project web-page and remove unneded patch

  This release fixing some vulnerabilities, that defeats the purpose
  of the package entirely, so this is why MFH is requested.

  PR:		215989
  Submitted by:	Damien Fleuriot <dam@my.gd>
  MFH:        2017Q1

Changes:
  head/shells/lshell/Makefile
  head/shells/lshell/distinfo
  head/shells/lshell/files/patch-lshell_checkconfig.py
  head/shells/lshell/files/patch-lshell_shellcmd.py
  head/shells/lshell/pkg-descr
  head/shells/lshell/pkg-plist
Comment 18 Ruslan Makhmatkhanov freebsd_committer 2017-02-06 17:11:02 UTC
Committed, thank you!

Please note, that this port supports python version 3, but port should be tweaked a bit to make it package with python3.
Comment 19 commit-hook freebsd_committer 2017-02-06 17:16:54 UTC
A commit references this bug:

Author: rm
Date: Mon Feb  6 17:16:00 UTC 2017
New revision: 433499
URL: https://svnweb.freebsd.org/changeset/ports/433499

Log:
  MFH: r433496

  shells/lshell: update to 0.9.18.20160916

  - pass maintainership to submitter
  - switch to github
  - add NO_ARCH, update project web-page and remove unneded patch

  This release fixing some vulnerabilities, that defeats the purpose
  of the package entirely, so this is why MFH is requested.

  PR:		215989
  Submitted by:	Damien Fleuriot <dam@my.gd>

  Approved by:	ports-secteam (feld)

Changes:
_U  branches/2017Q1/
  branches/2017Q1/shells/lshell/Makefile
  branches/2017Q1/shells/lshell/distinfo
  branches/2017Q1/shells/lshell/files/patch-lshell_checkconfig.py
  branches/2017Q1/shells/lshell/files/patch-lshell_shellcmd.py
  branches/2017Q1/shells/lshell/pkg-descr
  branches/2017Q1/shells/lshell/pkg-plist