webgui: ocsp staple support

[kulikov-a]

Hi!
ref. https://forum.opnsense.org/index.php?topic=26812.msg130038#msg130038 )
had some time and decided to try:
update script is based on lighty cert-staple.sh example
marked as draft for discussion:
although the idea of using the OCSP stapling is absolutely technically correct and it has been tested with a LE certificate and seems to work:

TLS server extension "status request" (id=5), len=507
..
OCSP response:
======================================
OCSP Response Data:
    OCSP Response Status: successful (0x0)
    Response Type: Basic OCSP Response
    Version: 1 (0x0)
    Responder Id: C = US, O = Let's Encrypt, CN = R3
    Produced At: Feb  9 03:31:00 2022 GMT
    Responses:
    Certificate ID:
      Hash Algorithm: sha1
      Issuer Name Hash: 48DAC9A0FB2BD32D4FF0DE68D2F567B735F9B3C4
      Issuer Key Hash: 142EB317B75856CBAE500940E61FAF9D8B14C2C6
      Serial Number: 04FC5B51C0D2D74BD437B057D039C68EAD66
    Cert Status: good
    This Update: Feb  9 03:00:00 2022 GMT
    Next Update: Feb 16 02:59:58 2022 GMT

    Signature Algorithm: sha256WithRSAEncryption
         50:29:c5:d5:cc:32:cf:0a:16:dc:6c:fd:01:64:62:c9:3b:50:
         86:b1:ac:84:4c:d7:bd:88:b0:6c:d9:a6:75:ce:f8:a7:c3:6f:
         45:30:b1:ed:7b:c2:a4:6a:5a:e4:08:c6:a0:9e:7b:59:83:61:
         33:38:b1:67:be:16:44:b1:52:8b:c5:d2:a9:1f:4b:a1:f5:4e:
         50:1d:65:5e:45:25:23:22:2c:2f:84:26:94:fd:37:87:63:1f:
         c5:61:8e:fa:22:75:e1:78:90:ef:a3:71:fe:71:85:1b:7b:54:
         bb:e5:3b:3a:28:2f:66:4c:40:60:49:73:f0:94:31:9b:e6:f7:
         ed:57:95:2b:2e:89:74:83:5e:74:58:85:cd:0f:33:6c:96:84:
         33:14:63:75:1e:70:7e:82:dd:d7:18:14:25:42:84:83:06:7e:
         69:2e:46:6c:c6:ea:34:bb:bc:7d:35:85:4c:11:34:08:42:44:
         a6:d6:da:9b:5a:e8:18:b3:43:2d:05:3e:f2:bf:ae:94:7b:a4:
         43:3f:a1:c8:77:f3:db:97:e7:c7:91:b9:ad:73:a6:73:16:90:
         1b:cb:4f:db:28:44:37:ce:85:e6:1b:26:0a:2f:4f:e3:59:42:
         46:be:c2:45:74:b2:71:55:f2:ae:db:fb:0a:63:05:5a:ea:af:
         de:31:0a:66
======================================

it seems to me that there will be quite a lot of questions due to possible misuse (use of "must staple" certs) ..

Thanks!

[gstrauss]

Is there a better alternative to copying the lighttpd cert-staple.sh? Perhaps the lighttpd package should be extended to install the script for general use, so that the script stays maintained as part of the lighttpd package.

Before enabling OCSP Must-Staple, there should be a scheduled job to periodically (once a day) run cert-staple.sh to refresh the staple.der. The following warning message in the PR is ok for initial testing, but I do not think it sufficient for commit.

<?=gettext("Retrieve OCSP data everytime webGUI (re)start and staple it along with the certificate as part of the TLS handshake. Automatic update is only working during start/restart, you need to setup a cron job to periodically update this data too.");?>

[kulikov-a]

@gstrauss
Hi!

Perhaps the lighttpd package should be extended to install the script for general use, so that the script stays maintained as part of the lighttpd package.

