Avoid race condition when renewing certificates

Only single certificate can be processed at the same time. Make sure to obtain exclusive lock before executing renewal command.
voxpupuli · Aug 7, 2023 · f0d6009 · f0d6009
1 parent 84e2e60
commit f0d6009
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 13 deletions.
diff --git a/files/certbot_lock.sh b/files/certbot_lock.sh
@@ -0,0 +1,14 @@
+# Managed by Puppet
+LOCKFILE="/var/lock/certbot"
+LOCKFD=99
+# private
+function _lock()          { flock "$@" $LOCKFD; }
+function _release_lock()  { _lock -u; _lock -xn && rm -f $LOCKFILE; }
+function _prepare_lock()  { eval "exec $LOCKFD>\"$LOCKFILE\""; trap _release_lock EXIT; }
+
+# on start
+_prepare_lock
+
+# public
+function exlock()         { _lock -x --timeout 30; }   # obtain an exclusive lock
+function unlock()         { _lock -u; }                # drop a lock
diff --git a/manifests/init.pp b/manifests/init.pp
@@ -105,6 +105,12 @@
 
   contain letsencrypt::renew
 
+  file { '/usr/share/certbot_lock.sh':
+    ensure  => file,
+    mode    => '0544',
+    content => file("${module_name}/certbot_lock.sh"),
+  }
+
   $certificates.each |$certificate, $properties| {
     letsencrypt::certonly { $certificate: * => $properties }
   }

diff --git a/spec/defines/letsencrypt_certonly_spec.rb b/spec/defines/letsencrypt_certonly_spec.rb
@@ -2,6 +2,22 @@
 
 require 'spec_helper'
 
+def renew_command(cmd, env = [])
+  environ = ''
+  env.each do |e|
+    environ += "\nexport #{e}"
+  end
+  <<~EOS
+    #!/bin/sh#{environ}
+    if [ -f '/usr/share/certbot_lock.sh' ]; then
+      . '/usr/share/certbot_lock.sh'
+    fi
+    exlock
+    #{cmd}
+    unlock
+  EOS
+end
+
 describe 'letsencrypt::certonly' do
   on_supported_os.each do |os, facts|
     context "on #{os} based operating systems" do
@@ -225,7 +241,9 @@ class { 'letsencrypt::plugin::dns_cloudflare':
 
         it { is_expected.to compile.with_all_deps }
         it { is_expected.to contain_cron('letsencrypt renew cron foo.example.com').with_command('"/var/lib/puppet/letsencrypt/renew-foo.example.com.sh"').with_ensure('present') }
-        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content("#!/bin/sh\nletsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a apache --cert-name 'foo.example.com' -d 'foo.example.com'\n") }
+
+        script = renew_command("letsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a apache --cert-name 'foo.example.com' -d 'foo.example.com'")
+        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content(script) }
       end
 
       context 'with hook' do
@@ -281,7 +299,9 @@ class { 'letsencrypt::plugin::dns_cloudflare':
 
         it { is_expected.to compile.with_all_deps }
         it { is_expected.to contain_cron('letsencrypt renew cron foo.example.com').with_hour(13).with_ensure('present') }
-        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content("#!/bin/sh\nletsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a standalone --cert-name 'foo.example.com' -d 'foo.example.com'\n") }
+
+        script = renew_command("letsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a standalone --cert-name 'foo.example.com' -d 'foo.example.com'")
+        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content(script) }
       end
 
       context 'with manage_cron and out of range defined cron_hour (integer)' do
@@ -308,7 +328,9 @@ class { 'letsencrypt::plugin::dns_cloudflare':
 
         it { is_expected.to compile.with_all_deps }
         it { is_expected.to contain_cron('letsencrypt renew cron foo.example.com').with_hour('00').with_ensure('present') }
-        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content("#!/bin/sh\nletsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a standalone --cert-name 'foo.example.com' -d 'foo.example.com'\n") }
+
+        script = renew_command("letsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a standalone --cert-name 'foo.example.com' -d 'foo.example.com'")
+        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content(script) }
       end
 
       context 'with manage_cron and defined cron_hour (array)' do
