diff --git a/server-ca/chocolate.py b/server-ca/chocolate.py index fc651535f..6d14f15a0 100755 --- a/server-ca/chocolate.py +++ b/server-ca/chocolate.py @@ -133,12 +133,29 @@ class session(object): def handlesession(self, m, r): if r.failure.IsInitialized(): return - # TODO: perhaps some code belongs here to enforce rules about which - # combinations of protocol messages can occur together. I think the - # rules are: Client must send either nothing (polling for updates) - # or exactly one of request, failure, or completedchallenge. Client - # may not send proceed, challenge, or success. If the rules are - # violated, we should self.die(r, r.BadRequest) and return. + # Note that m.challenge and m.completedchallenge present + # as lists, which are True if they are nonempty. By + # contrast, m.proceed, m.success, m.request, and m.failure + # are always True but have an .IsInitialized() property + # indicating whether they are actually present in m as + # messages from the client. + # + # Check for some ways in which the message from the client + # can be inappropriate. + if m.challenge or m.proceed.IsInitialized() or m.success.IsInitialized(): + self.die(r, r.BadRequest, uri="https://ca.example.com/failures/invalidfromclient") + return + distinct_messages = 0 + if m.request.IsInitialized(): distinct_messages += 1 + if m.failure.IsInitialized(): distinct_messages += 1 + if m.completedchallenge: distinct_messages += 1 + if distinct_messages > 1: + self.die(r, r.BadRequest, uri="https://ca.example.com/failures/mixedmessages") + return + # The rule that a new session must contain a request is enforced + # by handlenewsession. The rule that an existing session must + # not contain a request is enforced by handleexistingsession. + # TODO: check that there are no bad cases that slip through. if m.session == "": # New session r.session = random() @@ -269,8 +286,7 @@ class session(object): def send_challenges(self, m, r): if r.failure.IsInitialized(): return - # TODO: This needs a more sophisticated notion of success/failure, - # and also of the possibility of multiple data strings. + # TODO: This needs a more sophisticated notion of success/failure. for c in self.challenges(): # Currently, we can only handle challenge type 0 (dvsni) # TODO: unify names "succeeded" vs. "satisfied"? @@ -319,11 +335,7 @@ class session(object): # or similar. # Send reply - if m.debug: - web.header("Content-type", "text/plain") - return "SAW MESSAGE: %s\nRESPONSE: %s\n" % (str(m), str(r)) - else: - return r.SerializeToString() + return r.SerializeToString() def GET(self): web.header("Content-type", "text/html") diff --git a/server-ca/chocolate_protocol.proto b/server-ca/chocolate_protocol.proto index a4d1543f2..2bc75bbd2 100644 --- a/server-ca/chocolate_protocol.proto +++ b/server-ca/chocolate_protocol.proto @@ -106,8 +106,4 @@ message chocolatemessage { /* Sent by SERVER to issue the requested certificate */ optional Success success = 8; - - /* For debugging; should be removed in final protocol. */ - optional bool debug = 9; /* Causes server to return text instead of - message! */ } diff --git a/server-ca/daemon.py b/server-ca/daemon.py index 692072309..5fa03c168 100644 --- a/server-ca/daemon.py +++ b/server-ca/daemon.py @@ -12,12 +12,12 @@ # The queue mechanism with pending-* is supposed to control # concurrency issues properly, but this needs verification # to ensure that there are no possible race conditions. -# Generally, the server is not supposed to change sessions -# very much once they have been added to a queue, except -# for marking them no longer live if the server realizes -# that something bad has happened to them. There may be -# some exceptions, and they should all be analyzed for -# possible races. +# Generally, the server process (as distinct from the daemon) +# is not supposed to change sessions at all once they have +# been added to a queue, except for marking them no longer +# live if the server realizes that something bad has happened +# to them. There may be some exceptions, and they should all +# be analyzed for possible races. # TODO: The daemon should probably check for timeouts before # advancing sessions' state. Currently timeouts can only @@ -102,14 +102,47 @@ def testchallenge(session): # that it has completed the challenges. Information about # the client's reporting could be stored in the database. # Then the CA doesn't need to poll prematurely. - if False: # if challenges all succeed + all_satisfied = True + for i, name in enumerate(r.lrange("%s:names" % session, 0, -1)): + challenge = "%s:%d" % (session, i) + challtime = r.hget(challenge, "challtime") + challtype = r.hget(challenge, "type") + name = r.hget(challenge, "name") + satisfied = r.hget(challenge, "satisfied") == "True" + failed = r.hget(challenge, "failed") == "True" + # TODO: check whether this challenge is too old + if not satisfied and not failed: + if challtype == 0: # DomainValidateSNI + dvsni_nonce = r.hget(challenge, "dvsni:nonce") + dvsni_r = r.hget(challenge, "dvsni:r") + dvsni_ext = r.hget(challenge, "dvsni:ext") + if verify_challenge(name, dvsni_r, dvsni_nonce)[0]: + r.hset(challenge, "satisfied", True) + else: + all_satisfied = False + # TODO: distinguish permanent and temporarily failures + # can cause a permanent failure under some conditions, causing + # the session to become dead. TODO: need to articulate what + # those conditions are + else: + # Don't know how to handle this challenge type + all_satisfied = False + elif not satisfied: + all_satisfied = False + if all_satisfied: + # Challenges all succeeded, so we should prepare to issue + # the requested cert. + # TODO: double-check that there were > 0 challenges, + # so that we don't somehow mistakenly issue a cert in + # response to an empty list of challenges (even though + # the daemon that put this session on the queue should + # also have implicitly guaranteed this). r.hset(session, "state", "issue") r.lpush("pending-issue", session) else: + # Some challenges are not verified. + # Put this session back on the stack to try to verify again. r.lpush("pending-testchallenge", session) - # can also cause a failure under some conditions, causing the - # session to become dead. TODO: need to articulate what those - # conditions are def issue(session): if r.hget(session, "live") != "True": @@ -130,9 +163,6 @@ def issue(session): # should never happen. r.lrem("pending-requests", session) return - # Note that we can push this back into the original queue. - # TODO: need to add a way to make sure we don't test the same - # TODO: actually make this call issue the cert csr = r.hget(session, "csr") cert = CSR.issue(csr) r.hset(session, "cert", cert)