From 97c16fbce1e9535f26353f3eab303f75cb597260 Mon Sep 17 00:00:00 2001
From: Phil Dibowitz <phil@ipom.com>
Date: Mon, 3 Feb 2025 02:16:41 -0800
Subject: [PATCH] fb_apt: Significant updates

* Deprecate `node['fb_apt']['repos']` which was always a bad API
  (sorry), and replace it with `node['fb_apt']['sources']` which
  integrates nicely with the new `node['fb_apt']['keymap']`
* Deprecate `node['fb_apt']['keys']` which was very broken on modern
  apt and replace it with a new `node['fb_apt']['keymap']`
* Update syntax for security and update repos on modern debian and
  ubuntu
* Remove old Ubuntu 16 cruft
* Lots of cleanups and refactoring for readability

Signed-off-by: Phil Dibowitz <phil@ipom.com>
---
 cookbooks/fb_apt/README.md                    |  97 +++++++++---
 cookbooks/fb_apt/attributes/default.rb        |  19 ++-
 cookbooks/fb_apt/libraries/default.rb         | 139 ++++++++++++++----
 cookbooks/fb_apt/resources/keys.rb            | 104 ++++++++++---
 cookbooks/fb_apt/resources/sources_list.rb    |  71 ++-------
 .../fb_apt/templates/default/sources.list.erb |  14 +-
 6 files changed, 304 insertions(+), 140 deletions(-)

diff --git a/cookbooks/fb_apt/README.md b/cookbooks/fb_apt/README.md
index 560f2732f..6d4da6ff2 100644
--- a/cookbooks/fb_apt/README.md
+++ b/cookbooks/fb_apt/README.md
@@ -8,21 +8,25 @@ Requirements
 
 Attributes
 ----------
+* node['fb_apt']['allow_modified_pkg_keyrings']
+* node['fb_apt']['apt_update_log_path']
 * node['fb_apt']['config']
 * node['fb_apt']['distro']
+* node['fb_apt']['keymap']
+* node['fb_apt']['keymap'][$NAME]
 * node['fb_apt']['keys']
 * node['fb_apt']['keyserver']
 * node['fb_apt']['mirror']
-* node['fb_apt']['preserve_sources_list_d']
 * node['fb_apt']['preferences']
+* node['fb_apt']['preserve_sources_list_d']
+* node['fb_apt']['preserve_unknown_keyrings']
 * node['fb_apt']['repos']
+* node['fb_apt']['sources']
+* node['fb_apt']['sources'][$NAME]
 * node['fb_apt']['update_delay']
 * node['fb_apt']['want_backports']
 * node['fb_apt']['want_non_free']
 * node['fb_apt']['want_source']
-* node['fb_apt']['preserve_unknown_keyrings']
-* node['fb_apt']['allow_modified_pkg_keyrings']
-* node['fb_apt']['apt_update_log_path']
 
 Usage
 -----
@@ -34,50 +38,94 @@ to 0. The actual update is done via the `execute[apt-get update]` resource,
 which other cookbooks can suscribe to or notify as well.
 
 ### Repository sources
+
 By default the cookbook will setup the base distribution repos based on the
 codename (as defined in `node['lsb']['codename']`) using a sensible default
 mirror for the package sources. The mirror can be customized with
-`node['fb_apt']['mirror']`; if set to `nil`, base repos will not be included
-at all in `/etc/apt/sources.list`. If base repos are enabled, the additional
+`node['fb_apt']['mirror']`; if set to `nil`, base repos will not be included at
+all in `/etc/apt/sources.list`. If base repos are enabled, the additional
 `backports` and `non-free` sources can be enabled with the
 `node['fb_apt']['want_backports']` and `node['fb_apt']['want_non_free']`
 attributes, and source code repos can be enabled with
 `node['fb_apt']['want_source']`; these all default to `false`.
 
