From 4f815c200f62cf4e4bf660649066d9bd65393ae0 Mon Sep 17 00:00:00 2001
From: J08nY
Date: Mon, 31 Jul 2017 20:43:37 +0200
Subject: Refactor filesystem key store operations into FSKey. Introduce DBKey.

---
 src/mailman_pgp/model/address.py         | 44 +++++----------
 src/mailman_pgp/model/db_key.py          | 53 ++++++++++++++++++
 src/mailman_pgp/model/fs_key.py          | 95 ++++++++++++++++++++++++++++++++
 src/mailman_pgp/model/list.py            | 68 ++++++++++-------------
 src/mailman_pgp/pgp/keygen.py            | 36 +++++++-----
 src/mailman_pgp/pgp/tests/test_keygen.py | 93 +++++++++++++++++--------------
 src/mailman_pgp/pgp/tests/test_pgp.py    |  7 ++-
 src/mailman_pgp/styles/base.py           |  4 +-
 src/mailman_pgp/utils/file.py            | 62 +++++++++++++++++++++
 9 files changed, 337 insertions(+), 125 deletions(-)
 create mode 100644 src/mailman_pgp/model/db_key.py
 create mode 100644 src/mailman_pgp/model/fs_key.py
 create mode 100644 src/mailman_pgp/utils/file.py

diff --git a/src/mailman_pgp/model/address.py b/src/mailman_pgp/model/address.py
index 0cfefe2..a912e87 100644
--- a/src/mailman_pgp/model/address.py
+++ b/src/mailman_pgp/model/address.py
@@ -16,18 +16,16 @@
 # this program.  If not, see <http://www.gnu.org/licenses/>.
 
 """Model for PGP enabled addresses."""
-import os
-from os.path import exists, isfile, join
 
 from mailman.database.types import SAUnicode
 from mailman.interfaces.usermanager import IUserManager
-from pgpy import PGPKey
 from sqlalchemy import Boolean, Column, Integer, String
 from sqlalchemy.orm import reconstructor
 from zope.component import getUtility
 
 from mailman_pgp.config import config
 from mailman_pgp.model.base import Base
+from mailman_pgp.model.fs_key import FSKey
 
 
 class PGPAddress(Base):
@@ -41,15 +39,15 @@ class PGPAddress(Base):
     key_confirmed = Column(Boolean, default=False)
 
     def __init__(self, address):
-        super().__init__()
+        super().__init__(email=address.email)
         self._init()
-        self.email = address.email
         self._address = address
 
     @reconstructor
     def _init(self):
         self._address = None
-        self._key = None
+        self._key = FSKey(config.pgp.keydir_config['user_keydir'],
+                          self.email + '.asc', True)
 
     @property
     def key(self):
@@ -58,33 +56,22 @@ class PGPAddress(Base):
         :return:
         :rtype: pgpy.PGPKey
         """
-        if self.key_fingerprint is None:
-            return None
-        if self._key is None:
-            if exists(self.key_path) and isfile(self.key_path):
-                self._key, _ = PGPKey.from_file(self.key_path)
-        return self._key
+        self._key.reload()
+        return self._key.key
 
     @key.setter
-    def key(self, new_key):
+    def key(self, value):
         """
 
-        :param new_key:
-        :type new_key: PGPKey
+        :param value:
+        :type value: pgpy.PGPKey
         """
-        if self.key_fingerprint is not None:
-            try:
-                os.remove(self.key_path)
-            except FileNotFoundError:
-                pass
-        if new_key is None:
+        if value is None:
             self.key_fingerprint = None
-            self._key = None
         else:
-            self.key_fingerprint = str(new_key.fingerprint)
-            with open(self.key_path, 'w') as out:
-                out.write(str(new_key))
-            self._key = new_key
+            self.key_fingerprint = value.fingerprint
+        self._key.key = value
+        self._key.save()
 
     @property
     def key_path(self):
@@ -93,10 +80,7 @@ class PGPAddress(Base):
         :return:
         :rtype: str
         """
