Hi,
On Tue, 2018-02-06 at 21:09 +0100, Erik Kapfer wrote:
> Update script for OpenVPNs CRL cause OpenVPN refactors the CRL handling since
> v.2.4.0 .
> Script checks the next update field from the CRL and executes an update
> before it expires.
> Script is placed under fcron.daily for daily checks.
>
> Signed-off-by: Erik Kapfer <erik.kapfer(a)ipfire.org>
> ---
> config/ovpn/openvpn-crl-updater | 88
> +++++++++++++++++++++++++++++++++++++++++
> config/rootfiles/common/openvpn | 1 +
> lfs/openvpn | 6 +++
> 3 files changed, 95 insertions(+)
> create mode 100644 config/ovpn/openvpn-crl-updater
>
> diff --git a/config/ovpn/openvpn-crl-updater b/config/ovpn/openvpn-crl-updater
> new file mode 100644
> index 0000000..9063b04
> --- /dev/null
> +++ b/config/ovpn/openvpn-crl-updater
> @@ -0,0 +1,88 @@
> +#!/bin/bash
> +
> +#############################################################################
> ############
There is an extra empty line before the header and an extra hash in the first
line of the header.
> +#
> #
> +# This file is part of the IPFire Firewall.
> #
> +#
> #
> +# IPFire is free software: you can redistribute it and/or modify
> #
> +# it under the terms of the GNU General Public License as published by
> #
> +# the Free Software Foundation, either version 3 of the License, or
> #
> +# (at your option) any later version.
> #
> +#
> #
> +# IPFire is distributed in the hope that it will be useful,
> #
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> #
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> #
> +# GNU General Public License for more details.
> #
> +#
> #
> +# You should have received a copy of the GNU General Public License
> #
> +# along with IPFire. If not, see <http://www.gnu.org/licenses/>.
> #
> +#
> #
> +# Copyright (C) 2007 IPFire-Team <info(a)ipfire.org>.
> #
> +#
> #
> +#############################################################################
> ############
> +#
> #
> +# Script Name: openvpn-crl-updater
> #
> +# Description: This script checks the "Next Update:" field of the CRL
> #
> +# and renews it if needed, which prevents the expiration of OpenVPNs CRL.
> #
> +# With OpenVPN 2.4.x the CRL handling has been refactored,
> #
> +# whereby the verification logic has been removed from
> ssl_verify_<backend>.c . #
> +# For more infos:
> #
> +# https://github.com/OpenVPN/openvpn/commit/160504a2955c4478cd2c0323452929e
> 07016a336 #
> +#
> #
> +# Run Information: If OpenVPNs CRL is presant,
*present*
> #
> +# this script provides a cronjob which checks daily if an update of the
> CRL #
> +# is needed. If the expiring date reaches the value
> #
> +# (defined in the 'UPDATE' variable in days) before the CRL expiration, an
> openssl #
> +# command will be executed to renew the CRL.
> #
> +# Script execution will be logged into /var/log/messages.
> #
> +#
> #
> +# Author: Erik Kapfer
> #
> +#
> #
> +# Date: 06.02.2018
Dates are not required. Git does this for us.
> #
> +#
> #
> +#############################################################################
> ############
> +
> +# Check if OpenVPN is active or if the CRL is presant
> +if [ ! -e "/var/ipfire/ovpn/crls/cacrl.pem" ]; then
> + exit 0;
> +fi
You got a hardcoded path here. Variables are set after this. It probably makes
sense to move the check after the initialisation block and then check things
and/or exit.
> +## Paths
> +OVPN="/var/ipfire/ovpn"
> +CRL="${OVPN}/crls/cacrl.pem"
> +CAKEY="${OVPN}/ca/cakey.pem"
> +CACERT="${OVPN}/ca/cacert.pem"
> +OPENSSLCONF="${OVPN}/openssl/ovpn.cnf"
> +
> +## Values
> +# CRL check for the 'Next Update:' in seconds
> +EXPIRINGDATEINSEC="$((
> +$(/bin/date -d "$(/usr/bin/openssl crl -in "${CRL}" -text | \
> + /bin/grep -oP 'Next Update: *\K.*')" +%s) - \
> + $(/bin/date +%s) \
> +))"
You never need to use "/bin" or so before a command. The shell will find it.
Just use date, grep, and (further down) openssl.
And I didn't mean just breaking the lines. I meant splitting this into smaller
chunks that are easy to understand and modify if we need to. Like:
NOW="$(date "+%s")"
EXPIRES_AT="$(openssl ... | grep ...)"
# Convert into seconds from epoch
EXPIRES_AT="$(date "${EXPIRES_AT}" "+%s")"
EXPIRINGDATEINSEC=$(( EXPIRES_AT - NOW ))
I find this way easier to read and audit and it will execute in the same amount
of time.
> +# Day in seconds to calculate
> +DAYINSEC="86400"
> +
> +# Convert seconds to days
> +NEXTUPDATE="$((EXPIRINGDATEINSEC / DAYINSEC))"
Here this is super easy to read and understand. Way better.
> +# Update of the CRL in days before CRL expiring date
> +UPDATE="14"
> +
> +
> +# Check if OpenVPNs CRL needs to be renewed
> +if [ ${NEXTUPDATE} -le ${UPDATE} ]; then
> + if /usr/bin/openssl ca -gencrl -keyfile "${CAKEY}" -cert "${CACERT}" -out
> "${CRL}" -config "${OPENSSLCONF}"; then
> + logger -t openvpn "CRL has been updated"
> + else
> + logger -t openvpn "error: Could not update CRL"
> + fi
> +fi
> +
> +exit 0
> +
> +
> +# EOF
> +
> diff --git a/config/rootfiles/common/openvpn b/config/rootfiles/common/openvpn
> index 2b63424..131d798 100644
> --- a/config/rootfiles/common/openvpn
> +++ b/config/rootfiles/common/openvpn
> @@ -1,3 +1,4 @@
> +etc/fcron.daily/openvpn-crl-updater
> #usr/include/openvpn-msg.h
> #usr/include/openvpn-plugin.h
> #usr/lib/openvpn
> diff --git a/lfs/openvpn b/lfs/openvpn
> index 3913f02..1ecc18c 100644
> --- a/lfs/openvpn
> +++ b/lfs/openvpn
> @@ -96,5 +96,11 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
> mv -v /var/ipfire/ovpn/verify /usr/lib/openvpn/verify
> chown root:root /usr/lib/openvpn/verify
> chmod 755 /usr/lib/openvpn/verify
> + # Add crl updater
> + mv -v /var/ipfire/ovpn/openvpn-crl-updater /etc/fcron.daily
> + chown root:root /etc/fcron.daily/openvpn-crl-updater
> + chmod 750 /etc/fcron.daily/openvpn-crl-updater
> +
> @rm -rf $(DIR_APP)
> @$(POSTBUILD)
> +
There is an extra empty line at the end of the LFS file.
Best,
-Michael