-Additional repository sources can be added with `node['fb_apt']['repos']`. By
-default `fb_apt` will clobber existing contents in `/etc/apt/sources.list.d` to
-ensure it has full control on the repository list; this can be disabled with
+Additional repository sources can be added with `node['fb_apt']['sources']`
+in this way:
+
+```ruby
+node.default['fb_apt']['sources']['cool_repo'] = {
+  'url' => 'https://cool_repo.com/',
+  'suite' => 'stable',
+  'components' => ['main'],
+  'key' => 'cool_repo', # this references keymap, see below
+}
+```
+
+Entries in `sources` support the following keys:
+
+* `type` - The type of repo, `deb` or `deb-src` - Optional, defaults to `deb`
+* `url` - The URL of the repo
+* `suite` - The suite to pull from - usually the OS version codename
+* `components` - An array of components
+* `options` - If present, must be a hash of options to put, such as `arch`
+* `key` - A special-case option. This should be a string that maps to a key
+  in `node['fb_apt']['keymap']`. The `options` hash will be updated with the
+  `signed-by` value set to the appropriate path for the keyring generated.
+
+By default `fb_apt` will clobber existing contents in `/etc/apt/sources.list.d`
+to ensure it has full control on the repository list; this can be disabled with
 `node['fb_apt']['preserve_sources_list_d']`.
 
+*NOTE*: Older versions of this cookbook used `node['fb_apt']['repos']`. This
+is deprecated. As of this writing, sources in this list will still be added
+to the system, but a warning will be printed. The old syntax was significantly
+lacking, didn't play well with keys, and was hard to modify.
+
 ### Keys
-They `keys` hash is pre-populated with any keys from pkg-owned keyrings that
-exist in `/etc/apt/trusted.gpg.d/` so you don't need to worry about keeping
-a list of repository keys in sync.
 
-You can add to this, but setting a key of your keyid and a value of either `nil`
-or the PEM-encoded key. If `key` is `nil` the key will be automatically fetched
-from the `node['fb_apt']['keyserver']` keyserver (`keys.gnupg.net` by default).
-Example:
+The `node['fb_apt']['keymap']` is designed to make it easy to work with the
+per-repo keys that modern Apt requires. Simple associate a PEM value with a
+name, and then use that name in any entries in `node['fb_apt']['sources']`
+signed by that key. `fb_apt` will take the PEM, generate a keyring in
+`/etc/apt/trusted.gpg.d/${NAME}.gpg` and populate the signed-by values in your
+`sources.list`.
 
-```
-node.default['fb_apt']['keys']['94558F59'] = nil
-node.default['fb_apt']['keys']['F3EFDBD9'] = <<-eos
+For example:
+
+```ruby
+node.default['fb_apt']['keys']['cool'] = <<-eos
 -----BEGIN PGP PUBLIC KEY BLOCK-----
 ...
+-----END PGP PUBLIC KEY BLOCK-----
 eos
+
+node.default['fb_apt']['sources']['cool_app'] = {
+  ...
+  'key' => 'cool',
+}
 ```
 
-Automatic key fetching can be disabled by setting the keyserver to `nil`; this
-will produce an exception for any unspecified key.
+You can also make the value a http/https URL, but if you do, the file will be
+placed as-is in `trusted.gpg.d`, so it must be of the right format. Chef's
+`remote_file` resource will be used to manage the file. This is intended for
+repos who make full keyrings available instead of armored PEMs.
 
-By default any keyring in `/etc/apt/trusted.gpg.d` that is not owned by a
-package will be deleted unless you set `preserve_unknown_keyrings` to false.
+Anything in `/etc/apt/trusted.gpg.d` that is owned by a package or by this
+cookbook will be kept, but any other file in there will be removed.  unless you
+set `preserve_unknown_keyrings` to false.
 
 If a keyring owned by a package is found to have been modified (based on
 `dpkg -V`), then the run will fail, unless `allow_modified_pkg_keyrings` is
 set.
 
+*NOTE*: Older versions of this cookbook used `node['fb_apt']['keys']` which
+attempted to pull keyid's from the internet and load them via the now-deprecated
+`apt-key`. Use of that API will cause a warning, though this cookbook does still
+support it for now. However, modern `apt-key` does nothing, so your config will
+break if you do not migrate.
+
 ### Configuration
+
 APT behaviour can be customized using `node['fb_apt']['config']`, which will be
 used to populate `/etc/apt/apt.conf`. Note that this will take precedence over
 anything in `/etc/apt/apt.conf.d`. Example:
@@ -89,6 +137,7 @@ node.default['fb_apt']['config']['Acquire::http'].merge!({
 ```
 
 ### Preferences