-        if self.key_fingerprint is None:
-            return None
-        return join(config.pgp.keydir_config['user_keydir'],
-                    self.key_fingerprint + '.asc')
+        return self._key.key_path
 
     @property
     def address(self):
diff --git a/src/mailman_pgp/model/db_key.py b/src/mailman_pgp/model/db_key.py
new file mode 100644
index 0000000..45eafc7
--- /dev/null
+++ b/src/mailman_pgp/model/db_key.py
@@ -0,0 +1,53 @@
+# Copyright (C) 2017 Jan Jancar
+#
+# This file is a part of the Mailman PGP plugin.
+#
+# This program 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.
+#
+# This program 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
+# this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Database stored PGP key."""
+from sqlalchemy import Column, Integer, LargeBinary
+from sqlalchemy.orm import reconstructor
+
+from mailman_pgp.model.base import Base
+from mailman_pgp.utils.pgp import key_from_blob
+
+
+class DBKey(Base):
+    """Database stored PGP key."""
+    __tablename__ = 'keys'
+
+    id = Column(Integer, primary_key=True)
+    key_material = Column(LargeBinary)
+
+    def __init__(self):
+        super().__init__()
+
+    @reconstructor
+    def _init(self):
+        self._key = None
+
+    @property
+    def key(self):
+        if self._key is None:
+            self._key = key_from_blob(self.key_material)
+        return self._key
+
+    @key.setter
+    def key(self, value):
+        if value is None:
+            self._key = None
+            self.key_material = bytes()
+        else:
+            self._key = value
+            self.key_material = bytes(value)
diff --git a/src/mailman_pgp/model/fs_key.py b/src/mailman_pgp/model/fs_key.py
new file mode 100644
index 0000000..c7c86eb
--- /dev/null
+++ b/src/mailman_pgp/model/fs_key.py
@@ -0,0 +1,95 @@
+# Copyright (C) 2017 Jan Jancar
+#
+# This file is a part of the Mailman PGP plugin.
+#
+# This program 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.
+#
+# This program 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
+# this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Filesystem stored PGP key."""
+from os import remove
+from os.path import getmtime, join
+
+from mailman_pgp.utils.file import locked_obj
+from mailman_pgp.utils.pgp import key_from_file
+
+
+class FSKey:
+    """Filesystem stored PGP key."""
+
+    def __init__(self, keydir, keyfile, load=False):
+        self._key = None
+        self._mtime = None
+        self.keydir = keydir
+        self.keyfile = keyfile
+        if load:
+            self.load()
+
+    @property
+    def key(self):
+        """
+
+        :rtype: pgpy.PGPKey
+        """
+        return self._key
+
+    @key.setter
+    def key(self, value):
+        """
+
+        :param value:
+        :type value: pgpy.PGPKey
+        """
+        self._key = value
+
+    @property
+    def key_path(self):
+        return join(self.keydir, self.keyfile)
+
+    @property
+    def lock_path(self):
+        return self.key_path + '.lock'
+
+    def _load(self):
+        try:
+            self.key = key_from_file(self.key_path)
+            self._mtime = getmtime(self.key_path)
+        except FileNotFoundError:
+            self.key = None
+            self._mtime = None
+
+    @locked_obj('lock_path')
+    def load(self):
+        self._load()
+
+    @locked_obj('lock_path')
+    def reload(self):
+        if self.key is None:
+            self._load()
+        else:
+            mtime = getmtime(self.key_path)
+            if self._mtime is None or mtime > self._mtime:
+                self._load()
+
+    @locked_obj('lock_path')
+    def save(self):
+        if self.key is None:
+            remove(self.key_path)
+            self._mtime = None
+        else:
+            with open(self.key_path, 'w') as key_file:
+                key_file.write(str(self.key))
+            self._mtime = getmtime(self.key_path)
+
+    @locked_obj('lock_path')
+    def delete(self):
+        remove(self.key_path)
diff --git a/src/mailman_pgp/model/list.py b/src/mailman_pgp/model/list.py
index 8448368..838bcab 100644
--- a/src/mailman_pgp/model/list.py
+++ b/src/mailman_pgp/model/list.py
@@ -17,14 +17,9 @@
 
 """Model for PGP enabled mailing lists."""
 
