Bug 494171

Summary: Review Request: hostapd - WLAN Accesspoint daemon
Product: [Fedora] Fedora Reporter: Adel Gadllah <adel.gadllah>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED DUPLICATE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: bruno, dcbw, fedora-package-review, goeran, gwync, herrold, kedars, kwizart, lemenkov, linville, marcin, mclroy, notting, tom, tomspur
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: 2009-11-01 12:57:35 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Adel Gadllah 2009-04-05 10:54:44 UTC
Spec URL: http://193.200.113.196/apache2-default/rpm/hostapd.spec
SRPM URL: http://193.200.113.196/apache2-default/rpm/hostapd-0.6.9-0.1.20090405gita0b2f99.fc10.src.rpm
Description: 
hostapd is a user space daemon for access point and authentication servers.
It implements IEEE 802.11 access point management,
IEEE 802.1X/WPA/WPA2/EAP Authenticators, RADIUS client, EAP server,
and RADIUS authentication server. 

----
rpmlint is silent,
koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1277840

Comment 1 Fabian Affolter 2009-04-05 16:30:37 UTC
Perhaps you are interested in the previous review request of hostap.

https://bugzilla.redhat.com/show_bug.cgi?id=230449

Comment 2 Adel Gadllah 2009-04-05 17:22:05 UTC
(In reply to comment #1)
> Perhaps you are interested in the previous review request of hostap.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=230449  

I know, we talked on IRC about this and the conses was that we should make a new package (upstream issues are fixed in the kernel now and the userspace parts in hostapd git, hence why I have built a git snapshot).

Comment 3 Marcin Łabanowski 2009-04-08 18:35:26 UTC
Note that it's a practice review, I need to do some in order to get a sponsoring.

* Naming: OK
* Version and release: OK
* Legal: OK
* No inclusion of pre-built binaries or libraries: OK
* Spec Legibility: OK
* Writing a package from scratch: OK
* Modifying an existing package: N/A
* Architecture Support: koji has built the package correctly, so I assume OK
* Filesystem layout: OK
- %{_datadir}/man/man5/hostapd.* - did you consider using %{_mandir} ?
* Use rpmlint: OK
* Changelogs: OK
* Tags: OK
* BuildRoot tag: OK
* Requires: OK
* BuildRequires: OK
* Summary and description: OK
* Documentation: not-OK
- Did you think about including hostapd/README?
- Did you think about building the documentation from hostapd/doc/ directory? I've made you a list of buildrequires for that: transfig, netpbm-progs, doxygen, graphviz, texlive-latex. I'm not sure if it's really necessary, since it contains information about API and friends and would make a little use for people outside the project.
* Compiler flags: OK
* Debuginfo packages: OK
* Devel packages: N/A
* Requiring Base Package: N/A
* Shared Libraries: N/A
* Packaging Static Libraries: N/A
* Duplication of system libraries: OK
* Beware of Rpath: OK
* Configuration files: OK
* Initscripts: not-OK
- The package would certainly benefit from initscript, since it's a system daemon.
* Desktop files: N/A
* Macros: mainly OK
- See my comment for "Filesystem layout"
- You are mixing %{optflags} with $RPM_BUILD_ROOT. In my opinion it's not really bad, 
* Handling Locale Files: N/A, hostapd is not localised.
* Timestamps: not-OK
- Add -p to your install commands.
* Parallel make: OK
* Scriptlets: OK
* Conditional dependencies: OK
* Build packages with separate user accounts: not-OK ;)
* Relocatable packages: OK
* Code Vs Content: OK
* File and Directory Ownership: OK
* Users and Groups: N/A, hostapd requires root priviledges, doesn't it?
* Web Applications: N/A
* Conflicts: OK
* No External Kernel Modules: OK
* No Files or Directories under /srv: OK
* Bundling of multiple projects: OK
* All patches should have an upstream bug link or comment: not-OK
- The Patch1 is Fedora-specific, but it doesn't say so. There is no description of it neither.
* Application Specific Guidelines: N/A

