Store library key pair in library table (PP-11) (ThePalaceProject#1251)

* Update where we store library public / private key pairs.
* Add a migration

alembic/versions/20230706_04bbd03bf9f1_migrate_library_key_pair.py

@@ -0,0 +1,69 @@
+"""Migrate library key pair
+
+Revision ID: 04bbd03bf9f1
+Revises: f08f9c6bded6
+Create Date: 2023-07-06 14:40:17.970603+00:00
+
+"""
+import json
+import logging
+
+import sqlalchemy as sa
+from Crypto.PublicKey import RSA
+
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = "04bbd03bf9f1"
+down_revision = "f08f9c6bded6"
+branch_labels = None
+depends_on = None
+
+log = logging.getLogger(f"palace.migration.{revision}")
+log.setLevel(logging.INFO)
+log.disabled = False
+
+
+def upgrade() -> None:
+    # Add the new columns as nullable, add the values, then make them non-nullable
+    op.add_column(
+        "libraries",
+        sa.Column("public_key", sa.Unicode(), nullable=True),
+    )
+    op.add_column(
+        "libraries",
+        sa.Column("private_key", sa.LargeBinary(), nullable=True),
+    )
+
+    # Now we update the value stored for the key pair
+    connection = op.get_bind()
+    libraries = connection.execute("select id, short_name from libraries")
+    for library in libraries:
+        setting = connection.execute(
+            "select cs.value from configurationsettings cs "
+            "where cs.library_id = (%s) and cs.key = 'key-pair' and cs.external_integration_id IS NULL",
+            (library.id,),
+        ).fetchone()
+        if setting and setting.value:
+            _, private_key_str = json.loads(setting.value)
+            private_key = RSA.import_key(private_key_str)
+        else:
+            log.info(f"Library {library.short_name} has no key pair, generating one...")
+            private_key = RSA.generate(2048)
+
+        private_key_bytes = private_key.export_key("DER")
+        public_key_str = private_key.publickey().export_key("PEM").decode("utf-8")
+
+        connection.execute(
+            "update libraries set public_key = (%s), private_key = (%s) where id = (%s)",
+            (public_key_str, private_key_bytes, library.id),
+        )
+
+    # Then we make the columns non-nullable
+    op.alter_column("libraries", "public_key", nullable=False)
+    op.alter_column("libraries", "private_key", nullable=False)
+
+
+def downgrade() -> None:
+    op.drop_column("libraries", "private_key")
+    op.drop_column("libraries", "public_key")

api/admin/controller/library_settings.py

@@ -121,8 +121,14 @@ def process_post(self, validators_by_type=None):
 
     def create_library(self, short_name, library_uuid):
         self.require_system_admin()
+        public_key, private_key = Library.generate_keypair()
         library, is_new = create(
-            self._db, Library, short_name=short_name, uuid=str(uuid.uuid4())
+            self._db,
+            Library,
+            short_name=short_name,
+            uuid=str(uuid.uuid4()),
+            public_key=public_key,
+            private_key=private_key,
         )
         return library, is_new
 

api/authenticator.py

@@ -278,12 +278,6 @@ def __init__(
 
         self.log = logging.getLogger("Authenticator")
 
-        # Make sure there's a public/private key pair for this
-        # library. This makes it possible to register the library with
-        # discovery services. Store the public key here for
-        # convenience; leave the private key in the database.
-        self.public_key, ignore = self.key_pair
-
         if saml_providers:
             for provider in saml_providers:
                 self.saml_providers_by_name[provider.label()] = provider
@@ -705,7 +699,17 @@ def create_authentication_document(self) -> str:
             doc["service_area"] = service_area
 
         # Add the library's public key.
-        doc["public_key"] = dict(type="RSA", value=self.public_key)
+        if self.library and self.library.public_key:
+            doc["public_key"] = dict(type="RSA", value=self.library.public_key)
+        else:
+            error_library = (
+                self.library.short_name
+                if self.library
+                else f'Library ID "{self.library_id}"'
+            )
+            self.log.error(
+                f"{error_library} has no public key to include in auth document."
+            )
 
         # Add feature flags to signal to clients what features they should
         # offer.
@@ -741,12 +745,6 @@ def create_authentication_document(self) -> str:
 
         return json.dumps(doc)
 
-    @property
-    def key_pair(self) -> Tuple[str | None, str | None]:
-        """Look up or create a public/private key pair for use by this library."""
-        setting = ConfigurationSetting.for_library(Configuration.KEY_PAIR, self.library)
-        return Configuration.key_pair(setting)
-
     @classmethod
     def _geographic_areas(
         cls, library: Optional[Library]

api/config.py

@@ -1,6 +1,7 @@
 import json
 
 from Crypto.Cipher import PKCS1_OAEP
+from Crypto.Cipher.PKCS1_OAEP import PKCS1OAEP_Cipher
 from Crypto.PublicKey import RSA
 from flask_babel import lazy_gettext as _
 
@@ -168,12 +169,6 @@ class Configuration(CoreConfiguration):
     # disable.
     RESERVATIONS_FEATURE = "https://librarysimplified.org/rel/policy/reservations"
 
-    # Name of the library-wide public key configuration setting for
-    # negotiating a shared secret with a library registry. The setting
-    # is automatically generated and not editable by admins.
-    #
-    KEY_PAIR = "key-pair"
-
     SITEWIDE_SETTINGS = CoreConfiguration.SITEWIDE_SETTINGS + [
         {
             "key": BEARER_TOKEN_SIGNING_SECRET,
@@ -731,36 +726,7 @@ def configuration_contact_uri(cls, library):
         )
 
     @classmethod
-    def key_pair(cls, setting):
-        """Look up a public-private key pair in a ConfigurationSetting.
-
-        If the value is missing or incorrect, a new key pair is
-        created and stored.
-
-        TODO: This could go into ConfigurationSetting or core Configuration.
-
-        :param public_setting: A ConfigurationSetting for the public key.
-        :param private_setting: A ConfigurationSetting for the private key.
-
-        :return: A 2-tuple (public key, private key)
-        """
-        public = None
-        private = None
-
-        try:
-            public, private = setting.json_value
-        except Exception as e:
-            pass
-
-        if not public or not private:
-            key = RSA.generate(2048)
-            public = key.publickey().exportKey().decode("utf8")
-            private = key.exportKey().decode("utf8")
-            setting.value = json.dumps([public, private])
-        return public, private
-
-    @classmethod
-    def cipher(cls, key):
+    def cipher(cls, key: bytes) -> PKCS1OAEP_Cipher:
         """Create a Cipher for a public or private key.
 
         This just wraps some hard-to-remember Crypto code.

api/registration/registry.py

@@ -337,30 +337,7 @@ def push(
                 _("%r is not a valid registration stage") % stage
             )
 
-        # Verify that a public/private key pair exists for this library.
-        # This key pair is created during initialization of the
-        # LibraryAuthenticator, so this should always be present.
-        #
-        # We can't just create the key pair here because the process
-        # of pushing a registration involves the other site making a
-        # request to the circulation manager. This means the key pair
-        # needs to be committed to the database _before_ the push
-        # attempt starts.
-        key_pair = ConfigurationSetting.for_library(
-            Configuration.KEY_PAIR, self.library
-        ).json_value
-        if not key_pair:
-            # TODO: We could create the key pair _here_. The database
-            # session will be committed at the end of this request,
-            # so the push attempt would succeed if repeated.
-            return SHARED_SECRET_DECRYPTION_ERROR.detailed(
-                _(
-                    "Library %(library)s has no key pair set.",
-                    library=self.library.short_name,
-                )
-            )
-        public_key, private_key = key_pair
-        cipher = Configuration.cipher(private_key)
+        cipher = Configuration.cipher(self.library.private_key)
 
         # Before we can start the registration protocol, we must fetch
         # the remote catalog's URL and extract the link to the

core/model/library.py

@@ -3,14 +3,16 @@
 
 import logging
 from collections import Counter
-from typing import TYPE_CHECKING, List
+from typing import TYPE_CHECKING, List, Tuple
 
+from Crypto.PublicKey import RSA
 from expiringdict import ExpiringDict
 from sqlalchemy import (
     Boolean,
     Column,
     ForeignKey,
     Integer,
+    LargeBinary,
     Table,
     Unicode,
     UniqueConstraint,
@@ -155,6 +157,12 @@ class Library(Base, HasSessionCache):
         cascade="all, delete-orphan",
     )
 
+    # The library's public / private RSA key-pair.
+    # The public key is stored in PEM format.
+    public_key = Column(Unicode, nullable=False)
+    # The private key is stored in DER binary format.
+    private_key = Column(LargeBinary, nullable=False)
+
     # Typing specific
     collections: List[Collection]
 
@@ -220,6 +228,15 @@ def default(cls, _db):
         default_library.is_default = True
         return default_library
 
+    @classmethod
+    def generate_keypair(cls) -> Tuple[str, bytes]:
+        """Generate a public / private keypair for a library."""
+        private_key = RSA.generate(2048)
+        public_key = private_key.public_key()
+        public_key_str = public_key.export_key("PEM").decode("utf-8")
+        private_key_bytes = private_key.export_key("DER")
+        return public_key_str, private_key_bytes
+
     @hybrid_property
     def library_registry_short_name(self):
         """Gets library_registry_short_name from database"""

core/scripts.py

@@ -1055,13 +1055,14 @@ def do_run(self, _db=None, cmd_args=None, output=sys.stdout):
                 raise ValueError("Could not locate library '%s'" % args.short_name)
         else:
             # No existing library. Make one.
-            library, ignore = get_one_or_create(
+            public_key, private_key = Library.generate_keypair()
+            library, ignore = create(
                 _db,
                 Library,
-                create_method_kwargs=dict(
-                    uuid=str(uuid.uuid4()),
-                    short_name=args.short_name,
-                ),
+                uuid=str(uuid.uuid4()),
+                short_name=args.short_name,
+                public_key=public_key,
+                private_key=private_key,
             )
 
         if args.name:

tests/api/admin/controller/test_analytics_services.py

@@ -11,7 +11,6 @@
     AdminRole,
     ConfigurationSetting,
     ExternalIntegration,
-    Library,
     create,
     get_one,
 )
@@ -232,9 +231,7 @@ def test_analytics_services_post_errors(
             )
             assert response.uri == NO_SUCH_LIBRARY.uri
 
-        library, ignore = create(
-            settings_ctrl_fixture.ctrl.db.session,
-            Library,
+        library = settings_ctrl_fixture.ctrl.db.library(
             name="Library",
             short_name="L",
         )
@@ -272,9 +269,7 @@ def test_analytics_services_post_errors(
     def test_analytics_services_post_create(
         self, settings_ctrl_fixture: SettingsControllerFixture
     ):
-        library, ignore = create(
-            settings_ctrl_fixture.ctrl.db.session,
-            Library,
+        library = settings_ctrl_fixture.ctrl.db.library(
             name="Library",
             short_name="L",
         )
@@ -344,15 +339,11 @@ def test_analytics_services_post_create(
     def test_analytics_services_post_edit(
         self, settings_ctrl_fixture: SettingsControllerFixture
     ):
-        l1, ignore = create(
-            settings_ctrl_fixture.ctrl.db.session,
-            Library,
+        l1 = settings_ctrl_fixture.ctrl.db.library(
             name="Library 1",
             short_name="L1",
         )
-        l2, ignore = create(
-            settings_ctrl_fixture.ctrl.db.session,
-            Library,
+        l2 = settings_ctrl_fixture.ctrl.db.library(
             name="Library 2",
             short_name="L2",
         )
@@ -434,9 +425,7 @@ def test_check_name_unique(self, settings_ctrl_fixture: SettingsControllerFixtur
     def test_analytics_service_delete(
         self, settings_ctrl_fixture: SettingsControllerFixture
     ):
-        l1, ignore = create(
-            settings_ctrl_fixture.ctrl.db.session,
-            Library,
+        l1 = settings_ctrl_fixture.ctrl.db.library(
             name="Library 1",
             short_name="L1",
         )