-from os import remove
-from os.path import exists, isfile, join
-
-from flufl.lock import Lock
 from mailman.database.types import Enum, SAUnicode
 from mailman.interfaces.action import Action
 from mailman.interfaces.listmanager import (IListManager, ListDeletingEvent)
-from pgpy import PGPKey
 from public import public
 from sqlalchemy import Boolean, Column, Integer
 from sqlalchemy.orm import reconstructor
@@ -34,7 +29,7 @@ from zope.event import classhandler
 from mailman_pgp.config import config
 from mailman_pgp.database import transaction
 from mailman_pgp.model.base import Base
-from mailman_pgp.pgp.keygen import ListKeyGenerator
+from mailman_pgp.model.fs_key import FSKey
 
 
 @public
@@ -44,7 +39,7 @@ class PGPMailingList(Base):
     __tablename__ = 'pgp_lists'
 
     id = Column(Integer, primary_key=True)
-    list_id = Column(SAUnicode, index=True)
+    list_id = Column(SAUnicode, index=True, unique=True)
 
     # Signature related properties
     unsigned_msg_action = Column(Enum(Action), default=Action.reject)
@@ -61,16 +56,20 @@ class PGPMailingList(Base):
     encrypt_outgoing = Column(Boolean, default=True)
 
     def __init__(self, mlist):
-        super().__init__()
+        """
+
+        :param mlist:
+        :type mlist: mailman.model.mailinglist.MailingList
+        """
+        super().__init__(list_id=mlist.list_id)
         self._init()
-        self.list_id = mlist.list_id
         self._mlist = mlist
 
     @reconstructor
     def _init(self):
         self._mlist = None
-        self._key = None
-        self._key_generator = None
+        self._key = FSKey(config.pgp.keydir_config['list_keydir'],
+                          self.list_id + '.asc', True)
 
     @property
     def mlist(self):
@@ -83,45 +82,35 @@ class PGPMailingList(Base):
             self._mlist = getUtility(IListManager).get_by_list_id(self.list_id)
         return self._mlist
 
+    @property
+    def fs_key(self):
+        return self._key
+
     @property
     def key(self):
         """
+        The private part of the list's keypair.
 
         :return:
         :rtype: pgpy.PGPKey
         """
-        if self._key is None:
-            # Check the file
-            if exists(self.key_path) and isfile(self.key_path):
-                self._key, _ = PGPKey.from_file(self.key_path)
-        return self._key
+        self._key.reload()
+        return self._key.key
 
     @key.setter
     def key(self, value):
-        with Lock(self.key_path + '.lock'):
-            self._key = value
-            if value is None:
-                remove(self.key_path)
-            else:
-                with open(self.key_path, 'w') as key_file:
-                    key_file.write(str(value))
-
-    def generate_key(self, block=False):
-        self._key = None
-        self._key_generator = ListKeyGenerator(config.pgp.primary_key_args,
-                                               config.pgp.sub_key_args,
-                                               self.mlist.display_name,
-                                               self.mlist.posting_address,
-                                               self.mlist.request_address,
-                                               self.key_path)
-        self._key_generator.start()
-        if block:
-            self._key_generator.join()
-            return self.key
+        """
+
+        :param value:
+        :type value:
+        """
+        self._key.key = value
+        self._key.save()
 
     @property
     def pubkey(self):
         """
+        The public part of the list's keypair.
 
         :return:
         :rtype: pgpy.PGPKey
@@ -133,18 +122,19 @@ class PGPMailingList(Base):
     @property
     def key_path(self):
         """