Is that possible?

there should be a scheduled job

Is it better with the latest commit? ;)

[gstrauss]

Thanks @kulikov-a

Is it better with the latest commit? ;)

Looks better! However, I have only visually reviewed the PR. I defer to @fichtner to review if this PR fits opnsense patterns.

Perhaps the lighttpd package should be extended to install the script for general use, so that the script stays maintained as part of the lighttpd package.

Is that possible?

Should be. That was my intention when I wrote cert-staple.sh in upstream lighttpd.

[kulikov-a]

@gstrauss Hi )
I made the logic reverse: autocron is enabled by default

Should be.

Obviously, the point here is my lack of knowledge: I don’t fully understand what you mean. some make options to include cert-staple.sh ?

and if you allow a couple of questions on the cert-staple.sh (especially if it will be maintained as a lighty part): it not works on FreeBSD (date -d is illegal option ) and it would be great if it was a little more verbose imho. can you look at it please (i made some changes in the last commit).
Thanks!

[gstrauss]

lighttpd development occurs on git.lighttpd.net, not here. You're welcome to submit a PR to https://github.com/lighttpd/lighttpd1.4/

can you look at it please

I prefer using source control to compare changes. I am not going to pull your cut-n-paste, and then modified cert-staple.sh from this PR.

Separately, in lighttpd development -- and not in this PR -- I will take a look at date argument portability for *BSD.

[gstrauss]

@kulikov-a I appreciate the effort that you have put into this PR.

That said, since we are dealing with certificates and security, my code review comments will be strict.

• It is poor practice to cut-n-paste code from another package.
  cert-staple.sh comes from lighttpd. Ideally, opnsense lighttpd package should include cert-staple.sh in its installed files.
• It is poor practice to cut-n-paste code from another package and not have the code in a separate commit, with well-documented origin.
• It is poor practice to cut-n-paste code from another package and then to modify that code, and not have those modifications in a separate commit.

You appear to have deleted the code from the script which calculates ocsp_expire.

A future version of lighttpd will address the different FreeBSD date command arguments with special-casing

--- a/doc/scripts/cert-staple.sh
+++ b/doc/scripts/cert-staple.sh
@@ -46,7 +46,14 @@ ocsp_status="$(printf %s "$OCSP_RESP" | head -1)"
 next_update="$(printf %s "$OCSP_RESP" | grep 'Next Update:')"
 next_date="$(printf %s "$next_update" | sed 's/.*Next Update: //')"
 [ -n "$next_date" ] || errexit
-ocsp_expire=$(date -d "$next_date" +%s)
+sysname=$(uname -s)
+if [ "$sysname" = "FreeBSD" ] || \
+   [ "$sysname" = "OpenBSD" ] || \
+   [ "$sysname" = "DragonFly" ]; then
+    ocsp_expire=$(date -j -f "%b %e %T %Y %Z" "$next_date" "+%s")
+else
+    ocsp_expire=$(date -d "$next_date" +%s)
+fi
 
 # validate OCSP response
 ocsp_verify=$(openssl ocsp -issuer "$CHAIN_PEM" -verify_other "$CHAIN_PEM" -cert "$CERT_PEM" -respin "$OCSP_TMP" -no_nonce -out /dev/null 2>&1)

[kulikov-a]

@gstrauss Thanks!
actually I wanted to make a pr at the lighty repo to discuss, but I just didn’t have time )

opnsense lighttpd package should include cert-staple.sh in its installed files

I would really like to know how to do this. and to know @fichtner opinion on this: what if it becomes necessary to make changes specific to the opnsense (verbose output or some..)? it may be more correct in this case (the impossibility of using the source file) to change the name of the script to eliminate confusion (and indicate the origin of the file in the comments)?