@@ -322,7 +344,9 @@ class { 'letsencrypt::plugin::dns_cloudflare':
 
         it { is_expected.to compile.with_all_deps }
         it { is_expected.to contain_cron('letsencrypt renew cron foo.example.com').with_hour([1, 13]).with_ensure('present') }
-        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content("#!/bin/sh\nletsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a standalone --cert-name 'foo.example.com' -d 'foo.example.com'\n") }
+
+        script = renew_command("letsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a standalone --cert-name 'foo.example.com' -d 'foo.example.com'")
+        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content(script) }
       end
 
       context 'with manage_cron and defined cron_minute (integer)' do
@@ -336,7 +360,9 @@ class { 'letsencrypt::plugin::dns_cloudflare':
 
         it { is_expected.to compile.with_all_deps }
         it { is_expected.to contain_cron('letsencrypt renew cron foo.example.com').with_minute(15).with_ensure('present') }
-        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content("#!/bin/sh\nletsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a standalone --cert-name 'foo.example.com' -d 'foo.example.com'\n") }
+
+        script = renew_command("letsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a standalone --cert-name 'foo.example.com' -d 'foo.example.com'")
+        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content(script) }
       end
 
       context 'with manage_cron and defined cron_minute (string)' do
@@ -350,7 +376,9 @@ class { 'letsencrypt::plugin::dns_cloudflare':
 
         it { is_expected.to compile.with_all_deps }
         it { is_expected.to contain_cron('letsencrypt renew cron foo.example.com').with_minute('15').with_ensure('present') }
-        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content("#!/bin/sh\nletsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a standalone --cert-name 'foo.example.com' -d 'foo.example.com'\n") }
+
+        script = renew_command("letsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a standalone --cert-name 'foo.example.com' -d 'foo.example.com'")
+        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content(script) }
       end
 
       context 'with manage_cron and defined cron_minute (array)' do
@@ -364,7 +392,9 @@ class { 'letsencrypt::plugin::dns_cloudflare':
 
         it { is_expected.to compile.with_all_deps }
         it { is_expected.to contain_cron('letsencrypt renew cron foo.example.com').with_minute([0, 30]).with_ensure('present') }
-        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content("#!/bin/sh\nletsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a standalone --cert-name 'foo.example.com' -d 'foo.example.com'\n") }
+
+        script = renew_command("letsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a standalone --cert-name 'foo.example.com' -d 'foo.example.com'")
+        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content(script) }
       end
 
       context 'with manage_cron and ensure absent' do
