Bug 232436
| Summary: | security/hitch: Add port support for OCSP stapling data | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Ports & Packages | Reporter: | Graham Percival <grahamyvr> | ||||||||
| Component: | Individual Port(s) | Assignee: | Ryan Steinmetz <zi> | ||||||||
| Status: | Closed FIXED | ||||||||||
| Severity: | Affects Some People | CC: | cperciva, grahamyvr, joshruehlig | ||||||||
| Priority: | --- | Keywords: | easy, feature, needs-qa | ||||||||
| Version: | Latest | Flags: | bugzilla:
maintainer-feedback?
(zi) |
||||||||
| Hardware: | Any | ||||||||||
| OS: | Any | ||||||||||
| Attachments: |
|
||||||||||
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? 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}?
(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 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) Created attachment 198920 [details]
better hitch-OCSP patch
Use /var/cache/hitch instead of the previously-proposed /var/lib/hitch
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.
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. 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. 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. =] 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 Thanks! |
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".)