This patch adds code to securely store the ccache in an encrypted
session on the server (as previously discussed).  There are two things
we still need to decide:

  1. Where we store the Sqlite session table.  This can't be a random
temporary file because all the Apache processes must use the same file,
must know in advance.  There's no reason to write anything to a physical
disk, so I recommend we use something like /dev/shm/ipa-session.sqlite.
Whatever the default value, we'll make it configurable.

  2. Where we write the temporary clear-text ccache when resuming the
session.  We need to write the ccache to a temporary file just long
enough to bind to LDAP, after which we'll delete it.  This needs to be a
securely created, randomly named temporary file.  Using /tmp/ is fine,
but as with the session table, there's no reason to write anything to a
physical disk, so for performance it's far better if we write the ccache
to a memory-based file system like /dev/shm/.  I think creating a
directory like /dev/shm/ipa-tmp/ would be good, then we can write all
the ccache temp files in there.

There are a few features I left out of the Session class for now to make
it easier to review the basic implementation (and because these features
are trivial to add).  But on this list are:

  1. We should probably store the IP address that a session is started
from and only allow the session to be resumed from the same IP address.
My implementation certainly doesn't require this to be secure, but it
provides a bit of extra assurance that some may want (to the extend that
the IP address can be trusted, for which the prudent assumption is "not
at all").

  2. We should probably store the principal (extracted from the ccache)
in the session table.  My main reason for this is debugging, just so
it's easy to see what sessions exist and who has them.

I'm currently using python-crypto to do the encryption, but this might
change in the future.  Does anyone have an opinion as to whether NSS is
appropriate for doing purely symmetric crypto?  John mentioned that he
can add symmetric crypto to the python-nss bindings.

I'm also using SQLAlchemy to interact with Sqlite.  SQLAlchemy provides
solid protection against SQL injection (and is just a cool library), but
if people have a problem with this additional dependency, I can rework
it to run raw statements against the sqlite3 DB-API interface.

If anyone is missing these packages, just yum install python-crypto
python-sqlalchemy.  You may also need the python-sqlite package if
running under Pyhon2.4.
>From f71c04d46da06dbcf312585fa81fc7cbff201498 Mon Sep 17 00:00:00 2001
From: Jason Gerard DeRose <jder...@redhat.com>
Date: Tue, 28 Jul 2009 12:59:41 -0600
Subject: [PATCH] Add secure session storage

---
 ipaserver/session.py                 |  265 ++++++++++++++++++++++++++++++++++
 tests/test_ipaserver/test_session.py |  232 +++++++++++++++++++++++++++++
 2 files changed, 497 insertions(+), 0 deletions(-)
 create mode 100644 ipaserver/session.py
 create mode 100644 tests/test_ipaserver/test_session.py

diff --git a/ipaserver/session.py b/ipaserver/session.py
new file mode 100644
index 0000000..d05cc39
--- /dev/null
+++ b/ipaserver/session.py
@@ -0,0 +1,265 @@
+# Authors:
+#   Jason Gerard DeRose <jder...@redhat.com>
+#
+# Copyright (C) 2008  Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# 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; version 2 only
+#
+# 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, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
+"""
+Securely store the ccache in an encrypted session.
+
+On the one hand, we would like to avoid doing modauthkerb negotiation at each
+request because it's rather slow.  On the other hand, storing the ccache in
+a conventional cookie-based session leaves these sensitive credentials lingering
+on the server: if Apache or IPA becomes compromised, an attacker could use the
+ccache(s) to do substantial harm.
+
+The `Session` class stores the ccache encrypted on the server in a way that
+minimizes its exposure (in fact, it's no more exposed than when doing
+modauthkerb negotiation at each request).  The scheme works as follows:
+
+  1. When a session is created, a random 256-bit session-key is generated
+
+  2. The ccache is encrypted to this session-key (using AES-256), and the
+     resulting encrypted-ccache is what's stored in the Sqlite session table on
+     the server
+
+  3. A SHA-256 hash of the session-key is used to index the session (this hash
+     is the primary-key of the session table)
+
+  4. The session-key is passed to the client (browser) as a cookie, which the
+     client then sends back with each subsequent request
+
+The important thing about the above is that the session is storing neither the
+ccache (in clear-text) nor the session-key needed to decrypt the
+encrypted-ccache.  When subsequent requests are received, the clear-text ccache
+is retrieved using the session-key provided by the client, and this clear-text
+ccache is kept around only long enough to bind to LDAP.
+
+For some background, see this thread:
+
+  https://www.redhat.com/archives/freeipa-devel/2009-July/msg00114.html
+"""
+
+import os
+from base64 import b16encode, b16decode
+try:
+    from hashlib import sha256
+except ImportError:
+    from Crypto.Hash.SHA256 import new as sha256
+from Crypto.Cipher import AES
+from sqlalchemy import create_engine, MetaData, select
+from sqlalchemy import Table, Column, BLOB
+
+
+class SessionError(StandardError):
+    """
+    Exception raised when a session cannot be resumed (or does not exist).
+    """
+    pass
+
+
+class Session(object):
+    """
+    Securely store the ccache in an encrypted session.
+
+    For a single-process server, the default ``dburi`` of
+    ``'sqlite:///:memory:'`` should be used.  For example:
+
+    >>> Session()
+    Session(dburi='sqlite:///:memory:')
+
+    For a multi-process server, all processes must share the same session
+    database, so a Sqlite file dburi needs to be used.  As there's no reason to
+    actually write the session table to a physical disk, a memory-based file
+    system would be best, something like ``'sqlite:///dev/shm/ipa.sqlite'``.
+
+    Use `Session.create()` to create a new session, `Session.resume()` to
+    decrypt the ccache and resume a session, and `Session.destroy()` to remove
+    a session from the session table (likely the target of a *Logout* button
+    in the UI).
+    """
+
+    def __init__(self, dburi='sqlite:///:memory:'):
+        """
+        Initialize the `Session` instance.
+
+        :param dburi: The SQLAlchemy URI for the session database
+        """
+        self.dburi = dburi
+        self.engine = create_engine(dburi)
+        self.metadata = MetaData()
+        # FIXME: I kept the initial implementation minimal, but we may want to
+        # additionally store:
+        #
+        #   * The principal (extracted from the ccache)
+        #
+        #   * The client IP address
+        #
+        #   * A hash of the ccache to make sure a ccache isn't in the session
+        #     table more than once
+        #
+        # We probably also want to only resume sessions from the same IP address
+        # as they were created.
+        self.table = Table('session', self.metadata,
+            Column('id', BLOB, primary_key=True),
+            Column('encrypted_ccache', BLOB),
+        )
+        self.metadata.create_all(self.engine)
+        self.insert = self.table.insert()
+
+    def __repr__(self):
+        return '%s(dburi=%r)' % (self.__class__.__name__, self.dburi)
+
+    def create(self, ccache):
+        """
+        Create a session in which to securely store ``ccache``.
+
+        Note that the ``ccache`` argument is an ``str`` instance containing the
+        *contents* of the credential cache, not its path on the file system.
+
+        To create a session, the following steps are taken:
+
+          1. A random 256-bit session key is created
+
+          2. The ccache is encrypted to the session-key using AES-256
+
+          3. The encrypted-ccache is stored in the session table using the
+             SHA-256 hash of the session-key as the row-ID (the primary-key)
+
+        This method returns the session-key in base16 encoding, which should be
+        returned to the client as a cookie.
+
+        :param ccache: The contents of the credential cache (``str``)
+        :return: A new random 256-bit base16-encoded session key (``str``)
+        """
+        key = os.urandom(32)
+        conn = self.engine.connect()
+        conn.execute(self.table.insert(),
+            id=sha256(key).digest(),
+            encrypted_ccache=self._encrypt(key, ccache),
+        )
+        return b16encode(key)
+
+    def resume(self, b16key):
+        """
+        Resume the session encrypted with ``b16key``.
+
+        The following steps are taken when resuming a session:
+
+          1. The base16 encoded session-key is decoded
+
+          2. The correct session is retrieved using the SHA-256 hash of the
+             session-key
+
+          3. If the session exits, the encrypted-ccache is decrypted using
+             the session-key
+
+        If the session can be successfully resumed, the clear-text contents of
+        the ccache is returned in an ``str`` instance.  Otherwise,
+        `SessionError` is raised.
+
+        :param b16key: The base16-encoded session key (``str``)
+        :return: The decrypted credential cache contents (``str``)
+        """
+        if type(b16key) is not str:
+            raise SessionError("b16key is not <type 'str'>")
+        if len(b16key) != 64:
+            raise SessionError('len(b16key) != 64')
+        try:
+            key = b16decode(b16key)
+        except TypeError:
+            raise SessionError('b16key is invalid base16')
+        query = select(
+            [self.table.c.encrypted_ccache],
+            self.table.c.id == sha256(key).digest(),
+        )
+        conn = self.engine.connect()
+        encrypted_ccache = conn.execute(query).scalar()
+        if encrypted_ccache is None:
+            raise SessionError('no such session')
+        return self._decrypt(key, str(encrypted_ccache))
+
+    def destroy(self, b16key):
+        """
+        Destroy the session encrypted with ``b16key``.
+
+        If there's a session corresponding to ``b16key``, it will be deleted
+        from the session table.  If no such session exists, no action is taken.
+        Either way, there is no return value.
+
+        If ``b16key`` is the incorrect length or is not valid base16 encoding,
+        `SessionError` is raised.
+
+        :param b16key: The base16-encoded session key (``str``)
+        """
+        if type(b16key) is not str:
+            raise SessionError("b16key is not <type 'str'>")
+        if len(b16key) != 64:
+            raise SessionError('len(b16key) != 64')
+        try:
+            key = b16decode(b16key)
+        except TypeError:
+            raise SessionError('b16key is invalid base16')
+        conn = self.engine.connect()
+        conn.execute(
+            self.table.delete(self.table.c.id == sha256(key).digest())
+        )
+
+    def _encrypt(self, key, ccache):
+        """
+        Encrypt ``ccache`` using ``key``.
+
+        :param key: The 256-bit (32 byte) encryption key (``str``)
+        :param ccache: The credential cache (``str``)
+        :return: The encrypted credential cache (``str``)
+        """
+        if type(ccache) is not str:
+            raise SessionError("ccache is not <type 'str'>")
+        if len(ccache) == 0:
+            raise SessionError('len(ccache) == 0')
+        return self._cipher(key).encrypt(ccache)
+
+    def _decrypt(self, key, encrypted_ccache):
+        """
+        Decrypt `encrypted_ccache` using `key`.
+
+        :param key: The 256-bit (32 byte) encryption key (``str``)
+        :param encrypted_ccache: The encrypted credential cache (``str``)
+        :return: The decrypted credential cache (``str``)
+        """
+        if type(encrypted_ccache) is not str:
+            raise SessionError("encrypted_ccache is not <type 'str'>")
+        if len(encrypted_ccache) == 0:
+            raise SessionError('len(encrypted_ccache) == 0')
+        return self._cipher(key).decrypt(encrypted_ccache)
+
+    def _cipher(self, key):
+        """
+        Return a symmetric cipher for encrypting and decrypting with ``key``.
+
+        This helper method creates the AES cipher used by `Session.encrypt()`
+        and `Session.decrypt()`.
+
+        :param key: The 256-bit (32 byte) encryption key (``str``)
+        :return: A ``Crypto.Cipher.AES`` symetric cipher
+        """
+        if type(key) is not str or len(key) != 32:
+            raise SessionError()
+        # As we use this key only to encrypt this ccache, we can safely use the
+        # first half of the key as an initialization vector.  Cryptographic
+        # weaknesses only arise from using the same key and the same IV to
+        # encrypt *multiple* messages.
+        return AES.new(key, AES.MODE_CFB, key[:16])
diff --git a/tests/test_ipaserver/test_session.py b/tests/test_ipaserver/test_session.py
new file mode 100644
index 0000000..02500d8
--- /dev/null
+++ b/tests/test_ipaserver/test_session.py
@@ -0,0 +1,232 @@
+# Authors:
+#   Jason Gerard DeRose <jder...@redhat.com>
+#
+# Copyright (C) 2008  Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# 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; version 2 only
+#
+# 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, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
+"""
+Test the `ipaserver.session` module.
+"""
+
+import os
+from base64 import b16encode, b16decode
+try:
+    from hashlib import sha256
+except ImportError:
+    from Crypto.Hash.SHA256 import new as sha256
+from Crypto.Cipher import AES
+from sqlalchemy import select
+from tests.util import raises, TempDir, assert_equal
+from ipaserver.session import Session, SessionError
+
+
+class test_Session(object):
+    klass = Session
+
+    def key_ccache_cipher(self):
+        key = os.urandom(32)
+        ccache = os.urandom(451)
+        cipher = self.cipher(key)
+        return (key, ccache, cipher)
+
+    def cipher(self, key):
+        """
+        Helper function to return an AES cipher instance.
+        """
+        return AES.new(key, AES.MODE_CFB, key[:16])
+
+    def test_create(self):
+        """
+        Test the `ipaserver.session.Session.create` method.
+        """
+        inst = self.klass()
+        ccache = os.urandom(529)
+
+        b16key = inst.create(ccache)
+        assert type(b16key) is str
+        assert len(b16key) == 64
+        key = b16decode(b16key)
+        conn = inst.engine.connect()
+        query = select(
+            [inst.table],
+            inst.table.c.id == sha256(key).digest(),
+        )
+        row = conn.execute(query).fetchone()
+        len(row) == 2
+        assert str(row['encrypted_ccache']) == self.cipher(key).encrypt(ccache)
+
+        # Check that ccache must be str type:
+        e = raises(SessionError, inst.create, u'Not a ccache!')
+        assert str(e) == "ccache is not <type 'str'>"
+
+        # Check that ccache cannot be empty
+        e = raises(SessionError, inst.create, '')
+        assert str(e) == 'len(ccache) == 0'
+
+    def test_resume(self):
+        """
+        Test the `ipaserver.session.Session.resume` method.
+        """
+        inst = self.klass()
+        ccache1 = os.urandom(529)
+        ccache2 = os.urandom(600)
+
+        # Test creating and resuming a session:
+        session1 = inst.create(ccache1)
+        assert inst.resume(session1) == ccache1
+
+        # Test restoring a non-existent session:
+        e = raises(SessionError, inst.resume, b16encode(os.urandom(32)))
+        assert str(e) == 'no such session'
+
+        # Create another session, test that both are still available:
+        session2 = inst.create(ccache2)
+        assert session1 != session2
+        assert inst.resume(session1) == ccache1
+        assert inst.resume(session2) == ccache2
+
+        # Test with wrong b16key type:
+        b16key = ('A' * 64).decode('ascii')
+        e = raises(SessionError, inst.resume, b16key)
+        assert str(e) == "b16key is not <type 'str'>"
+
+        # Test with incorrect length:
+        for l in (16, 32, 63, 65, 128):
+            b16key = 'A' * l
+            e = raises(SessionError, inst.resume, b16key)
+            assert str(e) == 'len(b16key) != 64'
+
+        # Test with non-base16 characters
+        b16keys = [
+            os.urandom(64),
+            'a' * 64,
+            'G' * 64,
+        ]
+        for b16key in b16keys:
+            e = raises(SessionError, inst.resume, b16key)
+            assert str(e) == 'b16key is invalid base16'
+
+    def test_destroy(self):
+        """
+        Test the `ipaserver.session.Session.destroy` method.
+        """
+        inst = self.klass()
+        ccache1 = os.urandom(529)
+        ccache2 = os.urandom(600)
+
+        # Test destroying a non-existent session:
+        assert inst.destroy(b16encode(os.urandom(32))) is None
+
+        # Create a few sessions:
+        session1 = inst.create(ccache1)
+        session2 = inst.create(ccache2)
+        assert inst.resume(session1) == ccache1
+        assert inst.resume(session2) == ccache2
+
+        # Destroy one session:
+        assert inst.destroy(session1) is None
+        assert inst.resume(session2) == ccache2
+        e = raises(SessionError, inst.resume, session1)
+        assert str(e) == 'no such session'
+
+        # Destroy the other session:
+        assert inst.destroy(session2) is None
+        e = raises(SessionError, inst.resume, session1)
+        assert str(e) == 'no such session'
+        e = raises(SessionError, inst.resume, session2)
+        assert str(e) == 'no such session'
+
+        # Test with wrong b16key type:
+        b16key = ('A' * 64).decode('ascii')
+        e = raises(SessionError, inst.destroy, b16key)
+        assert str(e) == "b16key is not <type 'str'>"
+
+        # Test with incorrect length:
+        for l in (16, 32, 63, 65, 128):
+            b16key = 'A' * l
+            e = raises(SessionError, inst.destroy, b16key)
+            assert str(e) == 'len(b16key) != 64'
+
+        # Test with non-base16 characters
+        b16keys = [
+            os.urandom(64),
+            'a' * 64,
+            'G' * 64,
+        ]
+        for b16key in b16keys:
+            e = raises(SessionError, inst.destroy, b16key)
+            assert str(e) == 'b16key is invalid base16'
+
+    def test_encrypt(self):
+        """
+        Test the `ipaserver.session.Session._encrypt` method.
+        """
+        (key, ccache, cipher) = self.key_ccache_cipher()
+        inst = self.klass()
+        encrypted_ccache = inst._encrypt(key, ccache)
+        assert encrypted_ccache != ccache
+        assert encrypted_ccache == cipher.encrypt(ccache)
+
+        # Check that ccache must be str type:
+        e = raises(SessionError, inst._encrypt, key, u'Not a ccache!')
+        assert str(e) == "ccache is not <type 'str'>"
+
+        # Check that ccache cannot be empty
+        e = raises(SessionError, inst._encrypt, key, '')
+        assert str(e) == 'len(ccache) == 0'
+
+    def test_decrypt(self):
+        """
+        Test the `ipaserver.session.Session._decrypt` method.
+        """
+        (key, ccache, cipher) = self.key_ccache_cipher()
+        encrypted_ccache = cipher.encrypt(ccache)
+        assert encrypted_ccache != ccache
+        inst = self.klass()
+        assert inst._decrypt(key, encrypted_ccache) == ccache
+
+        # Test the round trip again just using Session:
+        key = os.urandom(32)
+        ccache = os.urandom(29 * 31)
+        assert inst._decrypt(key, inst._encrypt(key, ccache)) == ccache
+
+        # Check that encrypted_ccache must be str type:
+        e = raises(SessionError, inst._decrypt, key, u'Not a ccache!')
+        assert str(e) == "encrypted_ccache is not <type 'str'>"
+
+        # Check that encrypted_ccache cannot be empty
+        e = raises(SessionError, inst._decrypt, key, '')
+        assert str(e) == 'len(encrypted_ccache) == 0'
+
+    def test_cipher(self):
+        """
+        Test the `ipaserver.session.Session._cipher` method.
+        """
+        inst = self.klass()
+
+        # Test with wrong key length:
+        for l in (16, 24, 31, 33):
+            key = os.urandom(l)
+            raises(SessionError, inst._cipher, key)
+
+        # Test with wrong key type:
+        keys = [
+            ('a' * 32).decode('ascii'),
+            list('a' for i in xrange(32)),
+            47,
+        ]
+        for key in keys:
+            raises(SessionError, inst._cipher, key)
-- 
1.6.0.4

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to