Bug 595011

Summary: Review Request: sshdfilter - Filter for SSH ports
Product: [Fedora] Fedora Reporter: David Highley <dhighley>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED NOTABUG QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: 14CC: fedora, fedora-package-review, metherid, notting, pahan
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-12-22 19:30:58 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Attachments:
Description Flags
e-mail asking upstream to check this work
none
Full thread e-mail to upstream regarded this review none

Description David Highley 2010-05-22 16:02:44 EDT
Spec URL: http://www.highley-recommended.com/sshdfilter/sshdfilter.spec
SRPM URL: http://www.highley-recommended.com/sshdfilter/sshdfilter-1.5.5-1.fc12.src.rpm
Description: Filter for SSH ports
Perl script that monitors ssh connections looking for break in attempts. If a break in attempt is detected it dynamically creates a iptable block rule. Block rules are aged and removed after a configurable time. You can also restrict which accounts are able to use the connection. I have used it for several years. Script was created by Greg at http://ww.csc.liv.ac.ut/~greg/sshdfilter/ but does not seem to be maintain now. I have attempted to communicate with him about this packaging. I worked with Dominick Grift on selinux policy for this package as well.
Comment 1 Rahul Sundaram 2010-05-22 16:56:26 EDT
Is this your first package review?  If so, you need to set this to block FE-NEEDSPONSOR
Comment 2 Rafael Aquini 2010-05-22 18:19:36 EDT
David,

In spite of this is being an informal review, I ask you to consider some of the
following observations, please.

* As I could not find your Fedora Account, I don't know if this is your first
RPM package submission or not. If this is you first submission, take a closer
look at: http://fedoraproject.org/wiki/PackageMaintainers/Join

* As you stated that there is no longer upstream maintenance for sshdfilter you
should be prepared to maintain it as you were its own upstream.
Take a look at:
http://fedoraproject.org/wiki/PackageMaintainers/WhyUpstream#Some_Examples_Of_Exceptions

* I think there is no need to sshdfilter.spec explicitly list any 'Requires:'
at all. Even though, if they are really necessary, listing twice rsyslog is
surely unnecessary. For further reference on 'Requires:', take a look at:
http://fedoraproject.org/wiki/Packaging:Guidelines#Requires 

* You should write a  %changelog section, with useful stuff to keep track of
modifications made to the package.


* Under %install section of Spec:
  - There is no need for 'mkdir $RPM_BUILD_ROOT'
  - You should be using the '-p' option to preserve file timestamps
  - You shouldn't be forcing that .gz extention to man files. Let rpmbuild do
its job.
  - For the sake of legibility, consider rewrite spec's %install to sth like:

%install
rm -rf  %{buildroot}
install -p -m 0644 -D etc/%{name}rc     
%{buildroot}/%{_sysconfdir}/%{name}rc
install -p -m 0755 -D etc/init.d/%{name}
%{buildroot}/%{_sysconfdir}/rc.d/init.d/%{name}
install -p -m 0755 -D source/%{name}.pl  %{buildroot}/%{_sbindir}/%{name}
install -p -m 0644 -D man/%{name}.1      %{buildroot}/%{_mandir}/man1/%{name}.1
install -p -m 0644 -D man/%{name}rc.5   
%{buildroot}/%{_mandir}/man5/%{name}rc.5
mkdir -p %{buildroot}/%{_var}/run/
mkfifo -m 0644 %{buildroot}/%{_var}/run/%{name}.fifo


* Under %files, consider rewrite to sth like:
%files
%defattr(-,root,root,-)
%doc INSTALL todo
%{_sysconfdir}/%{name}rc
%{_sysconfdir}/rc.d/init.d/%{name}
%{_sbindir}/%{name}
%{_mandir}/man1/%{name}.1*
%{_mandir}/man5/%{name}rc.5*
%{_var}/run/%{name}.fifo


* Once RPM packages keep track of their own files, there is no need to do all
those 'rm -f' in %postun


* Please, consider a cleaner approach to modify config files owned by other
packages. 


Best regards
Comment 3 David Highley 2010-05-22 19:18:57 EDT
Rafael,

