Bug 232436 - security/hitch: Add port support for OCSP stapling data
Summary: security/hitch: Add port support for OCSP stapling data
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: Ryan Steinmetz
URL:
Keywords: easy, feature, needs-qa
Depends on:
Blocks:
 
Reported: 2018-10-19 06:01 UTC by Graham Percival
Modified: 2018-12-20 21:09 UTC (History)
3 users (show)

See Also:
bugzilla: maintainer-feedback? (zi)


Attachments
Create a dir for OCSP stapling data (1.10 KB, patch)
2018-10-19 06:01 UTC, Graham Percival
no flags Details | Diff
better hitch-OCSP patch (1.41 KB, patch)
2018-11-03 19:09 UTC, Graham Percival
no flags Details | Diff
hitch OCSP draft 3 (1.39 KB, patch)
2018-11-21 01:42 UTC, Graham Percival
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Graham Percival 2018-10-19 06:01:48 UTC
Created attachment 198350 [details]
Create a dir for OCSP stapling data

The attached patch:
- creates /var/lib/hitch/ for storing OCSP stapling data (with appropriate ownership)
- adds ocsp-dir=/var/lib/hitch to hitch.conf.sample -- but commented out

I'm not certain if you want to enable ocsp for everybody, or leave it commented out, or not use the patch at all.  (I don't know enough about hitch / tls / OCSP to judge whether it would be good to run this "out of the box".)
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2018-10-19 06:20:15 UTC
Thanks for your contribution Graham

/var/db is the canonical (man 7 hier) shared place for data/database files and the location that most (if not all) ports use if they don't other dedicate a special place for themselves elsewhere in LOCALBASE.

Could you update your patch to use that location?
Comment 2 Colin Percival freebsd_committer 2018-10-19 07:24:05 UTC
Two minor comments here:

1. /var/lib/foo/ is common on Linux systems; I think it might be a hard-coded default in hitch.  But if possible we should patch hitch to put its files in the right place.

2. Creating directories outside of ${STAGEDIR} in post-install will break packaging... I'm not sure what the right way of doing this is.  Kubilay, can you advise here?  Does Graham just need to add ${STAGEDIR} in the appropriate places, or is there extra magic needed because /var/db/ isn't under ${PREFIX}?
Comment 3 Kubilay Kocak freebsd_committer freebsd_triage 2018-10-19 09:37:59 UTC
(In reply to Colin Percival from comment #2)

Yep, if hard-coded, the software should either be:

1) configured at build time to use a different path (autoconf ./configure based build systems often use --localstatedir for this), OR
2) patched if not a compiled software (python package, for instance) or based in configuration files.

STAGEDIR prefixing is required for every path, but PREFIX is not for this /var/* case, *but* ...

perhaps not due to policy [1], which states "The port must respect the value of this variable."

But because the vast majority of ports don't use it, for things outside of /usr/local (the default value for PREFIX).

I found only these examples of PREFIX-prefixed paths for /var/db:

emulators/tpm-emulator/Makefile: @${MKDIR} ${STAGEDIR}${PREFIX}/var/db/tpm
x11-fonts/linux-c6-fontconfig/Makefile: ${MKDIR} ${STAGEDIR}${PREFIX}/var/db/fontconfig
x11-fonts/linux-c7-fontconfig/Makefile: ${MKDIR} ${STAGEDIR}${PREFIX}/var/db/fontconfig
www/mod_gnutls/Makefile:DBDIR= ${PREFIX}/var/db/${PORTNAME}
/usr/ports/net/turnserver/Makefile:     ${MKDIR} ${STAGEDIR}${PREFIX}/var/db

These would install into /usr/local/var/db by default. One might consider these to be 'wrong' (I do). Though, technically, so is 'not respecting PREFIX'.

Clarity on this front, even if only the addition of specific exceptions like 'not needed for things outside of /usr/local' in PREFIX documentation would be handy. 

But in the meantime and in summary: Always use STAGEDIR, don't need to use PREFIX for this specific case.

[1] https://www.freebsd.org/doc/en/books/porters-handbook/porting-prefix.html
Comment 4 Graham Percival 2018-10-19 17:39:50 UTC
Thanks for the comments!  The path is not hard-coded, so it'll be easy to change to a different /var directory.

On that note, would /var/cache/hitch be more suitable?  As far as I understand it, OCSP stapling is a form of caching.

(incidentally, I only went with /var/lib because I noticed a /var/lib/dbus on my system, so I didn't look further into linux-vs-freebsd paths)
Comment 5 Graham Percival 2018-11-03 19:09:36 UTC
Created attachment 198920 [details]
better hitch-OCSP patch

Use /var/cache/hitch instead of the previously-proposed /var/lib/hitch
Comment 6 Graham Percival 2018-11-03 19:11:37 UTC
Thanks for the comments!  From offline communication with cperciva, I went with /var/cache/hitch instead of /var/db.  I also added ${STAGEDIR} as appropriate, and then discovered that I should add @dir /var/cache/hitch to pkg-plist.
Comment 7 Colin Percival freebsd_committer 2018-11-20 05:42:59 UTC
Is it intentional that the ocsp-dir setting in hitch.conf.sample is commented out?  It looks like /var/lib/hitch is hard-coded into hitch as a default (https://github.com/varnish/hitch/blob/372f3559c720ab598ae0360d1c1696c7b8189e4b/src/configuration.c#L208) so with your patch the default behaviour will be to have broken OCSP stapling, I think?

One obvious option is to add a patch which changes the hard-coded default.
Comment 8 Graham Percival 2018-11-21 01:42:02 UTC
Created attachment 199398 [details]
hitch OCSP draft 3

Huh, I misread the code.  From this:

    if (cfg->OCSP_DIR != NULL) {

https://github.com/varnish/hitch/blob/372f3559c720ab598ae0360d1c1696c7b8189e4b/src/configuration.c#L1610

I got the impression that OCSP was disabled by default.  But that's not the case, due to the line you pointed out.  (That line was a later change to the code:
https://github.com/varnish/hitch/commit/39f616d8b7aa12e3ac4df29a67d339121827b84f
.  The original code _did_ treat it as an optional argument.  But the change was back in Nov 2016 so I should have noticed it.  Oops.)


My preference at this point is to uncomment out the line in the config file.


On a separate note, I've discovered the
    @dir(username,groupname,) /var/cache/hitch
format for pkg-list, and also the %%replace_var%% format.  I've made use of both in the latest patch.
Comment 9 joshruehlig 2018-12-19 17:56:51 UTC
Just to notify everyone CC'ed here. Hitch version 1.5 with UNIX domain socket (UDS) support was release 2 days ago. Looking forward to the update, and addition of easier to enable OCSP. =]
Comment 10 commit-hook freebsd_committer 2018-12-20 21:08:39 UTC
A commit references this bug:

Author: zi
Date: Thu Dec 20 21:07:48 UTC 2018
New revision: 487922
URL: https://svnweb.freebsd.org/changeset/ports/487922

Log:
  - Update to 1.5.0
  - Add OCSP stapling smarts to port [1]

  PR:		232436 [1]
  Submitted by:	Graham Percival [1]

Changes:
  head/security/hitch/Makefile
  head/security/hitch/distinfo
  head/security/hitch/files/hitch.conf.sample
  head/security/hitch/files/hitch.conf.sample.in
  head/security/hitch/pkg-message
  head/security/hitch/pkg-plist
Comment 11 Ryan Steinmetz freebsd_committer freebsd_triage 2018-12-20 21:09:05 UTC
Thanks!