From e3a206b8484ef7919053a38e4e6b92492c8a8642 Mon Sep 17 00:00:00 2001
From: Hendrik Jäger <gitcommit@henk.geekmail.org>
Date: Tue, 13 Feb 2024 22:14:13 +0100
Subject: cleanups, comments, simplification, etc

---
 macir.rb | 210 +++++++++++++++++++++++++++++++++------------------------------
 1 file changed, 111 insertions(+), 99 deletions(-)

diff --git a/macir.rb b/macir.rb
index 986312c..133a1db 100644
--- a/macir.rb
+++ b/macir.rb
@@ -10,6 +10,20 @@ require 'time'
 require 'English'
 
 
+def read_file(path)
+  if File.readable?(path)
+    p "File #{path} is readable, trying to parse"
+    file_content = File.read(path)
+  elsif File.exist?(path)
+    warn "The file #{path} exists but is not readable. Make it readable or specify different path"
+    raise
+  else
+    p "File #{path} does not exist, trying to create"
+    file_content = nil
+  end
+  return file_content
+end
+
 def read_config(path = 'config.yaml')
   p "Reading config from #{path}"
   YAML.load_file(path)
@@ -30,46 +44,48 @@ def ensure_cert_dir(path = './certs')
   File.world_readable?(path) && warn('WARNING! Certificate directory is world readable! This could be a serious security issue!')
   File.file?(path) && raise('Certificate directory is not a directory but a file. Aborting.')
   File.writable?(path) || raise('Certificate directory is not writable. Aborting.')
+  return true
+rescue Errno::ENOENT
+  abort "Could not create directory #{path}. Maybe parent directory does not exist?"
+rescue SystemCallError
+  warn 'Something went wrong when checking the certificate directory. Please report a bug so this problem can be caught more gracefully.'
+  raise
 end
 
 def read_account_key(path = 'pkey.pem')
   p "Reading account key from #{path}"
-  if File.readable?(path)
-    p "File #{path} is readable, trying to parse"
-    privatekey_string = File.read(path)
-    private_key = OpenSSL::PKey::EC.new(privatekey_string)
-  elsif File.exist?(path)
-    raise("The file #{path} exists but is not readable. Make it readable or specify different path")
-  else
-    p "File #{path} does not exist, trying to create"
+  account_key_string = read_file(path)
+  if account_key_string.nil?
     private_key = OpenSSL::PKey::EC.generate('prime256v1')
     File.write(path, private_key.private_to_pem)
+  else
+    private_key = OpenSSL::PKey::EC.new(account_key_string)
   end
   return private_key
 end
 
+# TODO: divide and simplify
 def read_cert_key(cert_name)
   folder = "./certs/#{cert_name}/"
   path = "#{folder}/current.key"
   p "cert_name #{cert_name}: Reading cert key from #{path}"
-  if File.readable?(path)
-    p "cert_name #{cert_name}: File #{path} is readable, trying to parse"
-    privatekey_string = File.read(path)
-    private_key = OpenSSL::PKey::EC.new(privatekey_string)
-  elsif File.exist?(path)
-    raise("cert_name #{cert_name}: The file #{path} exists but is not readable. Make it readable or specify different path")
-  else
+  cert_key_string = read_file(path)
+  if cert_key_string.nil?
+    p "cert_name #{cert_name}: dir #{folder} does not exist, trying to create"
+    FileTest.directory?(folder) || Dir.mkdir(folder, 0o0700)
     p "cert_name #{cert_name}: File #{path} does not exist, trying to create"
     private_key = OpenSSL::PKey::EC.generate('prime256v1')
     pkey_file = File.new("#{folder}#{Time.now.to_i}.key", 'w')
     pkey_file.write(private_key.private_to_pem)
     File.symlink(File.basename(pkey_file), "#{File.dirname(pkey_file)}/current.key")
+  else
+    private_key = OpenSSL::PKey::EC.new(cert_key_string)
   end
   return private_key
 end
 
 def lookup_ns(domain)
-  p "Domain #{domain}: Creating recursor object for checking challenge propagation"
+  p "Domain #{domain}: Creating resolver object for looking up NS records"
   rec = Dnsruby::Resolver.new
   p "Domain #{domain}: Getting NS records for #{domain}"
   rec.query_no_validation_or_recursion(domain, 'NS')
@@ -79,6 +95,7 @@ rescue StandardError => e
 end
 
 def lookup_soa(domain)