+        The path to this list's key in the `list_keydir`.
 
-        :return:
+        :return: List key path.
         :rtype: str
         """
-        return join(config.pgp.keydir_config['list_keydir'],
-                    self.list_id + '.asc')
+        return self._key.key_path
 
     @staticmethod
     def for_list(mlist):
         """
 
         :param mlist:
+        :type mlist: mailman.model.mailinglist.MailingList
         :return:
         :rtype: PGPMailingList|None
         """
diff --git a/src/mailman_pgp/pgp/keygen.py b/src/mailman_pgp/pgp/keygen.py
index 684b81a..57a94d7 100644
--- a/src/mailman_pgp/pgp/keygen.py
+++ b/src/mailman_pgp/pgp/keygen.py
@@ -25,22 +25,32 @@ from pgpy import PGPKey, PGPUID
 from pgpy.constants import (
     CompressionAlgorithm, HashAlgorithm, KeyFlags, SymmetricKeyAlgorithm)
 
+from mailman_pgp.config import config
+from mailman_pgp.utils.pgp import key_from_file
+
 
 class ListKeyGenerator(mp.Process):
     """A multiprocessing list key generator."""
 
-    def __init__(self, primary_args, subkey_args, display_name,
-                 posting_address,
-                 request_address, key_path):
+    def __init__(self, pgp_list):
         super().__init__(
-                target=self.generate,
-                args=(primary_args, subkey_args, display_name, posting_address,
-                      request_address, key_path),
+                target=self._run,
+                args=(config.pgp.primary_key_args, config.pgp.sub_key_args,
+                      pgp_list.mlist.display_name,
+                      pgp_list.mlist.posting_address,
+                      pgp_list.mlist.request_address,
+                      pgp_list.key_path),
                 daemon=True)
+        self._pgp_list = pgp_list
+
+    def generate(self, block=False):
+        self.start()
+        if block:
+            self.join()
+            return key_from_file(self._pgp_list.key_path)
 
-    def generate(self, primary_args, subkey_args, display_name,
-                 posting_address,
-                 request_address, key_path):
+    def _run(self, primary_args, subkey_args, display_name, posting_address,
+             request_address, key_path):
         """
         Generate the list keypair and save it.
 