It is the first submission and I did create a Fedora account and subscribe to the required mailing lists. I did try and read the copious packaging guides. I understand that I would become the maintainer of this package. My one concern in that is having access to test on unreleased Fedora versions. I have made all the suggested corrections to the spec file, with the exception of the last one. 

I know of no way to cleanly deal with iptables and rsyslog. The script needs to read what is being logged to determine if port scans and ssh connections are being made. It needs to have a chain defined to drop block rules into the iptables rules. I know of no other way of accomplishing this, I'm open for ideas and felt that this was an issue as well for general distribution. There are issues with iptables as a restart will empty the block chain. But I thought the benefits out weighed the issues. I did add some more checking to deal with updating so that the rsyslog.conf and iptables files were not modified if they all ready had the needed modifications.

Thank you for the suggestion on cleaning up the spec file.
Comment 4 Rahul Sundaram 2010-05-22 19:23:47 EDT
Adding you to the FE-NEEDSPONSOR blocker.   Refer to

https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group
Comment 5 Rafael Aquini 2010-05-23 16:22:51 EDT
David

Please, consider the following suggestions:

TIP: In order to avoid confusion to future reviewers, for every modification you do to your package you should submit a new URL with your Spec and the new SRPM built from it.


* Take a closer look to where you are installing sshdfilter SysV initscript:
>  install -p -m 755 -D etc/init.d/%{name} \
>                        %{buildroot}%{_sysconfdir}/init.d/%{name}
  
 - A better location would be %{buildroot}/%{_sysconfdir}/rc.d/init.d/%{name} 
 Let me show you why:
> [aquini@optiplex ~]$ ls -l /etc/init.d
> lrwxrwxrwx. 1 root root 11 2009-12-28 20:17 /etc/init.d -> rc.d/init.d


* Since you are using the macro %doc to 'INSTALL', 'todo' and 'iptables.example' under %files, you don't have  a real necessity to install them under %install section (it's redundant). Actually it would be better if you just use %doc to do the job of installing those documents.


* I'm not a legal expert, but I think you should use GPL+ as Licence: instead of just GPL. 
 - http://fedoraproject.org/wiki/Packaging:Perl#License_tag


* To improve your changelogs, take a look at:
 - http://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs


I will be looking closer to your approach to do changes in config files, to see if I will be able to come up with some suggestion. Meanwhile, do not hesitate in apply any improvements you want to sshdfilter.


Best regards
Comment 6 Rafael Aquini 2010-05-23 22:15:53 EDT
David,

Since a picture is a lot more valuable than a thousand words, take this comment as an example, please.

Spec URL: http://aquini.tchesoft.com/RPMS/sshdfilter/sshdfilter.spec
SRPM URL: http://aquini.tchesoft.com/RPMS/sshdfilter/sshdfilter-1.5.5-2.fc12.src.rpm

MD5 sum:
8f0422d6150841ba08017faa3dcc1a52  sshdfilter.spec
ea2d0b58375debb7d337f2930d33dc2d  sshdfilter-1.5.5-2.fc12.src.rpm

As you will see, I stripped off from this Spec your former %post and %postun sections. Well, in plain words there is no simple and safe way to have such modifications done in those config files. What I'm trying to mean is there is no simple way to predict how those files will be found when an user tries to install, update or remove sshdfilter package. Even if we could find out how to get this task accomplished on its whole complexity, it will represent a lot of (complex) work to those two RPM sections. And for the worst, your package would be messing with other RPMs belongings (that's not so good...). 
So, said that my suggestion is: do not change those files and instruct users, through documentation, to do just after sshdfilter installation instead.

Of course all of that I've presented are just suggestions that you can consider or not.

Best regards.
Comment 7 David Highley 2010-05-24 09:29:21 EDT
I agree with most of the changes, but still think the post install should let the user know that they need to modify the iptables and rsyslog.conf files. If they do not get some kind of prompt then they maybe wondering how to get the package working. I can see not modifying them, some thing we do at work as we have to fix things so they install and uninstall cleanly. With most packages not requiring any user changes they are caught off guard when one is not able to automatically make all the changes needed.
Comment 8 Rahul Sundaram 2010-05-24 09:35:48 EDT
Either you got to modify whatever is needed to the best extend possible in the package itself or document it in something like README.Fedora.  Post-install echo is not sufficient for anyone not using yum on the command line.  This include users of PackageKit, Anaconda or kickstart
Comment 9 David Highley 2010-07-20 22:57:53 EDT
OK, after a hardware failure with my test box an operating system update and numerous other delays I hope I have addressed all concerns about this packaging. The package has been stripped down to the essentials and I wrote a completely new concise install file and now include a GNU license file. The upstream author responded to some of my changes and changed a few things for a new release. Please review:

Spec URL: http://www.highley-recommended.com/sshdfilter/sshdfilter.spec
SRPM URL: http://www.highley-recommended.com/sshdfilter/sshdfilter-1.5.7-1.fc13.src.rpm
RPM  URL: http://www.highley-recommended.com/sshdfilter/sshdfilter-1.5.7-1.fc13.noarch.rpm

MD5 sum:
09181a97d7534e19b146298447916bcc  sshdfilter.spec 98118c9ee2d99d8c2e6a8a0e0372b9bd  sshdfilter-1.5.7-1.fc13.src.rpm
5fff8501fb423aca31b19fb3c5e34071  sshdfilter-1.5.7-1.fc13.noarch.rpm
Comment 10 Rafael Aquini 2010-08-13 13:16:52 EDT
David,

Please, consider the following review:

Good:
* Package is named sshdfilter which follows the upstream project name
* spec file naming follows package naming
* License in spec and license text is GPL which is open source
* License text included in the tarball and listed on %file.
* Spec is legible and American English
* No locale files
* No shared libraries
* No bundled libraries
* Not relocatable
* No directories created unowned
* No duplicate files
* Default permissions are set
* Package is code
* No large documentation
* No %doc files are used at runtime
* No header files
* Not a GUI application
* Does not own files or directories from other packages
* All filenames are utf8


NEEDSWORK:
[1] ask upstream to include a license disclaimer as a header of sshdfilter perl script

[2] chkconfig line is missing in sshdfilter SysVInitScript. See http://fedoraproject.org/wiki/Packaging:SysVInitScript#.23_chkconfig:_line


[3] Source **DOES NOT** matches upstream:
* http://www.csc.liv.ac.uk/~greg/sshdfilter-1.5.7.tar.gz
   a022aeeb80b4a71e86d330d89dafb89d  Downloads/sshdfilter-1.5.7.tar.gz
   dda362b80c0a3f8297b08ff6f831fcbc  rpmbuild/SOURCES/sshdfilter-1.5.7.tar.gz



NOTE: [3] is a real Blocker to this package. You can't ship source that mismatch upstream's within your SRPM. That is forbidden accordingly to Packaging Guidelines and Review Policies. 
As I've wrote on Comment #2, if you are packaging a software without upstream maintenance, you should consider to take over the maintenance and become yourself this software upstream.


Regards
Comment 11 David Highley 2010-08-15 13:37:22 EDT
Good review, but here is the situation.

[1] Up stream only had a reference in a web page to being GPL. I asked which license and suggested that it should be added to the distribution. No answer.

[2] Can not do a "chkconfig on" as this application must be manually configured before it can run.

[3] Change notes clearly said that the RPM had been stripped down to just what was needed for the Fedora and RPM distribution. I guess you might consider it a fork. So I'm willing to maintain the fork.

I may just bail on getting it into the distribution as it seems that this application since it requires user configuration may not fit well the Fedora distribution. It also requires many selinux modifications before it will work. The selinux group is waiting for it to become a distribution package before they make changes. Kind of chicken or egg situation.
Comment 12 Rafael Aquini 2010-08-15 14:49:34 EDT
David,

Please consider the following:

[2] I did not mean you should do a 'chkconfig sshdfilter on', what I was trying to point out is that every SysV Inintscript must have a "chkconfig:" line in its header. That header line, which is missing in sshdfilter, tells chkconfig what runlevels are apropriated to running the script, and in what order the script will be started or killed on those runlevels. 