@@ -394,7 +424,9 @@ class { 'letsencrypt::plugin::dns_cloudflare':
         it { is_expected.to compile.with_all_deps }
         it { is_expected.to contain_file('/tmp/custom_vardir/letsencrypt').with_ensure('directory') }
         it { is_expected.to contain_cron('letsencrypt renew cron foo.example.com').with_command '"/tmp/custom_vardir/letsencrypt/renew-foo.example.com.sh"' }
-        it { is_expected.to contain_file('/tmp/custom_vardir/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content("#!/bin/sh\nletsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a apache --cert-name 'foo.example.com' -d 'foo.example.com'\n") }
+
+        script = renew_command("letsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a apache --cert-name 'foo.example.com' -d 'foo.example.com'")
+        it { is_expected.to contain_file('/tmp/custom_vardir/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content(script) }
       end
 
       context 'with custom plugin and manage cron and cron_success_command' do
@@ -410,7 +442,9 @@ class { 'letsencrypt::plugin::dns_cloudflare':
 
         it { is_expected.to compile.with_all_deps }
         it { is_expected.to contain_cron('letsencrypt renew cron foo.example.com').with_command '"/var/lib/puppet/letsencrypt/renew-foo.example.com.sh"' }
-        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content("#!/bin/sh\n(echo before) && letsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a apache --cert-name 'foo.example.com' -d 'foo.example.com' && (echo success)\n") }
+
+        script = renew_command("(echo before) && letsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a apache --cert-name 'foo.example.com' -d 'foo.example.com' && (echo success)")
+        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content(script) }
       end
 
       context 'without plugin' do
@@ -449,7 +483,9 @@ class { 'letsencrypt::plugin::dns_cloudflare':
         let(:params) { { environment: ['FOO=bar', 'FIZZ=buzz'], manage_cron: true } }
 
         it { is_expected.to compile.with_all_deps }
-        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_content "#!/bin/sh\nexport FOO=bar\nexport FIZZ=buzz\nletsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a standalone --cert-name 'foo.example.com' -d 'foo.example.com'\n" }
+
+        script = renew_command("letsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a standalone --cert-name 'foo.example.com' -d 'foo.example.com'", ['FOO=bar', 'FIZZ=buzz'])
+        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_content(script) }
       end
 
       context 'with manage cron and cron_output=suppress' do\
@@ -461,7 +497,9 @@ class { 'letsencrypt::plugin::dns_cloudflare':
 
         it { is_expected.to compile.with_all_deps }
         it { is_expected.to contain_cron('letsencrypt renew cron foo.example.com').with_command('"/var/lib/puppet/letsencrypt/renew-foo.example.com.sh"').with_ensure('present') }
-        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content("#!/bin/sh\nletsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a standalone --cert-name 'foo.example.com' -d 'foo.example.com' > /dev/null 2>&1\n") }
+
+        script = renew_command("letsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a standalone --cert-name 'foo.example.com' -d 'foo.example.com' > /dev/null 2>&1")
+        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content(script) }
       end
 
       context 'with manage cron and cron_output=log' do\
@@ -473,7 +511,9 @@ class { 'letsencrypt::plugin::dns_cloudflare':
 
         it { is_expected.to compile.with_all_deps }
         it { is_expected.to contain_cron('letsencrypt renew cron foo.example.com').with_command('"/var/lib/puppet/letsencrypt/renew-foo.example.com.sh"').with_ensure('present') }
-        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content("#!/bin/sh\nletsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a standalone --cert-name 'foo.example.com' -d 'foo.example.com' 2>&1 | logger -t letsencrypt-renew\n") }
+
+        script = renew_command("letsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a standalone --cert-name 'foo.example.com' -d 'foo.example.com' 2>&1 | logger -t letsencrypt-renew")
+        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content(script) }
       end
 
       context 'with manage cron and custom day of month' do
@@ -485,7 +525,9 @@ class { 'letsencrypt::plugin::dns_cloudflare':
 
         it { is_expected.to compile.with_all_deps }
         it { is_expected.to contain_cron('letsencrypt renew cron foo.example.com').with(monthday: [1, 15]).with_ensure('present') }
-        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content("#!/bin/sh\nletsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a standalone --cert-name 'foo.example.com' -d 'foo.example.com'\n") }
+
+        script = renew_command("letsencrypt --keep-until-expiring --text --agree-tos --non-interactive certonly --rsa-key-size 4096 -a standalone --cert-name 'foo.example.com' -d 'foo.example.com'")
+        it { is_expected.to contain_file('/var/lib/puppet/letsencrypt/renew-foo.example.com.sh').with_ensure('file').with_content(script) }
       end
 
       context 'with custom config_dir' do

diff --git a/templates/renew-script.sh.erb b/templates/renew-script.sh.erb
@@ -2,4 +2,9 @@
 <%- @environment.each do |environment| -%>
 export <%= environment %>
 <%- end -%>
+if [ -f '/usr/share/certbot_lock.sh' ]; then
+  . '/usr/share/certbot_lock.sh'
+fi
+exlock
 <%= @cron_cmd %>
+unlock