From 88c5b270ef805773f45650f0f37f6578379efd6d Mon Sep 17 00:00:00 2001
From: Seth Schoen <schoen@eff.org>
Date: Sat, 14 Jul 2012 23:01:39 -0700
Subject: [PATCH] implement locking for issuing certs with openssl ca

---
 server-ca/CSR.py    |  9 +++------
 server-ca/daemon.py | 30 +++++++++++++-----------------
 2 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/server-ca/CSR.py b/server-ca/CSR.py
index 9a659b8ba..1963ddf46 100644
--- a/server-ca/CSR.py
+++ b/server-ca/CSR.py
@@ -207,12 +207,9 @@ def encrypt(key, data):
 
 def issue(csr, subjects):
     """Issue a certificate requested by CSR, specifying the subject names
-    indicated in subjects, and return the certificate."""
-    # TODO: The caller should have to acquire a lock in the database to
-    #       prevent two certs from being issued at exactly the same time,
-    #       because openssl ca doesn't handle this case safely.  There
-    #       is a longer comment in daemon.py describing how to implement
-    #       this lock with Redis setnx.
+    indicated in subjects, and return the certificate.  Calls to this
+    function should be guarded with a lock to ensure that the calls never
+    overlapping calls."""
     if not subjects:
         return None
     csr = str(csr)
diff --git a/server-ca/daemon.py b/server-ca/daemon.py
index b91889734..409519a3b 100644
--- a/server-ca/daemon.py
+++ b/server-ca/daemon.py
@@ -38,20 +38,8 @@
 # request, period, while still allowing clients to look
 # up successfully issued certs.
 # TODO: implement multithreading to allow several parallel
-# worker processes.  But note:
-
-#       The ca command is effectively a single user command: no locking
-#       is done on the various files and attempts to run more than one
-#       ca command on the same database can have unpredictable results.
+# worker processes.
 #
-#                             -- ca(1SSL)
-
-# So we need to implement our own locking mechanism.  This
-# can be done easily in Redis with "setnx":
-#            http://redis.io/commands/setnx
-# However apparently the proper recovery after crashes can
-# be complicated.
-
 # NOTE: The daemon enforces its own timeouts, which are
 # defined in the ancient() function.  These timeouts apply
 # to any session that has been placed in a queue and can
@@ -60,12 +48,19 @@
 # the server or the daemon (due to timeout or error) causes
 # a session to be treated as dead by both.
 
-import redis, time, CSR, sys, signal, hashlib
-r = redis.Redis()
-
+import redis, redis_lock, time, CSR, sys, signal, hashlib
 from sni_challenge.verify import verify_challenge
 from Crypto import Random
 
+r = redis.Redis()
+issue_lock = redis_lock(r, "issue_lock")
+# This lock guards the ability to issue certificates with "openssl ca",
+# which has no locking of its own.  We don't need locking for the updates
+# that the daemon performs on the sessions in the database because the
+# queues pending-makechallenge, pending-testchallenge, and pending-issue
+# are updated atomically and the daemon only ever acts on sessions that it
+# has removed from a queue.
+
 debug = "debug" in sys.argv
 clean_shutdown = False
 
@@ -232,7 +227,8 @@ def issue(session):
         return
     csr = r.hget(session, "csr")
     names = r.lrange("%s:names" % session, 0, -1)
-    cert = CSR.issue(csr, names)
+    with issue_lock:
+        cert = CSR.issue(csr, names)
     r.hset(session, "cert", cert)
     if cert:   # once issuing cert succeeded
         if debug: print "issued for", session