@@ -51,11 +61,11 @@ class ListKeyGenerator(mp.Process):
         :param request_address:
         :param key_path:
         """
-        key = self._create(primary_args, subkey_args, display_name,
-                           posting_address,
-                           request_address)
+        self.key = self._create(primary_args, subkey_args, display_name,
+                                posting_address,
+                                request_address)
         with Lock(key_path + '.lock'):
-            self._save(key, key_path)
+            self._save(self.key, key_path)
 
     def _create(self, primary_args, subkey_args, display_name, posting_address,
                 request_address):
diff --git a/src/mailman_pgp/pgp/tests/test_keygen.py b/src/mailman_pgp/pgp/tests/test_keygen.py
index a3a5499..7de1a19 100644
--- a/src/mailman_pgp/pgp/tests/test_keygen.py
+++ b/src/mailman_pgp/pgp/tests/test_keygen.py
@@ -16,25 +16,29 @@
 # this program.  If not, see <http://www.gnu.org/licenses/>.
 
 """Test the out-of-process key generator."""
-from os.path import exists, isfile, join
-from tempfile import TemporaryDirectory
+from os.path import exists, isfile
 from unittest import TestCase
 
+from mailman.app.lifecycle import create_list
 from parameterized import parameterized
-from pgpy import PGPKey
 from pgpy.constants import EllipticCurveOID, PubKeyAlgorithm
 
+from mailman_pgp.config import config
+from mailman_pgp.database import mm_transaction
+from mailman_pgp.model.list import PGPMailingList
 from mailman_pgp.pgp.keygen import ListKeyGenerator
-from mailman_pgp.testing.layers import PGPLayer
+from mailman_pgp.testing.layers import PGPConfigLayer
+from mailman_pgp.utils.pgp import key_from_file
 
 
 class TestKeygen(TestCase):
-    layer = PGPLayer
+    layer = PGPConfigLayer
 
     def setUp(self):
-        self.display_name = 'Display Name'
-        self.posting_address = 'posting@address.com'
-        self.request_address = 'posting-request@address.com'
+        with mm_transaction():
+            self.mlist = create_list('test@example.com',
+                                     style_name='pgp-default')
+        self.pgp_list = PGPMailingList.for_list(self.mlist)
 
     @parameterized.expand([
         # RSA + RSA
@@ -49,36 +53,43 @@ class TestKeygen(TestCase):
     ])
     def test_generate(self, primary_key_type, primary_key_size, sub_key_type,
                       sub_key_size):
-        with TemporaryDirectory() as temp_dir:
-            key_path = join(temp_dir, 'key.asc')
-            keygen = ListKeyGenerator((primary_key_type, primary_key_size),
-                                      (sub_key_type, sub_key_size),
-                                      self.display_name,
-                                      self.posting_address,
-                                      self.request_address, key_path)
-            keygen.start()
-            keygen.join()
-            self.assertTrue(exists(key_path))
-            self.assertTrue(isfile(key_path))
-
-            key, _ = PGPKey.from_file(key_path)
-            self.assertEqual(key.key_algorithm,
-                             primary_key_type)
-            self.assertEqual(key.key_size,
-                             primary_key_size)
-
-            subs = key.subkeys
-            self.assertEqual(len(subs), 1)
-
-            keyid, sub = subs.popitem()
-            self.assertEqual(sub.key_algorithm,
-                             sub_key_type)
-            self.assertEqual(sub.key_size,
-                             sub_key_size)
-
-            uids = key.userids
-            self.assertEqual(len(uids), 2)
-            for uid in uids:
-                self.assertEqual(uid.name, self.display_name)
-                self.assertIn(uid.email,
-                              (self.posting_address, self.request_address))
+        def reset_primary(primary_key_args):
+            config.pgp.primary_key_args = primary_key_args
+
+        self.addCleanup(reset_primary, config.pgp.primary_key_args)
+
+        def reset_sub(sub_key_args):
+            config.pgp.sub_key_args = sub_key_args
+
+        self.addCleanup(reset_sub, config.pgp.sub_key_args)
+
+        config.pgp.primary_key_args = (primary_key_type, primary_key_size)
+        config.pgp.sub_key_args = (sub_key_type, sub_key_size)
+
+        key_path = self.pgp_list.key_path
+        keygen = ListKeyGenerator(self.pgp_list)
+        ret_key = keygen.generate(True)
+        list_key = self.pgp_list.key
+        self.assertTrue(exists(key_path))
+        self.assertTrue(isfile(key_path))
+
+        key = key_from_file(key_path)
+        self.assertEqual(key.key_algorithm, primary_key_type)
+        self.assertEqual(key.key_size, primary_key_size)
+        self.assertEqual(ret_key.fingerprint, key.fingerprint)
+        self.assertEqual(list_key.fingerprint, key.fingerprint)
+
+        subs = key.subkeys
+        self.assertEqual(len(subs), 1)
+
+        keyid, sub = subs.popitem()
+        self.assertEqual(sub.key_algorithm, sub_key_type)
+        self.assertEqual(sub.key_size, sub_key_size)
+
+        uids = key.userids
+        self.assertEqual(len(uids), 2)
+        for uid in uids:
+            self.assertEqual(uid.name, self.pgp_list.mlist.display_name)
+            self.assertIn(uid.email,
+                          (self.pgp_list.mlist.posting_address,
+                           self.pgp_list.mlist.request_address))
diff --git a/src/mailman_pgp/pgp/tests/test_pgp.py b/src/mailman_pgp/pgp/tests/test_pgp.py
index ab2a69a..6ef531e 100644
--- a/src/mailman_pgp/pgp/tests/test_pgp.py
+++ b/src/mailman_pgp/pgp/tests/test_pgp.py
@@ -25,6 +25,7 @@ from mailman_pgp.config import config
 from mailman_pgp.database import mm_transaction, transaction
 from mailman_pgp.model.address import PGPAddress
 from mailman_pgp.model.list import PGPMailingList
+from mailman_pgp.pgp.keygen import ListKeyGenerator
 from mailman_pgp.testing.layers import PGPConfigLayer
 from mailman_pgp.testing.pgp import load_key
 
@@ -37,7 +38,7 @@ class TestPGP(TestCase):
             self.mlist = create_list('test@example.com',
                                      style_name='pgp-default')
         self.pgp_list = PGPMailingList.for_list(self.mlist)
-        self.list_key = self.pgp_list.generate_key(True)
+        self.list_key = ListKeyGenerator(self.pgp_list).generate(True)
 
         # Make Anne a member of this mailing list.
         self.anne = subscribe(self.mlist, 'Anne', email='anne@example.org')
@@ -61,3 +62,7 @@ class TestPGP(TestCase):
         with keyring.key(self.anne_key.fingerprint) as key:
             self.assertTrue(key.is_public)
             self.assertEqual(key.fingerprint, self.anne_key.fingerprint)
+
+    def test_archive_keydir(self):
+        keyring = config.pgp.archive_keyring
+        self.assertEqual(len(keyring), 0)
diff --git a/src/mailman_pgp/styles/base.py b/src/mailman_pgp/styles/base.py
index a7c3366..bad67ad 100644
--- a/src/mailman_pgp/styles/base.py
+++ b/src/mailman_pgp/styles/base.py
@@ -22,6 +22,7 @@ from public import public
 from mailman_pgp.config import config, mm_config
 from mailman_pgp.database import transaction
 from mailman_pgp.model.list import PGPMailingList
+from mailman_pgp.pgp.keygen import ListKeyGenerator
 
 
 @public
@@ -46,5 +47,6 @@ class PGPStyle:
         with transaction() as session:
             pgp_list = PGPMailingList(mailing_list)
             if generate:
-                pgp_list.generate_key()
+                keygen = ListKeyGenerator(pgp_list)
+                keygen.generate(True)
             session.add(pgp_list)
diff --git a/src/mailman_pgp/utils/file.py b/src/mailman_pgp/utils/file.py
new file mode 100644
index 0000000..d633c4a
--- /dev/null
+++ b/src/mailman_pgp/utils/file.py
@@ -0,0 +1,62 @@
+# Copyright (C) 2017 Jan Jancar
+#
+# This file is a part of the Mailman PGP plugin.
+#
+# This program 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.
+#
+# This program 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
+# this program.  If not, see <http://www.gnu.org/licenses/>.
+
+""""""
+from functools import wraps
+
+from flufl.lock import Lock
+
+
+def locked(lockfile, *lock_args, **lock_kwargs):
+    """
+
+    :param lockfile:
+    :param lock_args:
+    :param lock_kwargs:
+    :return:
+    """
+
+    def locked_decorator(func):
+        @wraps(func)
+        def wrapper(*args, **kwargs):
+            with Lock(lockfile, *lock_args, **lock_kwargs):
+                return func(*args, **kwargs)
+
+        return wrapper
+
+    return locked_decorator
+
+
+def locked_obj(lockattr, *lock_args, **lock_kwargs):
+    """
+
+    :param lockattr:
+    :param lock_args:
+    :param lock_kwargs:
+    :return:
+    """
+
+    def locked_decorator(func):
+        @wraps(func)
+        def wrapper(*args, **kwargs):
+            lockfile = getattr(args[0], lockattr)
+            locked_func = locked(lockfile, *lock_args, **lock_kwargs)(func)
+            return locked_func(*args, **kwargs)
+
+        return wrapper
+
+    return locked_decorator
-- 
cgit v1.2.3-70-g09d2