+  p "Domain #{domain}: Creating resolver object for looking up SOA records"
   rec = Dnsruby::Resolver.new
   p "Domain #{domain}: Getting SOA records for #{domain}"
   rec.query_no_validation_or_recursion(domain, 'SOA')
@@ -114,7 +131,7 @@ def build_tsig_object(apex, config)
   tsig_alg = config.dig('tsig_keys', tsig_name, 'algorithm')
 
   p "Domain #{apex}: Creating TSIG object"
-  tsig = Dnsruby::RR.create(
+  Dnsruby::RR.create(
     {
       name: tsig_name,
       type: 'TSIG',
@@ -126,13 +143,12 @@ end
 
 def deploy_dns_tokens_on_apex(apex, authzs, nameserver, config)
   update_packet = build_dns_update_packet(apex, authzs)
+  tsig = build_tsig_object(apex, config)
 
   p "Domain #{apex}: Creating object for contacting nameserver"
   res = Dnsruby::Resolver.new(nameserver)
   res.dnssec = false
 
-  tsig = build_tsig_object(apex, config)
-
   p "Domain #{apex}: Signing DNS UPDATE packet with TSIG object"
   tsig.apply(update_packet)
 
@@ -143,30 +159,29 @@ rescue StandardError => e
   raise
 end
 
-def deploy_dns01_challenge_token(domain, challenge, nameserver, config)
-  p "Domain #{domain}: Creating DNS UPDATE packet"
-
-  apex_domain = find_apex_domain(domain)
-
-  update = Dnsruby::Update.new(apex_domain)
-  # TODO: delete challenge token record after validation
-  update.delete("#{challenge.record_name}.#{domain}", challenge.record_type)
-  update.add("#{challenge.record_name}.#{domain}", challenge.record_type, 10, challenge.record_content)
-
-  p "Domain #{domain}: Creating object for contacting nameserver"
-  res = Dnsruby::Resolver.new(nameserver)
+def wait_for_challenge_on_ns(chal, ns, domain)
+  p "Domain #{domain}: Creating resolver object for checking propagation on #{ns}"
+  res = Dnsruby::Resolver.new(ns)
   res.dnssec = false
-
-  tsig = build_tsig_object(domain, config)
-
-  p "Domain #{domain}: Signing DNS UPDATE packet with TSIG object"
-  tsig.apply(update)
-
-  p "Domain #{domain}: Sending UPDATE to nameserver"
-  res.send_message(update)
-rescue StandardError => e
-  warn "Domain #{domain}: DNS Update failed: #{e}"
-  raise
+  res.do_caching = false
+  loop do
+    p "Domain #{domain}: Querying ACME challenge record"
+    result = res.query_no_validation_or_recursion("_acme-challenge.#{domain}", 'TXT')
+    propagated = result.answer.any? do |answer|
+      answer.rdata[0] == chal.record_content
+    end
+    break if propagated
+
+    p "Domain #{domain}: Not yet propagated, still old value, sleeping before checking again"
+    sleep(0.5)
+  rescue Dnsruby::NXDomain
+    p "Domain #{domain}: Not yet propagated, NXdomain, sleeping before checking again"
+    sleep(0.5)
+    retry
+  rescue StandardError => e
+    warn "Domain #{domain}: ACME challenge lookup failed: #{e}"
+    raise
+  end
 end
 
 def wait_for_challenge_propagation(domain, challenge)
@@ -178,30 +193,8 @@ def wait_for_challenge_propagation(domain, challenge)
   threads = []
 
   domain_auth_ns.answer.each do |ns|
-    threads << Thread.new(ns) do |my_ns|
-      nameserver = my_ns.rdata.to_s
-      p "Domain #{domain}: Creating resolver object for checking propagation on #{nameserver}"
-      res = Dnsruby::Resolver.new(nameserver)
-      res.dnssec = false
-      res.do_caching = false
-      loop do
-        p "Domain #{domain}: Querying ACME challenge record"
-        result = res.query_no_validation_or_recursion("_acme-challenge.#{domain}", 'TXT')
-        propagated = result.answer.any? do |answer|
-          answer.rdata[0] == challenge.record_content
-        end
-        break if propagated
-
-        p "Domain #{domain}: Not yet propagated, still old value, sleeping before checking again"
-        sleep(0.5)
-      rescue Dnsruby::NXDomain
-        p "Domain #{domain}: Not yet propagated, NXdomain, sleeping before checking again"
-        sleep(0.5)
-        retry
-      rescue StandardError => e
-        warn "Domain #{domain}: ACME challenge lookup failed: #{e}"
-        raise
-      end
+    threads << Thread.new(challenge, ns, domain) do |challenge, ns, domain|
+      wait_for_challenge_on_ns(challenge, ns.rdata.to_s, domain)
     end
   end
 
@@ -219,11 +212,11 @@ rescue Acme::Client::Error::BadNonce
 end
 
 def unvalidated_domains(d_a)
-  d_a.select do |d, a|
+  d_a.reject do |d, a|
     p "#{d} status #{a['auth'].dns01.status}"
     p a['auth'].dns01.status
     acme_request_with_retries { a['auth'].dns01.reload }
-    a['auth'].dns01.status != 'valid'
+    a['auth'].dns01.status == 'valid'
   end
 end
 
@@ -242,7 +235,6 @@ end
 
 def get_cert(order, cert_name, domains, domain_key)
   path = "./certs/#{cert_name}/"
-  crt_file = "#{path}/cert.pem"
   p "Cert #{cert_name}: Creating CSR object"
   csr = Acme::Client::CertificateRequest.new(
     private_key: domain_key,
@@ -264,6 +256,8 @@ def get_cert(order, cert_name, domains, domain_key)
   # pp order
   cert = acme_request_with_retries { order.certificate }
 
+  p "Cert #{cert_name}: creating dir"
+  FileTest.directory?(path) || Dir.mkdir(path, 0o0700)
   p "Cert #{cert_name}: Writing cert"
   cert_file = File.new("#{path}#{Time.now.to_i}.crt", 'w')
   cert_file.write(cert)
@@ -306,8 +300,8 @@ def make_client_for_ca_account(ca, id)
 
   email = id['email']
 
-  account_object = acme_request_with_retries { client.new_account(contact: "mailto:#{email}", terms_of_service_agreed: true) }
-  client
+  acme_request_with_retries { client.new_account(contact: "mailto:#{email}", terms_of_service_agreed: true) }
+  return client
 end
 
 def handle_apex_authzs(apex, authzs, config)
@@ -320,14 +314,19 @@ end
 config = read_config
 
 cert_dir = config.dig('global', 'cert_dir') || './certs/'
-
 ensure_cert_dir(cert_dir)
 
 
-domains = config['certs'].map { |_certname, cert_opts| cert_opts['domain_names'] }.flatten
+certs = config['certs']
+
+domains = certs.map { |_certname, cert_opts| cert_opts['domain_names'] }.flatten
 domain_attrs = domains.to_h { |d| [d, {}] }
 
 
+# we need the apex of each domain because that’s where we will send the dns update packet
+# TODO: we don’t really need to group this by apex domain or zone but by TSIG key AND zone
+# TODO: we don’t really need the apex but the zone under which that name is on the primary nameserver
+# TODO: we only need this when dns challenge is used
 domain_apex_threads = {}
 domains.map do |d|
   domain_apex_threads[d] = Thread.new(d) do |d|
@@ -337,17 +336,23 @@ domains.map do |d|
 end
 domain_apex_threads.each(&:join)
 
-domain_attrs.keys.each do |domain|
+domain_attrs.each_key do |domain|
   domain_attrs[domain][:apex] = domain_apex_threads[domain].value
 end
 
 apex_domains = domain_attrs.map do |_, v|
   apex = v[:apex]
-  domains_under_apex = domain_attrs.filter { |d,v| v[:apex] == apex }.keys
-  [apex, { :domains => domains_under_apex }]
+  domains_under_apex = domain_attrs.filter { |_d, v| v[:apex] == apex }.keys
+  [apex, { domains: domains_under_apex }]
 end.uniq.to_h
 
 
+# we want all NS entries for the apex domain for checking whether the challenge was deployed
+# TODO: this is only true for domains delegated from a registry
+#   a delegation purely done with NS records will work differently
+#   in that case "walking the domain" label by label and checking for NS entries might be necessary
+#   or maybe just letting it be configurable which NS to check would be better?
+#   for now: keeping it simple
 apex_domain_ns_threads = {}
 apex_domains.keys.map do |d|
   apex_domain_ns_threads[d] = Thread.new(d) do |d|
@@ -360,22 +365,22 @@ apex_domains.keys.map do |d|
 end
 apex_domain_ns_threads.each(&:join)
 
-apex_domains.keys.each do |domain|
+apex_domains.each_key do |domain|
   apex_domains[domain][:ns] = apex_domain_ns_threads[domain].value
-  # p "#{domain}: #{apex_domains[domain][:ns]}"
 end
-# p apex_domains
 
 
-certs = config['certs']
-certs.each_pair do |name, opts|
-  opts['ca_account'] || certs[name]['ca_account'] = config.dig('defaults', 'certs', 'ca_account')
+# every cert should have a ca_account associated, use default if not
+certs.each_pair do |cert, _opts|
+  certs[cert]['ca_account'] ||= config.dig('defaults', 'certs', 'ca_account')
 end
 
 
+# we need to know all ca_accounts in use to create a client for each
 ca_accounts = certs.map { |_, opts| opts['ca_account'] }.uniq
 
 
+# for each ca_account we need to use, we create a client
 account_threads = []
 ca_accounts.each do |ca_account|
   account_threads << Thread.new(ca_account) do |ca_account|
@@ -391,10 +396,13 @@ ca_accounts.each do |ca_account|
 end
 account_threads.each(&:join)
 
-ca_clients = account_threads.map { |t| t.value }
+ca_clients = account_threads.map(&:value)
 ca_clients = ca_clients[0].merge(*ca_clients[1..])
 
 
+# for each cert, we send an order
+# * with the corresponding client, i.e. using the corresponding CA and account
+# * with the corresponding domains
 certs.each_pair do |cert_name, cert_opts|
   client = ca_clients[cert_opts['ca_account']]
   domains = cert_opts['domain_names']
@@ -411,25 +419,38 @@ certs.each_pair do |cert_name, cert_opts|
 end
 
 
-certs.each_pair do |cert, opts|
+# from each cert order, we pull the authorizations needed for the domains to the domains attributes hash
+certs.each_pair do |_cert, opts|
   order_authorizations = acme_request_with_retries { opts['order'].authorizations }
   order_authorizations.each do |auth|
     domain_attrs[auth.domain]['auth'] = auth
   end
 end
 
-all_authorizations = domain_attrs.map do |d, attrs|
+
+# we need a list of all authorizations
+all_authorizations = domain_attrs.map do |_domain, attrs|
   attrs['auth']
 end
 
+
+# we group the authorizations by apex to be able to send them all at once
 authzs_by_apex = all_authorizations.group_by do |auth|
   domain_attrs[auth.domain][:apex]
 end
 
+
+# we use one thread per apex domain for deploying authorizations
+auth_threads = []
 authzs_by_apex.each_pair do |apex, authzs|
-  handle_apex_authzs(apex, authzs, config)
+  auth_threads << Thread.new(apex, authzs) do |apex, authzs|
+    handle_apex_authzs(apex, authzs, config)
+  end
 end
+auth_threads.each(&:join)
 
+
+# for each domain we use one thread to check whether the auth has been propagated
 propagation_threads = []
 domain_attrs.each_pair do |d, attrs|
   domain_attrs[d][:propagation_thread] = Thread.new(attrs['auth']) do |auth|
@@ -440,32 +461,23 @@ end
 
 propagation_threads.each(&:join)
 
-# validation_threads = {}
+
+# we make a list of all domains not yet validated
 p 'Finding unvalidated domains, initial run'
 unvalidated_domains = unvalidated_domains(domain_attrs)
+# we loop through that list and request validation until it is empty, i.e. all domains are validated
 until unvalidated_domains.empty?
   unvalidated_domains.each_pair do |_, attrs|
-  # WARNING: for some reason threading does not really work here!
-  # domain_attrs.each_pair do |d, attrs|
-    # domain_attrs[d][:auth_thread] = Thread.new(attrs) do |attrs|
-    # validation_threads[d] = Thread.new(attrs) do |attrs|
-      # wait_for_challenge_validation(auth.domain, auth.dns01)
-      # pp auth.dns01
-      # pp auth.dns01
-      # p attrs['auth'].object_id
-      # wait_for_challenge_validation(attrs['auth'].dns01, attrs['auth'].domain)
-    # acme_request_with_retries { attrs['auth'].dns01.reload }
     p 'Requesting validation of challenge'
     acme_request_with_retries { attrs['auth'].dns01.request_validation }
   end
-  # validation_threads[d] = domain_attrs[d][:auth_thread]
   p 'Finding unvalidated domains, next run'
   unvalidated_domains = unvalidated_domains(domain_attrs)
   sleep(0.2)
 end
 
-# validation_threads.each_pair { |d, t| t.join }
 
+# for each cert, we request the cert
 certs.each_pair do |name, opts|
   cert_key = read_cert_key(name)
   get_cert(opts['order'], name, opts['domain_names'], cert_key)
-- 
cgit v1.2.3