+
 You can fine tune which versions of packages will be selected for installation
 by tweaking APT preferences via `node['fb_apt']['preferences']`. Note that we
 clobber the contents of `/etc/apt/preferences.d` to ensure this always takes
@@ -104,12 +153,14 @@ node.default['fb_apt']['preferences'][
 ```
 
 ### Distro
+
 As mentioned above, `fb_apt` can assemble the basic sources for you. It uses
 the LSB "codename" of the current systemd to build the URLs. In the event you
 want to use Chef to upgrade across distros, however, you can set
 `node['fb_apt']['distro']` to the appropriate name and it will be used instead.
 
 ### Logging `apt-get update`
+
 Set `node['fb_apt']['apt_update_log_path']` to log stdout and stderr of the
 `apt-get update` command invoked by this cookbook. This may be useful for
 debugging purposes. The caller must handle log rotation.
diff --git a/cookbooks/fb_apt/attributes/default.rb b/cookbooks/fb_apt/attributes/default.rb
index f4f21781f..b442d9985 100644
--- a/cookbooks/fb_apt/attributes/default.rb
+++ b/cookbooks/fb_apt/attributes/default.rb
@@ -25,21 +25,24 @@
 end
 
 default['fb_apt'] = {
+  'allow_modified_pkg_keyrings' => false,
+  'apt_update_log_path' => nil,
   'config' => {},
-  'repos' => [],
+  'distro' => nil,
+  'keymap' => {},
+  # deprecated, use keymap instead
+  'keys' => {},
   'keyserver' => 'keys.gnupg.net',
   'mirror' => mirror,
-  'security_mirror' => security_mirror,
   'preferences' => {},
   'preserve_sources_list_d' => false,
+  'preserve_unknown_keyrings' => false,
+  # deprecated, use sources instead
+  'repos' => [],
+  'security_mirror' => security_mirror,
+  'sources' => {},
   'update_delay' => 86400,
   'want_backports' => false,
   'want_non_free' => false,
   'want_source' => false,
-  'preserve_unknown_keyrings' => false,
-  'allow_modified_pkg_keyrings' => false,
-  'apt_update_log_path' => nil,
 }
-# fb_apt must be defined for this to work...
-keys = FB::Apt.get_official_keyids(node).map { |id| [id, nil] }.to_h
-default['fb_apt']['keys'] = keys
diff --git a/cookbooks/fb_apt/libraries/default.rb b/cookbooks/fb_apt/libraries/default.rb
index 306e743ff..311014b38 100644
--- a/cookbooks/fb_apt/libraries/default.rb
+++ b/cookbooks/fb_apt/libraries/default.rb
@@ -19,6 +19,9 @@
 module FB
   # APT utility functions
   class Apt
+    TRUSTED_D = '/etc/apt/trusted.gpg.d'.freeze
+    PEM_D = "#{Chef::Config[:file_cache_path]}/fb_apt_pems".freeze
+
     # Internal helper function to generate /etc/apt.conf entries
     def self._gen_apt_conf_entry(k, v, i = 0)
       indent = ' ' * i
@@ -116,35 +119,121 @@ def self._extract_keyids(rings)
       end.flatten
     end
 
-    # Here ye here ye, read this before touching keys!
-    #
-    # On modern debian and ubuntu, all keys are stored in files in
-    # `/etc/apt/trusted.gpg.d/`, and **never** on `/etc/apt/trusted.gpg`,
-    # this we can know what the Distro keys are by reading all keys in
-    # all keyring files owned by packages. So what's what we populate
-    # the default list with.
-    #
-    # However, for Ubuntu <= 16.04 they are on the `/etc/apt/trusted.gpg` list,
-    # so we hard-code those, the distros are old enough they won't change.
-    def self.get_official_keyids(node)
-      if node.ubuntu? && node['platform_version'].to_i <= 16
-        return %w{
-          40976EAF437D05B5
-          46181433FBB75451
-          3B4FE6ACC0B21F32
-          D94AA3F0EFE21092
-          0BFB847F3F272F5B
+    def self.get_legacy_keyids
+      _extract_keyids(['/etc/apt/trusted.gpg'])
+    end
+
+    def self.determine_base_repo_components(node)
+      components = %w{main}
+      if node.ubuntu?
+        components << 'universe'
+      end
+
+      if node['fb_apt']['want_non_free']
+        if node.debian?
+          components += %w{contrib non-free non-free-firmware}
+        elsif node.ubuntu?
+          components += %w{restricted multiverse}
+        else
+          fail "Don't know how to setup non-free for #{node['platform']}"
+        end
+      end
+
+      components
+    end
+
+    def self.base_sources(node)
+      base_repos = {}
+      sources = {}
+      mirror = node['fb_apt']['mirror']
+      security_mirror = node['fb_apt']['security_mirror']
+      # By default, we want our current distro to assemble to repo URLs.
+      # However, for when people want to upgrade across distros, we let
+      # them specify a distro to upgrade to.
+      distro = node['fb_apt']['distro'] || node['lsb']['codename']
+
+      # only add base repos if mirror is set and codename is available
+      if mirror && distro
+        components = FB::Apt.determine_base_repo_components(node)
+
+        base_repos['base'] = {
+          'url' => mirror,
+          'suite' => distro,
         }
+
+        # Security updates
+        pv = node['platform_version'].to_i
+        if node.debian? && distro != 'sid' && pv != 0 && pv > 9
+          # In buster/10 and before the suite was ${distro}/updates
+          # After that it became ${distro}-security
+          suite = pv == 10 ? "#{distro}/updates" : "#{distro}-security"
+          base_repos['security'] = {
+            'url' => "#{security_mirror}debian-security",
+            'suite' => suite,
+          }
+        elsif node.ubuntu?
+          base_repos['security'] = {
+            'url' => security_mirror,
+            'suite' => "#{distro}-security",
+          }
+        end
+
+        # Debian Sid doesn't have updates or backports
+        unless node.debian? && distro == 'sid'
+          # Stable updates
+          base_repos['updates'] = {
+            'url' => mirror,
+            'suite' => "#{distro}-updates",
+          }
+
+          if node['fb_apt']['want_backports']
+            base_repos['backports'] = {
+              'url' => mirror,
+              'suite' => "#{distro}-backports",
+            }
+          end
+        end
+
+        base_keyring = node.debian? ?
+          '/usr/share/keyrings/debian-archive-keyring.gpg' :
+          '/usr/share/keyrings/ubuntu-archive-keyring.gpg'
+        base_repos.each do |name, config|
+          config.merge!({
+                          'options' => {
+                            'signed-by' => base_keyring,
+                          },
+            'components' => components,
+            'type' => 'deb',
+                        })
+          sources[name] = config
+          if node['fb_apt']['want_source']
+            source["#{name}_src"] = config.merge({ 'type' => 'deb-src' })
+          end
+        end
       end
-      keyids = _extract_keyids(_get_owned_keyring_files(node))
-      Chef::Log.debug("fb_apt[keys]: Official keyids: #{keyids}")
-      keyids
+      sources
+    end
+
+    def self.gen_sources_line(config)
+      type = config['type'] || 'deb'
+      options = config['options'].dup || {}
+      if config['key']
+        options['signed-by'] = keyring_path_from_name(config['key'])
+      end
+      c_str = config['components'].join(' ')
+      options_str = ''
+      unless options.empty?
+        options_str = "[#{options.map { |k, v| "#{k}=#{v}" }.join(' ')}] "
+      end
+      "#{type} #{options_str}#{config['url']} #{config['suite']} #{c_str}"
+    end
+
+    def self.pem_path_from_name(name)
+      "#{PEM_D}/#{name}.asc"
     end
 
-    def self.get_installed_keyids(node)
-      rings = _get_owned_keyring_files(node)
-      rings << '/etc/apt/trusted.gpg'
-      _extract_keyids(rings)
+    def self.keyring_path_from_name(name)
+      "#{TRUSTED_D}/#{name}.gpg"
     end
   end
 end
diff --git a/cookbooks/fb_apt/resources/keys.rb b/cookbooks/fb_apt/resources/keys.rb
index 02c9b591d..a6c6bb49a 100644
--- a/cookbooks/fb_apt/resources/keys.rb
+++ b/cookbooks/fb_apt/resources/keys.rb
@@ -18,32 +18,94 @@
 
 unified_mode(false) if Chef::VERSION >= 18 # TODO(T144966423)
 action :run do
-  keyserver = node['fb_apt']['keyserver']
-  desired_keys = node['fb_apt']['keys'].to_hash
+  desired_keyids = node['fb_apt']['keys']
+  desired_keys = node['fb_apt']['keymap']
 
-  if desired_keys
-    installed_keys = FB::Apt.get_installed_keyids(node)
-    Chef::Log.debug(
-      "fb_apt[keys]: Installed keys: #{installed_keys.join(', ')}",
-    )
+  # if the user hasn't specified any keys of any time, don't manage
+  # keys in anyway
+  unless desired_keys || desired_keyids
+    return
+  end
 
-    legit_keyrings = FB::Apt._get_owned_keyring_files(node)
-    Dir.glob('/etc/apt/trusted.gpg.d/*').each do |keyring|
-      next if legit_keyrings.include?(keyring)
+  directory FB::Apt::PEM_D do
+    owner node.root_user
+    group node.root_group
+    mode '0755'
+  end
 
-      if node['fb_apt']['preserve_unknown_keyrings']
-        Chef::Log.warn(
-          "fb_apt[keys]: Unknown keyring #{keyring} being preserved!",
-        )
-      else
-        file keyring do
-          action :delete
-        end
+  # Remove unwanted keyrings
+  legit_keyrings = FB::Apt._get_owned_keyring_files(node) +
+    desired_keys.keys.map { |x| FB::Apt.keyring_path_from_name(x) }
+  Dir.glob("#{FB::Apt::TRUSTED_D}/*").each do |keyring|
+    next if legit_keyrings.include?(keyring)
+
+    if node['fb_apt']['preserve_unknown_keyrings']
+      Chef::Log.warn(
+        "fb_apt[keys]: Unknown keyring #{keyring} being preserved!",
+      )
+    else
+      file keyring do
+        action :delete
+      end
+    end
+  end
+  Dir.glob("#{FB::Apt::PEM_D}/*").each do |pem|
+    basename = ::File.basename(pem, '.asc')
+    next if desired_keys[basename]
+    file pem do
+      action :delete
+    end
+  end
+
+  # Generate wanted keyrings from PEMs passed in
+  desired_keys.each do |name, key|
+    src = FB::Apt.pem_path_from_name(name)
+    dst = FB::Apt.keyring_path_from_name(name)
+    if key.start_with?('http')
+      remote_file dst do
+        source key
+        owner node.root_user
+        group node.root_group
+        mode '0644'
       end
+      next
+    end
+
+    file src do
+      owner node.root_user
+      group node.root_group
+      mode '0644'
+      content "# This file is staging for Chef's fb_apt\n#{key}"
+      # delete the file or gpg will prompt to overwrite it
+      notifies :delete, "file[#{dst}]", :immediately
+      notifies :run, "execute[generate #{name} keyring]", :immediately
+    end
+
+    file dst do
+      action :nothing
     end
 
-    # Process keys to add
-    desired_keys.each do |keyid, key|
+    execute "generate #{name} keyring" do
+      command "gpg --dearmor -o #{dst} #{src}"
+      action :nothing
+    end
+  end
+
+  # Begin support for LEGACY stuff
+  #
+  # This stuff uses apt-key (deprecated) to add/remove/list stuff from
+  # /etc/apt/trusted.gpg. It even attempts to download keys from the internet,
+  # which is also deprecated.
+  unless desired_keyids.empty?
+    Chef::Log.warn(
+      'fb_apt: `node["fb_apt"]["keys"]` is deprecated! Please migrate to' +
+      ' `node["fb_apt"]["keymap"]',
+    )
+    installed_keys = FB::Apt.get_legacy_keyids
+
+    # Walk legacy keys and install them. This will install into
+    # the deprecated /etc/apt/trusted.gpg and is not gauranteed to work.
+    desired_keyids.each do |keyid, key|
       if installed_keys.include?(keyid)
         Chef::Log.debug(
           "fb_apt[keys]: Skipping keyid #{keyid} as it's already registered",
@@ -67,7 +129,7 @@
       end
     end
 
-    # Process keys to remove
+    # Then walk everything installed and remove what we don't expect
     installed_keys.each do |keyid|
       if desired_keys.keys.include?(keyid)
         Chef::Log.debug("fb_apt[keys]: Not deleting added keyid #{keyid}")
diff --git a/cookbooks/fb_apt/resources/sources_list.rb b/cookbooks/fb_apt/resources/sources_list.rb
index 344a47071..d5768c50f 100644
--- a/cookbooks/fb_apt/resources/sources_list.rb
+++ b/cookbooks/fb_apt/resources/sources_list.rb
@@ -18,66 +18,17 @@
 
 unified_mode(false) if Chef::VERSION >= 18 # TODO(T144966423)
 action :run do
-  mirror = node['fb_apt']['mirror']
-  security_mirror = node['fb_apt']['security_mirror']
-  # By default, we want our current distro to assemble to repo URLs.
-  # However, for when people want to upgrade across distros, we let
-  # them specify a distro to upgrade to.
-  distro = node['fb_apt']['distro'] || node['lsb']['codename']
-
-  # only add base repos if mirror is set and codename is available
-  if mirror && distro
-    components = %w{main}
-    if node.ubuntu?
-      components << 'universe'
-    end
-
-    if node['fb_apt']['want_non_free']
-      if node.debian?
-        components += %w{contrib non-free}
-      elsif node.ubuntu?
-        components += %w{restricted multiverse}
-      else
-        fail "Don't know how to setup non-free for #{node['platform']}"
-      end
-    end
-
-    components_entry = components.join(' ')
-    base_repos = [
-      # Main repo
-      "#{mirror} #{distro} #{components_entry}",
-    ]
-
-    # Security updates
-    if node.debian? && distro != 'sid'
-      base_repos <<
-        "#{security_mirror} #{distro}/updates #{components_entry}"
-    elsif node.ubuntu?
-      base_repos <<
-        "#{security_mirror} #{distro}-security " +
-        components_entry
-    end
-
-    # Debian Sid doesn't have updates or backports
-    unless node.debian? && distro == 'sid'
-      # Stable updates
-      base_repos << "#{mirror} #{distro}-updates #{components_entry}"
-
-      if node['fb_apt']['want_backports']
-        base_repos << "#{mirror} #{distro}-backports #{components_entry}"
-      end
-    end
-
-    repos = []
-    base_repos.each do |repo|
-      repos << "deb #{repo}"
-      if node['fb_apt']['want_source']
-        repos << "deb-src #{repo}"
-      end
-    end
-
-    # update repos list and ensure base repos come first
-    node.default['fb_apt']['repos'] = repos + node['fb_apt']['repos']
+  base_sources = FB::Apt.base_sources(node)
+  # update repos list and ensure base repos come first
+  node.default['fb_apt']['sources'] = base_sources.merge(
+    node['fb_apt']['sources'],
+  )
+
+  unless node['fb_apt']['repos'].empty?
+    Chef::Log.warn(
+      'fb_apt: `node["fb_apt"]["repos"]` is deprecated. Please migrate to' +
+      ' `node["fb_apt"]["sources"]`.',
+    )
   end
 
   template '/etc/apt/sources.list' do
diff --git a/cookbooks/fb_apt/templates/default/sources.list.erb b/cookbooks/fb_apt/templates/default/sources.list.erb
index 90d46c8d0..1054c5954 100644
--- a/cookbooks/fb_apt/templates/default/sources.list.erb
+++ b/cookbooks/fb_apt/templates/default/sources.list.erb
@@ -1,6 +1,14 @@
 # This file is maintained by Chef. Do not edit, all changes will be
 # overwritten. See fb_apt/README.md
 
-<% node['fb_apt']['repos'].each do |repo| -%>
-<%=  repo %>
-<% end -%>
+<% node['fb_apt']['sources'].each do |name, config| %>
+# <%= name %>
+<%=  FB::Apt.gen_sources_line(config) %>
+<% end %>
+<% unless node['fb_apt']['repos'].empty? %>
+# Repos from the legacy node['fb_apt']['repos'] - please migrate
+# these to node['fb_apt']['sources']
+<%   node['fb_apt']['repos'].each do |repo| %>
+<%=    repo %>
+<%   end %>
+<% end %>