Bug 191237 - [patch] security/ssl-admin allow staging and do not strip the key-direction keyword
Summary: [patch] security/ssl-admin allow staging and do not strip the key-direction k...
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: Olli Hauer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-21 09:20 UTC by Olli Hauer
Modified: 2014-06-25 05:23 UTC (History)
2 users (show)

See Also:


Attachments
ssl-admin add stage support (4.54 KB, patch)
2014-06-21 09:20 UTC, Olli Hauer
no flags Details | Diff
Ollis patches, merged and diff (1.43 KB, patch)
2014-06-24 16:41 UTC, Eric F Crist
no flags Details | Diff
Port correction with staging and an update to ssl-admin (1.90 KB, patch)
2014-06-24 19:14 UTC, Eric F Crist
no flags Details | Diff
Updates to 1.2.1 and provides staging, etc (1.98 KB, patch)
2014-06-25 03:35 UTC, Eric F Crist
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Olli Hauer freebsd_committer freebsd_triage 2014-06-21 09:20:23 UTC
Created attachment 143987 [details]
ssl-admin add stage support

small patch against current port

Changes:
 - add stage support
 - replace .default with .sample so the pkg-plist @sample macro can be used
 - preserve key-direction keyword in client.ovpn
   (allow in the current release to add the ta.key already inline in client.ovpn)

http://people.freebsd.org/~ohauer/diffs/ssl-admin-1.1.0_1.diff
Comment 1 Eric F Crist 2014-06-23 12:24:37 UTC
I am looking in to this today.
Comment 2 Eric F Crist 2014-06-24 16:41:15 UTC
Created attachment 144104 [details]
Ollis patches, merged and diff

The attached 1.2.0.patch as most of Ollie's changes and offers a new update to ssl-admin.
Comment 3 Eric F Crist 2014-06-24 17:04:54 UTC
Let's ignore that last patch I sent, and pretend it didn't happen.  I'll send a proper one (that works) shortly.
Comment 4 Olli Hauer freebsd_committer freebsd_triage 2014-06-24 17:22:13 UTC
Don't hurry,

it seems the ftp /pub/ssl-admin/ssl-admin-1.2.0.tar.xz distfile also has some different  ${VARSED} expressions then the .gz and freebsd/ssl-admin-1.20.(g|x)z files.

Also it seems the key-direction keyword will be stripped from the generated client.ovpn config.

If you look at my first patch you find files will be installed into ${DESTDIR}$targetdir. this will work regardless the OS since DESTDIR will be set by the Framework on FreeBSD and Linux during package building to get a clean install.

See http://www.gnu.org/software/automake/manual/html_node/DESTDIR.html

-- 
olli
Comment 5 Eric F Crist 2014-06-24 18:06:13 UTC
(In reply to Olli Hauer from comment #4)
> Don't hurry,
> 
> it seems the ftp /pub/ssl-admin/ssl-admin-1.2.0.tar.xz distfile also has
> some different  ${VARSED} expressions then the .gz and
> freebsd/ssl-admin-1.20.(g|x)z files.
> 
> Also it seems the key-direction keyword will be stripped from the generated
> client.ovpn config.
> 
> If you look at my first patch you find files will be installed into
> ${DESTDIR}$targetdir. this will work regardless the OS since DESTDIR will be
> set by the Framework on FreeBSD and Linux during package building to get a
> clean install.
> 
> See http://www.gnu.org/software/automake/manual/html_node/DESTDIR.html

I've patched ssl-admin so it will not strip key-direction (see https://www.secure-computing.net/trac/changeset/348/SCN%20Open%20Source for the change set).  I'm still trying to wrap my head around your changes to support staging, but think I just need to do a better job with configure, so the Makefile gets built properly to begin with.  I'm trying to avoid any patching at all by the ports tree, and to get the install process to do the right thing.
Comment 6 Olli Hauer freebsd_committer freebsd_triage 2014-06-24 18:34:16 UTC
Hm, the old 1.1.0 configure scrip is fine, I see no need to replace sed with VARSED.

The only addition was to add a possible DESTDIR in front of every targetdir.
In case DESTDIR is not set the Makefile will install all files the same way as before. DESTDIR will be set by the build environment if supported (on FreeBSD, OpenBSD, Linux rpm build ...) so you don't have to worry about this change.
Comment 7 Eric F Crist 2014-06-24 19:14:57 UTC
Created attachment 144105 [details]
Port correction with staging and an update to ssl-admin

Should resolve staging issues, now uses .xz, as well as a port update.
Comment 8 Olli Hauer freebsd_committer freebsd_triage 2014-06-24 20:30:07 UTC
Hint:

I found one small issue in the Makefile.

There is an additional not required '/' between ${DESTDIR} and the target.

A small fix for the port is required anyway, I will explain.

In the clean environment ssl-admin/openssl.conf is not already present so the Makefile will install this file and it will be perhaps included in the package.
In case someone installs the package an already present openssl.conf will be overwritten (not funny).

Therefor we have the @sample macro in pkg-plist, it will install openssl.conf.sample and if there is no openssl.conf then the sample file will be installed without '.sample'.
During package deinstallation `cmp' will be used and if the files are not identical only the '.sample' file will be removed.

The next is to install the file with -o root -g wheel (on Linux -o root -g root).

This way the port can be build without root privilegs but root is required to install the resulting package (@owner root, @group wheel at the top of the work/.PLIST.sub)

If it is OK for you I will commit the following resulting port.
http://people.freebsd.org/~ohauer/diffs/ssl-admin-1.2.0_rev3.tar.xz
Comment 9 Eric F Crist 2014-06-25 02:24:09 UTC
The port Makefile post-patch, I'm OK with.  I'm not OK with the patch-Makefile in files, however.  I'm aware that, on FreeBSD, the forward-slash is redundant, but semantically, that's OK.  I don't know what other distributions will do with regard to their own $DEST_DIR, so I feel it's safest to leave it as I have it.  

I will change how the installer handles the permission in the core project.  You're correct that this should be usable by non-root users, and I'll leave hardening the installation to the local admin.

You mention the @sample macro, but I don't see anything done with it.  I'd rather build the logic into the core Makefile and not depend upon FreeBSD-specific macros.

As with the @sample macro, I see nothing in your patch having to do with install as root/wheel.  What is going to generate the work/PLIST.sub?


I'll attach another patch shortly with my changes listed above, but I'm still not sure how you're wanting to handle the other things.
Comment 10 Eric F Crist 2014-06-25 03:35:13 UTC
Created attachment 144112 [details]
Updates to 1.2.1 and provides staging, etc

This should resolve outstanding issues with the FreeBSD build including staging.
Comment 11 commit-hook freebsd_committer freebsd_triage 2014-06-25 05:04:56 UTC
A commit references this bug:

Author: ohauer
Date: Wed Jun 25 05:04:08 UTC 2014
New revision: 359183
URL: http://svnweb.freebsd.org/changeset/ports/359183

Log:
  - update to 1.2.1
  - add stage support

  Changes:
   store ta.key inline in client.ovpn

  PR:		191237
  Submitted by:	ohauer
  Approved by:	ecrist@secure-computing.net (maintainer)

Changes:
  head/security/ssl-admin/Makefile
  head/security/ssl-admin/distinfo
  head/security/ssl-admin/pkg-plist
Comment 12 Olli Hauer freebsd_committer freebsd_triage 2014-06-25 05:23:24 UTC
committed