Comment 4 Gwyn Ciesla 2009-04-08 19:08:05 UTC
Minor nitpick, linked spec and SRPM spec differ. . .

< URL:          http://hostap.epitest.fi/hostapd/      
< # Generate tarball:
< # git clone git://w1.fi/srv/git/hostap.git
< # cd hostap
< # git-archive --format=tar --prefix=hostapd-0.6.9/ %{gitversion} | bzip2 > hostapd.`date +%Y%m%d`git%{gitversion}.tar.bz2     
---
> URL:          http://hostap.epitest.fi/hostapd/    

I assume the linked one is preferred, and is what my meta-review is of.

Otherwise, a pretty good review, particulart WRT the docs and initscripts.

Re: Macros, %{optflags} and $RPM_BUILD_ROOT have little to do with each other.  One is a macro and one a variable.  Using each is fine.  What is not recommended is mixing %{buildroot} and $RPM_BUILD_ROOT, you should pick one.

Upstream patch status would be nice as well.

Comment 5 Marcin Łabanowski 2009-04-08 19:17:03 UTC
Thank you.

> Re: Macros, %{optflags} and $RPM_BUILD_ROOT have little to do with each other. 
> One is a macro and one a variable.  Using each is fine.  What is not
> recommended is mixing %{buildroot} and $RPM_BUILD_ROOT, you should pick one.
I saw a review, where reviewer criticised the mixing, that's why I wasn't sure.

Comment 6 Adel Gadllah 2009-04-08 19:30:46 UTC
(In reply to comment #3)
> Note that it's a practice review, I need to do some in order to get a
> sponsoring.

OK, thanks for looking at the package.

> * Naming: OK
> * Version and release: OK
> * Legal: OK
> * No inclusion of pre-built binaries or libraries: OK
> * Spec Legibility: OK
> * Writing a package from scratch: OK
> * Modifying an existing package: N/A
> * Architecture Support: koji has built the package correctly, so I assume OK
> * Filesystem layout: OK
> - %{_datadir}/man/man5/hostapd.* - did you consider using %{_mandir} ?

OK, will fix.

> * Use rpmlint: OK
> * Changelogs: OK
> * Tags: OK
> * BuildRoot tag: OK
> * Requires: OK
> * BuildRequires: OK
> * Summary and description: OK
> * Documentation: not-OK
> - Did you think about including hostapd/README?

Missed that one, thanks will add.

> - Did you think about building the documentation from hostapd/doc/ directory?
> I've made you a list of buildrequires for that: transfig, netpbm-progs,
> doxygen, graphviz, texlive-latex. I'm not sure if it's really necessary, since
> it contains information about API and friends and would make a little use for
> people outside the project.

This docs are mostly useless. 
There are only relevant if you want to hack on hostapd itself, in this case you can build them or read the comments in the code. 

> * Compiler flags: OK
> * Debuginfo packages: OK
> * Devel packages: N/A
> * Requiring Base Package: N/A
> * Shared Libraries: N/A
> * Packaging Static Libraries: N/A
> * Duplication of system libraries: OK
> * Beware of Rpath: OK
> * Configuration files: OK
> * Initscripts: not-OK
> - The package would certainly benefit from initscript, since it's a system
> daemon.

Sure can add one.

> * Desktop files: N/A
> * Macros: mainly OK
> - See my comment for "Filesystem layout"
> - You are mixing %{optflags} with $RPM_BUILD_ROOT. In my opinion it's not
> really bad, 

That does not matter at all.

> * Handling Locale Files: N/A, hostapd is not localised.
> * Timestamps: not-OK
> - Add -p to your install commands.

OK

> * Parallel make: OK
> * Scriptlets: OK
> * Conditional dependencies: OK
> * Build packages with separate user accounts: not-OK ;)

???