the rest of the comments are also correct: the request was made for discussion (and in no case did I want to offend by using the file without your consent, if that's the case)

You appear to have deleted the code from the script which calculates ocsp_expire

yes and wanted to discuss it in pr: as far as I noticed, this variable is then used only for naming the file (besides i'm not sure if nextUpdate can be required - I thought that the absence of this field is allowed). therefore, I thought that I could save the code and not adapt the calculation for different OSes, but simply switch to the current time. or am I missing something?

@ThePowerofDreamS
sorry, what?

[gstrauss]

(and in no case did I want to offend by using the file without your consent, if that's the case)

The code review criticism is not about offense. It is about ongoing ownership and maintenance.

I would really like to know how to do this. and to know @fichtner opinion on this: what if it becomes necessary to make changes specific to the opnsense (verbose output or some..)? it may be more correct in this case (the impossibility of using the source file) to change the name of the script to eliminate confusion (and indicate the origin of the file in the comments)?

Package maintenance is an important concept. If lighttpd maintains and distributes cert-staple.sh (I do), then if opnsense needed a modification for the opnsense lighttpd package, then opnsense would have a small patch together with the opnsense lighttpd package. This would use the maintained upstream lighttpd cert-staple.sh while reducing the maintenance burden for the opnsense lighttpd package maintainer (not me) to making sure the small patch made sense.

Where feasible, proposing patches upstream is a good way to improve open source for everyone and to reduce the maintenance burden of extra patches for specific distributions such as opnsense.

Please do not continue repeating your suggestion to fork the script. Look for better options.

[gstrauss]

You appear to have deleted the code from the script which calculates ocsp_expire

yes and wanted to discuss it in pr: as far as I noticed, this variable is then used only for naming the file (besides i'm not sure if nextUpdate can be required - I thought that the absence of this field is allowed). therefore, I thought that I could save the code and not adapt the calculation for different OSes, but simply switch to the current time. or am I missing something?

You missed carefully documenting your intent in the code, as well as your changes and why.

In upstream lighttpd, I have a not-yet-released enhancement to cert-staple.sh which short-circuits and does not attempt to retrieve a new staple.der if the current staple.der is still valid for at least 25 hours, quickly checking the generated filename rather than re-parsing the der. This will speed up daily cron jobs and put less burden on upstream CAs which issue staples for 1 week.

tl;dr: if you do not intend to take over development and ongoing maintenance, you probably should not be copying or modifying that code.

[fichtner]

I'd say for a draft this is a good discussion, but eventually the only sane way is to move cert-staple.sh out of the docs directory in order to be visible to packaging systems. I'm sure maintainers will notice the change and pick it up.

I'd also suggest a copyright header for such scripts. It adds an aura of professionalism around it so it's easier to discuss on the required terms given here.

Cheers,
Franco

[kulikov-a]

@gstrauss i think i got your point and iiuc you are not interested in arguments why lighty-side script maintanance can be inconvenient for opnsense-specific use.

tl;dr:

would you be ok if i create a script from scratch and stop using the lighty script as an example\template?

[gstrauss]

would you be ok if

Not my call for opnsense. I am sharing my (hopefully qualified) opinions as a lighttpd developer.

@gstrauss i think i got your point and iiuc you are not interested in arguments why lighty-side script maintanance can be inconvenient for opnsense-specific use.

I think you may have missed my point: there should be no opnsense-specific use if it can be avoided. If there is something incompatible with opnsense and you report the issue upstream, then the issue will likely be addressed upstream.

I happened to be part of the thread here where you noted that cert-staple.sh from upstream lighttpd uses date with arguments not supported by FreeBSD. I have already written patches on my dev branch on upstream lighttpd, which are for the next release of lighttpd (currently scheduled Jan 2023)
https://git.lighttpd.net/lighttpd/lighttpd1.4/commit/04d9b13862e6a0b486b2f58aa6225ad8a7670fbf
https://git.lighttpd.net/lighttpd/lighttpd1.4/commit/ba9848adc8bd8d037b9c6690a5e78b3bae4c4916
(Those SHA's might change on my dev branch, but the commits updating cert-staple.sh will be included in the next lighttpd release)

would you be ok if i create a script from scratch and stop using the lighty script as an example\template?

Same as above, I think you missed the point about ownership and maintenance by domain experts.

kulikov-a I would recommend learning more about package maintenance and looking into submitting a PR to add one or both of the above patches to the cert-staple.sh script in lighttpd in the opnsense lighttpd package, and modifying the opnsense lighttpd package to include cert-staple.sh.

@fichtner wrote:

I'd say for a draft this is a good discussion, but eventually the only sane way is to move cert-staple.sh out of the docs directory in order to be visible to packaging systems. I'm sure maintainers will notice the change and pick it up.

I'd also suggest a copyright header for such scripts. It adds an aura of professionalism around it so it's easier to discuss on the required terms given here.

Adding a copyright makes sense. I'll add that upstream.

Regarding moving the script out of docs/scripts/, I disagree that "maintainers will notice" as a general statement. While you do a great job with opnsense, that is not the case with many other distros. It is frequently difficult to get a volunteer to find the time to merely update the source code binary hash and rebuild the package for the distro.

Currently, the lighttpd source code doc/ directory contains man pages, example scripts, systemd config, and others that package maintainers can optionally choose to install in a variety of locations. For example, some distros take doc/scripts/create-mime.conf.pl from the lighttpd source and package it to be installed into a location where it can be called from lighttpd.conf to generate mime assignments from /etc/mime.types. The same was intended for cert-staple.sh.

Since the lighttpd binary and shared objects are extracted and packaged from the build, why does the location of cert-staple.sh under doc/scripts/ in the source tree matter? (Please help me to understand the restriction.) cert-staple.sh can be extracted from that location and installed where desired. (I have not looked specifically at opnsense package management, so I might be misunderstanding some limitations.) Why must the script not be under doc/scripts/ to be visible to the opnsense packaging system? Are there other locations it should not be under, such as contrib/...?

[kulikov-a]

@gstrauss Thanks!

would recommend learning more about package maintenance and looking into submitting a PR

fair enough. long wanted..
but could you clarify one more moment please: i understand that "fixes" like the date work or the possible nextUpdate absence are worth creating a pr at the lighty repo, but what about the "features"? I expect problems when users start shooting themselves in the foot by attaching must-staple certs to the GUI and blocking access to the interface if something goes wrong...and imho it would be useful if the script could accept params like: openssl path, syslog and stderr usage etc. will you consider such requests or do you think it should go into a patch at opnsense lighttpd package?

[gstrauss]

@kulikov-a I would consider discussing feature requests. And I will always seriously consider requested changes to 'usage' and 'help'.

However, and please try not to take this the wrong way: thus far, I have had to repeat myself to educate you on basic best practices on security software ownership and maintenance by domain experts.

Absent evidence, I do not take very seriously any of your thoughts or expectations of what others need or want.

One example of your blind spots:

imho it would be useful if the script could accept params like: openssl path,

The can already be controlled with PATH. If something special is needed just for this script, then PATH can be set just for this script before executing this script. Were a different approach needed, another idiom would be to use an environment variable to obtain the path to openssl, and to allow the user to set the environment variable to override the default. However, you have not demonstrated the need, nor have you acknowledged that controlling the path to openssl is already possible with the existing script. There is a limit to how much time I choose to spend on the remedial training of contributors such as yourself.

[kulikov-a]

@gstrauss
thanks for the feedback
there are no more questions from me

changed the code, lighty example/template script is no longer used

[gstrauss]

The code contains the comment recommended to run this scripty every 5 min to keep stapled response fresh.
Is that recommendation from someone who runs servers professionally? I don't think so.

Scripts refreshing OCSP stapling responses are generally recommended to be run once per day.

e.g. once per day is more than sufficient for Let's Encrypt; OCSP stapling responses from Let's Encrypt are good for one week.