[1] & [3] Dealing with unresponsive upstream is a real issue for packagers. If sshdfilter's upstream has left the project behind, and you are really willing to get this software into Fedora, so I encourage you to fork it in another project. However, you can not modify the source package shipped by upstream before shipping it within you SRPM and consider it as a fork. The source inside a SRPM must always match its upstream's download.  


I don't think you should give up sshdfilter. It may sound cliché, but with no pain, no gain! Keep working on it because your goal, right now, should be getting this work reviewed and you sponsored to the packagers group. I don't have the power to sponsor you, but I can help reviewing your work and pointing the right way to you get it into Fedora. Also, you can count on me to help with any other matter regarded to this package. Feel free to contact me whenever you want.

Regards.
Comment 13 David Highley 2010-08-17 15:29:05 EDT
Hopefully I have addressed the three issues above adequately. I added the chkconfig line to the init script. Added a license reference in the Perl script. Changed the source distribution and location to my modifications.

Spec URL: http://www.highley-recommended.com/sshdfilter/sshdfilter.spec
SRPM URL:
http://www.highley-recommended.com/sshdfilter/sshdfilter-1.5.7-2.fc13.src.rpm
RPM  URL:
http://www.highley-recommended.com/sshdfilter/sshdfilter-1.5.7-2.fc13.noarch.rpm

I'm still not completely done with the modifications and documentations but I hope the packaging is now acceptable. If not please let me know. Thanks
Comment 14 David Highley 2010-08-17 15:29:49 EDT
Darn, forgot the signatures.

2ec3b2303fa8f9f6bf8c6b44d59cc694  sshdfilter.spec
e8b241f49d779e5b9a8c262bb15067d8  sshdfilter-1.5.7-2.fc13.noarch.rpm
986932d80e17a7122d2f29f4d25722a8  sshdfilter-1.5.7-2.fc13.src.rpm
Comment 15 Rafael Aquini 2010-08-17 19:54:17 EDT
Created attachment 439259 [details]
e-mail asking upstream to check this work
Comment 16 Rafael Aquini 2010-08-17 19:58:04 EDT
David,

Let me explain better my last comment, about forking a project (comment 12):