> * Relocatable packages: OK
> * Code Vs Content: OK
> * File and Directory Ownership: OK
> * Users and Groups: N/A, hostapd requires root priviledges, doesn't it?

Yes it does, so what? ;) (has nothing to do on how it is packaged, it will complain if you don't run it as root).

Moved the files to /usr/sbin/ instead of /usr/bin/ to make this more clear.

> * Web Applications: N/A
> * Conflicts: OK
> * No External Kernel Modules: OK
> * No Files or Directories under /srv: OK
> * Bundling of multiple projects: OK
> * All patches should have an upstream bug link or comment: not-OK
> - The Patch1 is Fedora-specific, but it doesn't say so. There is no description
> of it neither.

The patch is obivious (fixes the path in the makefile), but sure can add a comment.


> * Application Specific Guidelines: N/A  

New spec & package:

http://193.200.113.196/apache2-default/rpm/hostapd.spec
SRPM URL:
http://193.200.113.196/apache2-default/rpm/hostapd-0.6.9-0.2.20090405gita0b2f99.fc10.src.rpm

Comment 7 Gwyn Ciesla 2009-04-08 19:45:58 UTC
>> * Build packages with separate user accounts: not-OK ;)
>
>???

I missed this in the last pass.  What's this about?

Comment 8 Marcin Łabanowski 2009-04-08 20:40:52 UTC
> Yes it does, so what? ;) (has nothing to do on how it is packaged, it will
> complain if you don't run it as root).
>
> Moved the files to /usr/sbin/ instead of /usr/bin/ to make this more clear.
I thought about priviledge separation, like some daemons do. If hostapd supported that in its configuration files, or command line, adding an user would have been beneficial.

>> * Build packages with separate user accounts: not-OK ;)
> ???
https://fedoraproject.org/wiki/Packaging/Guidelines#Build_packages_with_separate_user_accounts
My wild guess was that you didn't build the package as the separate user, because there was a rather generic username set as owner of the files in the SRPM ;).

Comment 9 Adel Gadllah 2009-04-08 22:28:22 UTC
(In reply to comment #8)
> > Yes it does, so what? ;) (has nothing to do on how it is packaged, it will
> > complain if you don't run it as root).
> >
> > Moved the files to /usr/sbin/ instead of /usr/bin/ to make this more clear.
> I thought about priviledge separation, like some daemons do. If hostapd
> supported that in its configuration files, or command line, adding an user
> would have been beneficial.

It needs access to the kernel interfaces which are only available to root.

> >> * Build packages with separate user accounts: not-OK ;)
> > ???
> https://fedoraproject.org/wiki/Packaging/Guidelines#Build_packages_with_separate_user_accounts
> My wild guess was that you didn't build the package as the separate user,
> because there was a rather generic username set as owner of the files in the
> SRPM ;).  

... whatever

---
New spec & srpm:
http://193.200.113.196/apache2-default/rpm/hostapd.spec
http://193.200.113.196/apache2-default/rpm/hostapd-0.6.9-0.3.20090405gita0b2f99.fc10.src.rpm  

Changed the license to BSD.

"These program is dual-licensed under both the GPL version 2 and BSD
license. Either license may be used at your option."

As we build against openssl (which license is not compatible with the GPLv2) I opted for BSD.

Comment 10 Gwyn Ciesla 2009-04-09 12:48:26 UTC
>> >> * Build packages with separate user accounts: not-OK ;)
>> > ???
>> https://fedoraproject.org/wiki/Packaging/Guidelines#Build_packages_with_separate_user_accounts
>> My wild guess was that you didn't build the package as the separate user,
>> because there was a rather generic username set as owner of the files in the
>> SRPM ;).  
>
>... whatever

Pretty much.  It doesn't matter what user you build the SRPM as, the ownership is set in the %files attr and defattr sections.  Of course, don't build as root, but. . .

