Bug 465362 - Review Request: openconnect -- client for Cisco AnyConnect VPN
Review Request: openconnect -- client for Cisco AnyConnect VPN
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Peter Robinson
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2008-10-02 17:31 EDT by David Woodhouse
Modified: 2014-08-04 09:55 EDT (History)
9 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2008-12-18 12:44:18 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
pbrobinson: fedora‑review+
limburgher: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description David Woodhouse 2008-10-02 17:31:40 EDT
Spec URL: http://david.woodhou.se/anyconnect.spec
SRPM URL: ftp://ftp.infradead.org/pub/anyconnect-f9/anyconnect-0.91-1.src.rpm
Description: This package provides a client for Cisco's "AnyConnect" VPN, which uses HTTPS and DTLS protocols.
Comment 2 David Woodhouse 2008-10-06 19:57:54 EDT
When I use the NetworkManager plugin, I get this...

host=macbook.infradead.org type=AVC msg=audit(1223329615.277:1689): avc: denied { read write } for pid=13785 comm="openconnect" name="tun" dev=tmpfs ino=1866 scontext=unconfined_u:system_r:NetworkManager_t:s0 tcontext=system_u:object_r:tun_tap_device_t:s0 tclass=chr_file host=macbook.infradead.org type=SYSCALL msg=audit(1223329615.277:1689): arch=c000003e syscall=2 success=yes exit=4 a0=40a8c1 a1=2 a2=1e5 a3=7fffb4a6cc90 items=0 ppid=13768 pid=13785 auid=500 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=68 comm="openconnect" exe="/usr/bin/openconnect" subj=unconfined_u:system_r:NetworkManager_t:s0 key=(null)
Comment 3 David Woodhouse 2008-10-07 11:04:33 EDT
koji test build at http://koji.fedoraproject.org/koji/taskinfo?taskID=866710

rpmlint says:
3 packages and 0 specfiles checked; 0 errors, 0 warnings.
Comment 4 Daniel Walsh 2008-10-08 07:55:04 EDT
If you chcon -t vpnc_exec_t openconnect, does the connection work without any avc messages?
Comment 5 David Woodhouse 2008-10-08 08:22:53 EDT
Yes, it works fine then. Thanks.

I think it gives me a bunch of permissions I don't need, but certainly works.
Comment 6 Daniel Walsh 2008-10-08 18:33:13 EDT
What is the path to openconnect?
Comment 7 Peter Jones 2008-10-09 11:04:40 EDT
> %build
> make %{?_smp_mflags}
> %install
> rm -rf $RPM_BUILD_ROOT
> make install DESTDIR=$RPM_BUILD_ROOT

"make" should be using CFLAGS="%{optflags}", and %install should be using %makeinstall (though I'm not sure the latter will make any difference in this case at all.)
Comment 8 David Woodhouse 2008-10-09 11:10:31 EDT
The Makefile in the package uses $RPM_OPT_FLAGS if set, so there should be no
need to override...

    ifdef RPM_OPT_FLAGS
    OPT_FLAGS := -O2 -g -Wall

I've confirmed that the appropriate make flags do get used.

You're right; make install won't make any difference :)
Comment 9 Peter Jones 2008-10-09 11:11:42 EDT
rpmlint results:

pjones2:~/Download$ rpmlint openconnect-0.94-2.fc9.src.rpm openconnect.spec 
1 packages and 1 specfiles checked; 0 errors, 0 warnings.
Comment 10 Peter Jones 2008-10-09 11:16:13 EDT
COPYING.LGPL should be listed in %files as a %doc .
Comment 11 David Woodhouse 2008-10-09 11:24:09 EDT
Spec URL: http://david.woodhou.se/openconnect.spec
SRPM URL: ftp://ftp.infradead.org/pub/openconnect-f9/openconnect-0.94-3.fc9.src.rpm