[1] It might be acceptable, If and only if upstream is dead. Which means that no answer or releases came from upstream for a long time, and no one else has took the software maintenance. (I guess I didn't explain it clearly before). Please, take a closer look at:  
    http://fedoraproject.org/wiki/PackageMaintainers/WhyUpstream

[2] Sshdfilter does not look like a dead prokect. There are activity reported in its website, and you have some answers, also (comment 9). Perhaps, its developers are just overbusy. Submit them your patches and modifications, and be patient. I bet they will answer you!


In order to help you, I've mailed Greg, which seems to be the developer responsible for sshdfilter, to let him aware of this on going work. Let's just wait his answer.

Meanwhile, you can do some stuff to show sponsors your undertanding of Fedora Guidelines. Help other packagers having their work improved, and improve yourself through reviewing informally other review requests!

  http://fedoraproject.org/PackageReviewStatus/NEW.html


Take a look at following URLs, to see how:
http://fedoraproject.org/wiki/PackageReviewProcess
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored


Regards.
Comment 17 Rafael Aquini 2010-08-26 21:52:25 EDT
Created attachment 441373 [details]
Full thread e-mail to upstream regarded this review
Comment 18 Rafael Aquini 2010-08-26 21:53:34 EDT
David,

It has been less than 10 days, and I've got two replies from sshdfilter's upstream developer. That's great for your work here, as you won't need to cope with forking this software and take all this burden, just to fit it into Fedora guidelines. Even better than that, you will be fitting it into guidelines and yet you will have support from its upstream, which seems to be very cooperative and interested on keep sshdfilter improving without making it specific.

Said that, please consider the following approach in order to release a new version of sshdfilter's SRPM:

[1] Download & ship sshdfilter upstream's tarball within your SRPM -- Always;
[2] Whatever you may need to modify in the original source, do it through PATCHES.
    Patches can be applied to original tarball at SPEC %setup session. Remember to comment your patches in SPEC.
[3] Share your work with community, submitting those patches to upstream. Greg will be glad in having them and publishing them at sshdfilter project website.

Looking forward your reply,

Best regards.

P.S.: Feel free to contact me whenever you need, I'll be avaiable to help you if you want.
Comment 19 Rafael Aquini 2010-08-26 21:56:50 EDT
Small Fix:

**Patches can be applied to the original sources at SPEC %prep session**

Sorry!

Best Regards
Comment 20 Rafael Aquini 2010-11-07 19:41:07 EST
David,

There has been almost two months since your last update to this bug. Are you still interested in packaging sshdfilter? If so, please consider posting updates here soon. If you are getting trouble in be sponsored, I can help you pointing out what have to be done prior to your sponsorship. While I do not possess such power, I'm still willing to help you get sponsored.

Best regards
Comment 21 David Highley 2010-11-07 23:52:13 EST
I have not heard any feed back on the last changes. So I'm thinking I will see if I can put this on the wish list and see if some one who has maintainer rights is interested in it.
Comment 22 David Highley 2010-11-17 14:30:25 EST
OK, added another minor fix for boot up hang where the fifo was not open and sshdfilter would hang. Modification was to restart rsyslog service if when starting sshdfilter the fifo needed to be created.

Spec URL: http://www.highley-recommended.com/sshdfilter/sshdfilter.spec
SRPM URL:
http://www.highley-recommended.com/sshdfilter/sshdfilter-1.5.7-5.fc14.src.rpm
RPM  URL:
http://www.highley-recommended.com/sshdfilter/sshdfilter-1.5.7-5.fc14.noarch.rpm

I'm off to look for a package reviewer and sponsor.
Comment 23 David Highley 2010-11-18 21:22:19 EST
Found and fixed a typo in the init.d/sshdfilter file. Should have tested for fifo not existing, transcription error.

Spec URL: http://www.highley-recommended.com/sshdfilter/sshdfilter.spec
SRPM URL:
http://www.highley-recommended.com/sshdfilter/sshdfilter-1.5.7-6.fc14.src.rpm
RPM  URL:
http://www.highley-recommended.com/sshdfilter/sshdfilter-1.5.7-6.fc14.noarch.rpm
Comment 24 David Highley 2010-11-19 11:50:27 EST
Fixed another init script issue where the non standard pid file name required a direct file check instead of using the pidfileofproc function call.

Spec URL: http://www.highley-recommended.com/sshdfilter/sshdfilter.spec
SRPM URL:
http://www.highley-recommended.com/sshdfilter/sshdfilter-1.5.7-7.fc14.src.rpm
RPM  URL:
http://www.highley-recommended.com/sshdfilter/sshdfilter-1.5.7-7.fc14.noarch.rpm
Comment 25 Jason Tibbitts 2010-12-14 12:22:28 EST
From reading the comments I can't tell if you intend to submit this to Fedora or if you're just posting this here in case someone else wants to take it and add it to the distribution (comment 21).  So, which is it?  If you wish to submit this yourself, have you read http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group?  Have you done any other packaging or review work?
Comment 26 David Highley 2010-12-14 22:12:46 EST
Well it seems to take forever to make progress on this package. I intended originally to submit and maintain this package. I have read the get sponsored information and went so far as to contact someone I had communications with in the past who could sponsor me but did not get a reply.

I have also been communicating changes I think are needed back to the original author but get little interest. I still think it improves the security of an unrestricted ssh port to the open internet. With all the delays and lack of interest from the original author I guess my interest is fading at this point.
Comment 27 Jason Tibbitts 2010-12-22 14:39:42 EST
I understand that it can take some time to get a package accepted into the distribution, but the time generally decreases with the amount of effort you put in.  Since you say you've read the information on being sponsored, I have to ask where is the information on the package review work you've done yourself?  Why did you not answer my question about that?

In order for someone to sponsor you and basically allow you access to the machines of all Fedora users, the sponsors want some evidence that you understand packaging to some minimum level.  The best (but not the only) way to do that it to go about and look at the other review tickets (there are pl[enty) and help to improve those packages.  If you're not willing to do that, nobody's going to step up and take some responsibility for your actions by sponsoring you.  I'm certainly not going to do that.

So, if you really want the process to move forward, please look over http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group
Comment 28 David Highley 2010-12-22 19:30:58 EST
Sorry I have lost interest in this effort. I'm closing out the report.