Comment 11 Nicolas Chauvet (kwizart) 2009-04-15 10:14:57 UTC
why is this build against a git snapshot whereas http://hostap.epitest.fi/releases/hostapd-0.6.9.tar.gz is here ?

Comment 12 Nicolas Chauvet (kwizart) 2009-04-15 11:03:30 UTC
What is needed WRT openssl patches subdirectory. (given openssl version in F-10 openssl-0.9.8g-12.fc10.x86_64 / F-11 openssl-0.9.8k-1.fc11.x86_64.rpm ).

About the initscript. having /etc/hostapd.conf used as a configuration file seems nice but could be better with using /etc/hostapd/hostapd.conf instead) Then the current configuration file should be provided as an example only. 
So the initscript should exit 6 if there is no configuration file.

- I don't know what is mean by $OTHER_ARGS in the initscript.

- Logwach support is lacking.

Comment 13 Adel Gadllah 2009-04-15 14:36:43 UTC
(In reply to comment #11)
> why is this build against a git snapshot whereas
> http://hostap.epitest.fi/releases/hostapd-0.6.9.tar.gz is here ?  

Because it is needed to support mac80211 based drivers (works with anything but intel right now, intel support is missing on the driver side).

where 0.6.9 only supports older drivers or out of tree stuff like madwifi.

(In reply to comment #12)
> What is needed WRT openssl patches subdirectory. (given openssl version in F-10
> openssl-0.9.8g-12.fc10.x86_64 / F-11 openssl-0.9.8k-1.fc11.x86_64.rpm ).

The patches are for supporting EAP-FAST, they need to be applied to openssl and so this is out of the scope of this review.

> About the initscript. having /etc/hostapd.conf used as a configuration file
> seems nice but could be better with using /etc/hostapd/hostapd.conf instead)
> Then the current configuration file should be provided as an example only. 
> So the initscript should exit 6 if there is no configuration file.

OK, will change that.

> - I don't know what is mean by $OTHER_ARGS in the initscript.

Will remove.

> - Logwach support is lacking.  

OK, will add it.

Comment 14 Michael 2009-06-16 22:47:15 UTC
I wonder there is still no rpm for the hostapd in Fedora despite I see talks and I even see "Nobody's working on this" above. So I had to build it on my own, well, with the help of all the people out there. I'm happy now playing with it around. Well, a long story short.
Here is my short story: http://sites.google.com/site/mclroy/hostapd
and my rpm stuff:
http://sites.google.com/site/mclroy/hostapd/file/hostapd.spec
http://sites.google.com/site/mclroy/hostapd/file/hostapd-0.6.9-1.fc10.src.rpm
It works well with driver=wired and ieee8021x=1 for EAP-TLS authentication with freeradius. I think it is enough for inclusion into Fedora for educational purposes at least.

==> /var/log/messages <==
Jun 12 23:35:59 ex hostapd: eth0: STA 00:16:d3:ef:e1:ed IEEE 802.11: disassociated due to inactivity
Jun 12 23:36:00 ex hostapd: eth0: STA 00:16:d3:ef:e1:ed IEEE 802.11: deauthenticated due to inactivity
Jun 12 23:37:10 ex hostapd: eth0: STA 00:16:d3:ef:e1:ed RADIUS: starting accounting session 4A32A8A8-0000000A
Jun 12 23:37:10 ex hostapd: eth0: STA 00:16:d3:ef:e1:ed IEEE 802.1X: authenticated - EAP type: 13 (TLS)

==> /var/log/radius/radius.log <==
Fri Jun 12 23:37:10 2009 : Auth: Login OK: [1D] (from client localhost port 0 cli 00-16-D3-EF-E1-ED)

Some tcpdump:
00:16:d3:ef:e1:ed > 01:80:c2:00:00:03, ethertype EAPOL (0x888e), length 60: EAP code=1 id=1 length=0   
00:1d:72:37:8b:c4 > 00:16:d3:ef:e1:ed, ethertype EAPOL (0x888e), length 23: EAP code=2 id=0 length=5   
00:16:d3:ef:e1:ed > 01:80:c2:00:00:03, ethertype EAPOL (0x888e), length 60: EAP code=1 id=0 length=7   
00:1d:72:37:8b:c4 > 00:16:d3:ef:e1:ed, ethertype EAPOL (0x888e), length 24: EAP code=2 id=0 length=6   
00:16:d3:ef:e1:ed > 01:80:c2:00:00:03, ethertype EAPOL (0x888e), length 131: EAP code=1 id=0 length=113  
00:1d:72:37:8b:c4 > 00:16:d3:ef:e1:ed, ethertype EAPOL (0x888e), length 1042: EAP code=2 id=0 length=1024  
00:16:d3:ef:e1:ed > 01:80:c2:00:00:03, ethertype EAPOL (0x888e), length 60: EAP code=1 id=0 length=6   
00:1d:72:37:8b:c4 > 00:16:d3:ef:e1:ed, ethertype EAPOL (0x888e), length 1042: EAP code=2 id=0 length=1024  
00:16:d3:ef:e1:ed > 01:80:c2:00:00:03, ethertype EAPOL (0x888e), length 60: EAP code=1 id=0 length=6   
00:1d:72:37:8b:c4 > 00:16:d3:ef:e1:ed, ethertype EAPOL (0x888e), length 326: EAP code=2 id=0 length=308  
00:16:d3:ef:e1:ed > 01:80:c2:00:00:03, ethertype EAPOL (0x888e), length 1510: EAP code=1 id=0 length=1492  
00:1d:72:37:8b:c4 > 00:16:d3:ef:e1:ed, ethertype EAPOL (0x888e), length 24: EAP code=2 id=0 length=6   
00:16:d3:ef:e1:ed > 01:80:c2:00:00:03, ethertype EAPOL (0x888e), length 86: EAP code=1 id=0 length=68  
00:1d:72:37:8b:c4 > 00:16:d3:ef:e1:ed, ethertype EAPOL (0x888e), length 87: EAP code=2 id=0 length=69  
00:16:d3:ef:e1:ed > 01:80:c2:00:00:03, ethertype EAPOL (0x888e), length 60: EAP code=1 id=0 length=6   
00:1d:72:37:8b:c4 > 00:16:d3:ef:e1:ed, ethertype EAPOL (0x888e), length 22: EAP code=2 id=0 length=4

Comment 15 Dan Williams 2009-09-11 19:09:16 UTC
The hostapd openssl EAP-FAST patches already got upstreamed into openssl >= 0.9.8j (F11 and later).

https://bugzilla.redhat.com/show_bug.cgi?id=428181

so those patches shouldn't be relevant anymore.

Comment 16 Adel Gadllah 2009-09-29 08:11:21 UTC
Update from my side, as I no longer have my ath9k card I cannot test if stuff actually work (my other cards are intel which do not support AP mode yet), so if someone else want to work on this package feel free to do so.

Comment 17 Kedar Sovani 2009-10-16 05:55:47 UTC
This is still not included in Fedora.

Michael, you seem to have an almost-ready package. It still says "Nobody is working on it". I am looking forward for this package. If you aren't, I could take it?

Comment 18 Michael 2009-10-16 13:21:37 UTC
(In reply to comment #17)
> If you aren't, I could take it?
Sure.

Comment 19 Thomas Spura 2009-11-01 12:57:35 UTC
(In reply to comment #16)
> Update from my side, as I no longer have my ath9k card I cannot test if stuff
> actually work (my other cards are intel which do not support AP mode yet), so
> if someone else want to work on this package feel free to do so.  

The original submitter so longer won't to package this and Kedar maybe wants to take it…

This means, Kedar should make a own review requrest and close this as a DUBPLICATE of the new one.

Till then WONTFIX.

Comment 20 John W. Linville 2009-12-16 21:15:17 UTC

*** This bug has been marked as a duplicate of bug 548180 ***