Use %makeinstall, include COPYING.LGPL
Comment 12 Jason Tibbitts 2008-10-09 11:34:28 EDT
Wait, why %makeinstall?  We have an explicit prohibition against it unless the usual "make install DESTDIR=" doesn't work.
Comment 13 David Woodhouse 2008-10-09 11:37:41 EDT
Er, OK. I'll switch back.
Comment 14 Dominik 'Rathann' Mierzejewski 2008-10-09 14:53:23 EDT
(In reply to comment #7)
> and %install should be using
> %makeinstall (though I'm not sure the latter will make any difference in this
> case at all.)

Definitely NOT. https://fedoraproject.org/wiki/Packaging/Guidelines#Why_the_.25makeinstall_macro_should_not_be_used
Comment 15 David Woodhouse 2008-10-09 18:21:11 EDT
Spec URL: http://david.woodhou.se/openconnect.spec
SRPM URL: ftp://ftp.infradead.org/pub/openconnect-f9/openconnect-0.95-1.fc9.src.rpm

No %makeinstall. New upstream version with a few minor bug fixes.
Comment 16 Sergey Svishchev 2008-11-02 08:47:19 EST
The SRPM is not fetchable at the moment (openconnect-f9: Failed to change directory).
Comment 18 David Woodhouse 2008-11-24 19:42:11 EST
Oops, sorry. Make that...

Spec URL: http://david.woodhou.se/openconnect.spec
SRPM URL: http://david.woodhou.se/openconnect-0.98-1.fc9.src.rpm
Comment 19 Peter Robinson 2008-12-16 14:56:02 EST
This currently doesn't build for rawhide in koji
Comment 20 David Woodhouse 2008-12-16 15:06:44 EST
Oops. looks like I forgot to update the BuildRequires and recheck in mock after I added the NetworkManager bits. Sorry.

Testing this for myself now...

Spec URL: http://david.woodhou.se/openconnect.spec
SRPM URL: http://david.woodhou.se/openconnect-0.99-1.fc9.src.rpm
Comment 21 David Woodhouse 2008-12-16 15:10:27 EST
That worked.
Comment 22 Peter Robinson 2008-12-16 15:12:42 EST
Cool. I saw the NM errors but thought it might have been something to with the other package.
Comment 23 Peter Robinson 2008-12-16 16:04:23 EST
All looks good. The only issue is that the new tar ball isn't up on the ftp site to verify the md5sum. Other than that its APPROVED.

+ rpmlint output

$ rpmlint -i openconnect-0.99-1.fc9.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

+ package name satisfies the packaging naming guidelines
+ specfile name matches the package base name
+ package should satisfy packaging guidelines
+ license meets guidelines and is acceptable to Fedora
+ license matches the actual package license

+ %doc includes license file
+ spec file written in American English
+ spec file is legible
? upstream sources match sources in the srpm
  5dbb4f92b31fc81d8b9a59083eb160f7  openconnect-0.99.tar.gz
+ package successfully builds on at least one architecture
  built fine in koji
+ BuildRequires list all build dependencies
N/A %find_lang instead of %{_datadir}/locale/*
N/A binary RPM with shared library files must call ldconfig in %post and %postun
+ does not use Prefix: /usr
+ package owns all directories it creates
n/a no duplicate files in %files
+ %defattr line
+ %clean contains rm -rf $RPM_BUILD_ROOT
+ consistent use of macros
+ package must contain code or permissible content
n/a large documentation files should go in -doc subpackage
+ files marked %doc should not affect package
n/a header files should be in -devel
n/a static libraries should be in -static
n/a packages containing pkgconfig (.pc) files need 'Requires: pkgconfig'
n/a libfoo.so must go in -devel
n/a devel must require the fully versioned base
+ packages should not contain libtool .la files
n/a packages containing GUI apps must include %{name}.desktop file
+ packages must not own files or directories owned by other packages
+ %install must start with rm -rf %{buildroot} etc.
+ filenames must be valid UTF-8


n/a if there is no license file, packager should query upstream
n/a translations of description and summary for non-English languages, if
+ reviewer should build the package in mock
+ the package should build into binary RPMs on all supported architectures
+ review should test the package functions as described (basic testing but don't have a Cisco Anyconnect VPN currently available)
+ scriptlets should be sane
n/a pkgconfig files should go in -devel
+ shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or
Comment 24 Peter Robinson 2008-12-16 16:17:47 EST
BTW I noticed a README.secureid in the git repos that wasn't included. Coming from a support point of view in might be useful if it was included as docs in a release.
Comment 25 David Woodhouse 2008-12-16 16:25:18 EST
Thanks; I'll include README.SecurID.

The tarball should be available:
Comment 26 David Woodhouse 2008-12-16 16:27:39 EST
New Package CVS Request
Package Name: openconnect
Short Description: Open client for Cisco AnyConnect VPN
Owners: dwmw2
Branches: F-9 F-10 devel
Comment 27 Peter Robinson 2008-12-16 16:29:38 EST
tarball was there. firefox was caching the dir listing.

md5sum confirmed to match.
Comment 28 Kevin Fenzi 2008-12-17 17:06:09 EST
cvs done.
Comment 29 Nikos Mavrogiannopoulos 2014-07-31 11:50:52 EDT
Package Change Request
Package Name: openconnect
New Branches: epel7
Owners: nmav
Comment 30 Gwyn Ciesla 2014-07-31 12:13:52 EDT
Comments from the primary maintainer?
Comment 31 David Woodhouse 2014-07-31 13:11:33 EDT
(In reply to Jon Ciesla from comment #30)
> Comments from the primary maintainer?

Oh $DEITY yes please. Want to own RHEL[56] too?
Comment 32 Nikos Mavrogiannopoulos 2014-07-31 15:53:22 EDT
(In reply to David Woodhouse from comment #31)
> (In reply to Jon Ciesla from comment #30)
> > Comments from the primary maintainer?
> Oh $DEITY yes please. Want to own RHEL[56] too?

Yes, no problem with that.
Comment 33 David Woodhouse 2014-08-01 04:49:06 EDT
All yours; thanks. RHEL6 wants updating to OpenConnect 6.00 — I was going to do that but it's a chain-build with an updated NetworkManager-openconnect and I haven't yet found the time to do it and test it properly. Not sure I even have a RHEL6 VM with the required client certs still... (yeah, I should set up ocserv :)

Not sure how we handle kde-plasma-nm in EPEL; is it possible to build just the -openconnect subpackage somehow? Or is kde-plasma-nm already in EPEL instead of RHEL itself?

Ideally, OpenConnect should have just been in RHEL7 like vpnc is. It's not as if it's an esoteric requirement.
Comment 34 Rex Dieter 2014-08-01 07:42:58 EDT
David, I'll try to help out with testing the kde bits (on centos7) next week.
Comment 35 Nikos Mavrogiannopoulos 2014-08-04 03:48:06 EDT
John you have removed the fedora-cvs flag but you didn't create the branch. Was that intentional?
Comment 36 Gwyn Ciesla 2014-08-04 08:12:25 EDT
Yes, see comment #30.  Now, reset the fedora-cvs flag and I can proceed.
Comment 37 Gwyn Ciesla 2014-08-04 09:55:56 EDT
Git done (by process-git-requests).

Note You need to log in before you can comment on or make changes to this bug.