Bug 465362

Summary: Review Request: openconnect -- client for Cisco AnyConnect VPN
Product: [Fedora] Fedora Reporter: David Woodhouse <dwmw2>
Component: Package ReviewAssignee: Peter Robinson <pbrobinson>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: dwalsh, fedora-package-review, gwync, nmavrogi, notting, pbrobinson, pjones, rdieter, shattered
Target Milestone: ---Flags: pbrobinson: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-12-18 17:44:18 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 David Woodhouse 2008-10-02 21:31:40 UTC
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 23:57:54 UTC
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 15:04:33 UTC
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 11:55:04 UTC
If you chcon -t vpnc_exec_t openconnect, does the connection work without any avc messages?

Comment 5 David Woodhouse 2008-10-08 12:22:53 UTC
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 22:33:13 UTC
What is the path to openconnect?

Comment 7 Peter Jones 2008-10-09 15:04:40 UTC
> %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 15:10:31 UTC
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 := $(RPM_OPT_FLAGS)
    else
    OPT_FLAGS := -O2 -g -Wall
    endif

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 15:11:42 UTC
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 15:16:13 UTC
COPYING.LGPL should be listed in %files as a %doc .

Comment 11 David Woodhouse 2008-10-09 15:24:09 UTC
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 15:34:28 UTC
Wait, why %makeinstall?  We have an explicit prohibition against it unless the usual "make install DESTDIR=" doesn't work.
https://fedoraproject.org/wiki/Packaging/Guidelines#Why_the_.25makeinstall_macro_should_not_be_used

Comment 13 David Woodhouse 2008-10-09 15:37:41 UTC
Er, OK. I'll switch back.

Comment 14 Dominik 'Rathann' Mierzejewski 2008-10-09 18:53:23 UTC
(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 22:21:11 UTC
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 shattered 2008-11-02 13:47:19 UTC
The SRPM is not fetchable at the moment (openconnect-f9: Failed to change directory).

Comment 18 David Woodhouse 2008-11-25 00:42:11 UTC
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 19:56:02 UTC
This currently doesn't build for rawhide in koji
http://koji.fedoraproject.org/koji/taskinfo?taskID=1001992

Comment 20 David Woodhouse 2008-12-16 20:06:44 UTC
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 20:10:27 UTC
That worked.
http://koji.fedoraproject.org/koji/taskinfo?taskID=1002066

Comment 22 Peter Robinson 2008-12-16 20:12:42 UTC
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 21:04:23 UTC
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

Optional:

n/a if there is no license file, packager should query upstream
n/a translations of description and summary for non-English languages, if
available
+ 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
/usr/sbin

Comment 24 Peter Robinson 2008-12-16 21:17:47 UTC
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 21:25:18 UTC
Thanks; I'll include README.SecurID.

The tarball should be available:
 ftp://ftp.infradead.org/pub/openconnect/openconnect-0.99.tar.gz

Comment 26 David Woodhouse 2008-12-16 21:27:39 UTC
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 21:29:38 UTC
tarball was there. firefox was caching the dir listing.

md5sum confirmed to match.

Comment 28 Kevin Fenzi 2008-12-17 22:06:09 UTC
cvs done.

Comment 29 Nikos Mavrogiannopoulos 2014-07-31 15:50:52 UTC
Package Change Request
======================
Package Name: openconnect
New Branches: epel7
Owners: nmav

Comment 30 Gwyn Ciesla 2014-07-31 16:13:52 UTC
Comments from the primary maintainer?

Comment 31 David Woodhouse 2014-07-31 17:11:33 UTC
(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 19:53:22 UTC
(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 08:49:06 UTC
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 11:42:58 UTC
David, I'll try to help out with testing the kde bits (on centos7) next week.

Comment 35 Nikos Mavrogiannopoulos 2014-08-04 07:48:06 UTC
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 12:12:25 UTC
Yes, see comment #30.  Now, reset the fedora-cvs flag and I can proceed.

Comment 37 Gwyn Ciesla 2014-08-04 13:55:56 UTC
Git done (by process-git-requests).