[or-cvs] r9568: Implement proposal 106: stop requiring clients to have certi (in tor/trunk: . doc doc/spec/proposals src/common src/or)
nickm at seul.org
nickm at seul.org
Mon Feb 12 21:39:38 UTC 2007
Author: nickm
Date: 2007-02-12 16:39:33 -0500 (Mon, 12 Feb 2007)
New Revision: 9568
Modified:
tor/trunk/
tor/trunk/ChangeLog
tor/trunk/doc/TODO
tor/trunk/doc/spec/proposals/000-index.txt
tor/trunk/doc/spec/proposals/106-less-tls-constraint.txt
tor/trunk/src/common/tortls.c
tor/trunk/src/or/circuitbuild.c
tor/trunk/src/or/connection_or.c
tor/trunk/src/or/or.h
Log:
r11773 at catbus: nickm | 2007-02-12 15:18:48 -0500
Implement proposal 106: stop requiring clients to have certificates, and stop checking for nicknames in certificates. [See proposal 106 for rationale.] Also improve messages when checking TLS handshake, to re-resolve bug 382.
Property changes on: tor/trunk
___________________________________________________________________
svk:merge ticket from /tor/trunk [r11773] on 8246c3cf-6607-4228-993b-4d95d33730f1
Modified: tor/trunk/ChangeLog
===================================================================
--- tor/trunk/ChangeLog 2007-02-12 19:56:07 UTC (rev 9567)
+++ tor/trunk/ChangeLog 2007-02-12 21:39:33 UTC (rev 9568)
@@ -33,6 +33,14 @@
advance warning.
- Remove some never-implemented options. Mark PathlenCoinWeight as
obsolete.
+ - Implement proposal 106: Stop requiring clients to have well-formed
+ certificates; stop checking nicknames in certificates. (Clients have
+ certificates so that they can look like Tor servers, but in the future
+ we might want to allow them to look like regular TLS clients instead.
+ Nicknames in certificates serve no purpose other than making our
+ protocol easier to recognize on the wire.)
+ - Revise messages on handshake failure again to be even more clear about
+ which are incoming connections and which are outgoing.
Changes in version 0.1.2.7-alpha - 2007-02-06
Modified: tor/trunk/doc/TODO
===================================================================
--- tor/trunk/doc/TODO 2007-02-12 19:56:07 UTC (rev 9567)
+++ tor/trunk/doc/TODO 2007-02-12 21:39:33 UTC (rev 9568)
@@ -111,12 +111,14 @@
- recommend gaim.
- unrecommend IE because of ftp:// bug.
N - we should add a preamble to tor-design saying it's out of date.
-N - Document transport and natdport
+N . Document transport and natdport
+ o In man page
+ - In a good HOWTO.
- - Forward compatibility fixes
- - Caches should start trying to cache consensus docs?
-NR - Design
-N - Implement, if we think it's smart.
+ . Forward compatibility fixes
+ D Caches should start trying to cache consensus docs?
+ D Design
+ D Implement, if we think it's smart.
- Start uploading short and long descriptors; authorities should support
URLs to retrieve long descriptors, and should discard short descriptors
for now. Later, once tools use the "long descriptor" URLs, authorities
@@ -124,9 +126,11 @@
a descriptor.
NR - Design
N - Implement, if we think it's smart.
- - Check for any outstanding checks we do on the form or number of client
+ o Check for any outstanding checks we do on the form or number of client
certificates that would prevent us from executing certain
blocking-resistance strategies.
+ o Design (proposal 106)
+ o Implement
Things we'd like to do in 0.2.0:
- Proposals:
Modified: tor/trunk/doc/spec/proposals/000-index.txt
===================================================================
--- tor/trunk/doc/spec/proposals/000-index.txt 2007-02-12 19:56:07 UTC (rev 9567)
+++ tor/trunk/doc/spec/proposals/000-index.txt 2007-02-12 21:39:33 UTC (rev 9568)
@@ -24,5 +24,5 @@
103 Splitting identity key from regularly used signing key [OPEN]
104 Long and Short Router Descriptors [OPEN]
105 Version negotiation for the Tor protocol [OPEN]
-106 Checking fewer things during TLS handshakes [OPEN]
+106 Checking fewer things during TLS handshakes [FINISHED]
Modified: tor/trunk/doc/spec/proposals/106-less-tls-constraint.txt
===================================================================
--- tor/trunk/doc/spec/proposals/106-less-tls-constraint.txt 2007-02-12 19:56:07 UTC (rev 9567)
+++ tor/trunk/doc/spec/proposals/106-less-tls-constraint.txt 2007-02-12 21:39:33 UTC (rev 9568)
@@ -4,7 +4,7 @@
Last-Modified: $Date: 2007-01-30T07:50:01.643717Z $
Author: Nick Mathewson
Created:
-Status: Accepted
+Status: Finished
Overview:
Modified: tor/trunk/src/common/tortls.c
===================================================================
--- tor/trunk/src/common/tortls.c 2007-02-12 19:56:07 UTC (rev 9567)
+++ tor/trunk/src/common/tortls.c 2007-02-12 21:39:33 UTC (rev 9568)
@@ -672,56 +672,6 @@
return 1;
}
-/** Write the nickname (if any) that the peer connected on <b>tls</b>
- * claims to have into the first <b>buflen</b> characters of <b>buf</b>.
- * Truncate the nickname if it is longer than buflen-1 characters. Always
- * NUL-terminate. Return 0 on success, -1 on failure.
- */
-int
-tor_tls_get_peer_cert_nickname(int severity, tor_tls_t *tls,
- char *buf, size_t buflen)
-{
- X509 *cert = NULL;
- X509_NAME *name = NULL;
- int nid;
- int lenout;
- int r = -1;
-
- if (!(cert = SSL_get_peer_certificate(tls->ssl))) {
- log_fn(severity, LD_PROTOCOL, "Peer has no certificate");
- goto error;
- }
- if (!(name = X509_get_subject_name(cert))) {
- log_fn(severity, LD_PROTOCOL, "Peer certificate has no subject name");
- goto error;
- }
- if ((nid = OBJ_txt2nid("commonName")) == NID_undef)
- goto error;
-
- lenout = X509_NAME_get_text_by_NID(name, nid, buf, buflen);
- if (lenout == -1)
- goto error;
- if (((int)strspn(buf, LEGAL_NICKNAME_CHARACTERS)) < lenout) {
- log_fn(severity, LD_PROTOCOL,
- "Peer certificate nickname %s has illegal characters.",
- escaped(buf));
- if (strchr(buf, '.'))
- log_fn(severity, LD_PROTOCOL,
- " (Maybe it is not really running Tor at its "
- "advertised OR port.)");
- goto error;
- }
-
- r = 0;
-
- error:
- if (cert)
- X509_free(cert);
-
- tls_log_errors(severity, "getting peer certificate nickname");
- return r;
-}
-
/** DOCDOC */
static void
log_cert_lifetime(X509 *cert, const char *problem)
Modified: tor/trunk/src/or/circuitbuild.c
===================================================================
--- tor/trunk/src/or/circuitbuild.c 2007-02-12 19:56:07 UTC (rev 9567)
+++ tor/trunk/src/or/circuitbuild.c 2007-02-12 21:39:33 UTC (rev 9568)
@@ -70,6 +70,11 @@
uint16_t high_bit;
tor_assert(conn);
+ if (conn->circ_id_type == CIRC_ID_TYPE_NEITHER) {
+ log_warn(LD_BUG, "Bug: Trying to pick a circuit ID for a connection from "
+ "a client with no identity.");
+ return 0;
+ }
high_bit = (conn->circ_id_type == CIRC_ID_TYPE_HIGHER) ? 1<<15 : 0;
do {
/* Sequentially iterate over test_circ_id=1...1<<15-1 until we find a
Modified: tor/trunk/src/or/connection_or.c
===================================================================
--- tor/trunk/src/or/connection_or.c 2007-02-12 19:56:07 UTC (rev 9567)
+++ tor/trunk/src/or/connection_or.c 2007-02-12 21:39:33 UTC (rev 9568)
@@ -26,7 +26,7 @@
static digestmap_t *orconn_identity_map = NULL;
/** If conn is listed in orconn_identity_map, remove it, and clear
- * conn->identity_digest. */
+ * conn->identity_digest. Otherwise do nothing. */
void
connection_or_remove_from_identity_map(or_connection_t *conn)
{
@@ -35,8 +35,13 @@
if (!orconn_identity_map)
return;
tmp = digestmap_get(orconn_identity_map, conn->identity_digest);
- if (!tmp)
+ if (!tmp) {
+ if (!tor_digest_is_zero(conn->identity_digest)) {
+ log_warn(LD_BUG, "Didn't found connection on identity map when trying "
+ "to remove it.");
+ }
return;
+ }
if (conn == tmp) {
if (conn->next_with_same_id)
digestmap_set(orconn_identity_map, conn->identity_digest,
@@ -81,7 +86,7 @@
}
/** Change conn->identity_digest to digest, and add conn into
- * orconn_digest_map. */
+ * orconn_digest_map. */
static void
connection_or_set_identity_digest(or_connection_t *conn, const char *digest)
{
@@ -93,14 +98,22 @@
orconn_identity_map = digestmap_new();
if (!memcmp(conn->identity_digest, digest, DIGEST_LEN))
return;
- if (tor_digest_is_zero(conn->identity_digest))
+
+ /* If the identity was set previously, remove the old mapping. */
+ if (! tor_digest_is_zero(conn->identity_digest))
connection_or_remove_from_identity_map(conn);
memcpy(conn->identity_digest, digest, DIGEST_LEN);
+
+ /* If we're setting the ID to zero, don't add a mapping.*/
+ if (tor_digest_is_zero(digest))
+ return;
+
tmp = digestmap_set(orconn_identity_map, digest, conn);
conn->next_with_same_id = tmp;
-#if 0
+#if 1
+ /*XXXX012 change this back to if 0. */
/* Testing code to check for bugs in representation. */
for (; tmp; tmp = tmp->next_with_same_id) {
tor_assert(!memcmp(tmp->identity_digest, digest, DIGEST_LEN));
@@ -551,17 +564,22 @@
return !tor_tls_is_server(conn->tls);
}
-/** Conn just completed its handshake. Return 0 if all is well, and
+/** <b>Conn</b> just completed its handshake. Return 0 if all is well, and
* return -1 if he is lying, broken, or otherwise something is wrong.
*
- * Make sure he sent a correctly formed certificate. If it has a
- * recognized ("named") nickname, make sure his identity key matches
- * it. If I initiated the connection, make sure it's the right guy.
+ * If we initiated this connection (<b>started_here</b> is true), make sure
+ * the other side sent sent a correctly formed certificate. If I initiated the
+ * connection, make sure it's the right guy.
*
- * If we return 0, write a hash of the identity key into digest_rcvd,
- * which must have DIGEST_LEN space in it. (If we return -1 this
- * buffer is undefined.)
+ * Otherwise (if we _didn't_ initiate this connection), it's okay for
+ * the certificate to be weird or absent.
*
+ * If we return 0, and the certificate is as expected, write a hash of the
+ * identity key into digest_rcvd, which must have DIGEST_LEN space in it. (If
+ * we return -1 this buffer is undefined.) If the certificate is invalid
+ * or missing on an incoming connection, we return 0 and set digest_rcvd to
+ * DIGEST_LEN 0 bytes.
+ *
* As side effects,
* 1) Set conn->circ_id_type according to tor-spec.txt.
* 2) If we're an authdirserver and we initiated the connection: drop all
@@ -569,66 +587,68 @@
* this guy; and note that this guy is reachable.
*/
static int
-connection_or_check_valid_handshake(or_connection_t *conn, char *digest_rcvd)
+connection_or_check_valid_handshake(or_connection_t *conn, int started_here,
+ char *digest_rcvd)
{
- routerinfo_t *router;
crypto_pk_env_t *identity_rcvd=NULL;
char nickname[MAX_NICKNAME_LEN+1];
or_options_t *options = get_options();
int severity = server_mode(options) ? LOG_PROTOCOL_WARN : LOG_WARN;
- int started_here = connection_or_nonopen_was_started_here(conn);
const char *safe_address =
started_here ? conn->_base.address : safe_str(conn->_base.address);
- const char *peer_type = started_here ? "Router" : "Client or router";
+ const char *conn_type = started_here ? "outgoing" : "incoming";
+ int has_cert = 0, has_identity = 0;
check_no_tls_errors();
- if (! tor_tls_peer_has_cert(conn->tls)) {
- log_info(LD_PROTOCOL,"%s (%s:%d) didn't send a cert! Closing.",
- peer_type, safe_address, conn->_base.port);
+ has_cert = tor_tls_peer_has_cert(conn->tls);
+ if (started_here && !has_cert) {
+ log_info(LD_PROTOCOL,"Tried connecting to router at %s:%d, but it didn't "
+ "send a cert! Closing.",
+ safe_address, conn->_base.port);
return -1;
+ } else if (!has_cert) {
+ log_debug(LD_PROTOCOL,"Got incoming connection with no certificate. "
+ "That's ok.");
}
check_no_tls_errors();
- if (tor_tls_get_peer_cert_nickname(severity, conn->tls, nickname,
- sizeof(nickname))) {
- log_fn(severity,LD_PROTOCOL,"%s (%s:%d) has a cert without a "
- "valid nickname. Closing.",
- peer_type, safe_address, conn->_base.port);
- return -1;
- }
- check_no_tls_errors();
- log_debug(LD_OR, "%s (%s:%d) claims to be router '%s'",
- peer_type, safe_address, conn->_base.port, nickname);
- if (tor_tls_verify(severity, conn->tls, &identity_rcvd) < 0) {
- log_fn(severity,LD_OR,"%s which claims to be router '%s' (%s:%d),"
- " has a cert but it's invalid. Closing.",
- peer_type, nickname, safe_address, conn->_base.port);
- return -1;
+ if (has_cert) {
+ int v = tor_tls_verify(started_here?severity:LOG_INFO,
+ conn->tls, &identity_rcvd);
+ if (started_here && v<0) {
+ log_fn(severity,LD_OR,"Tried connecting to router at %s:%d: It"
+ " has a cert but it's invalid. Closing.",
+ safe_address, conn->_base.port);
+ return -1;
+ } else if (v<0) {
+ log_info(LD_PROTOCOL,"Incoming connection gave us an invalid cert "
+ "chain; ignoring.");
+ } else {
+ log_debug(LD_OR,"The certificate seems to be valid on %s connection "
+ "with %s:%d", conn_type, safe_address, conn->_base.port);
+ }
+ check_no_tls_errors();
}
- check_no_tls_errors();
- log_debug(LD_OR,"The router's cert is valid.");
- crypto_pk_get_digest(identity_rcvd, digest_rcvd);
- if (crypto_pk_cmp_keys(get_identity_key(), identity_rcvd)<0) {
- conn->circ_id_type = CIRC_ID_TYPE_LOWER;
+ if (identity_rcvd) {
+ has_identity=1;
+ crypto_pk_get_digest(identity_rcvd, digest_rcvd);
+
+ if (crypto_pk_cmp_keys(get_identity_key(), identity_rcvd)<0) {
+ conn->circ_id_type = CIRC_ID_TYPE_LOWER;
+ } else {
+ conn->circ_id_type = CIRC_ID_TYPE_HIGHER;
+ }
+ crypto_free_pk_env(identity_rcvd);
} else {
- conn->circ_id_type = CIRC_ID_TYPE_HIGHER;
+ memset(digest_rcvd, 0, DIGEST_LEN);
+ conn->circ_id_type = CIRC_ID_TYPE_NEITHER;
}
- crypto_free_pk_env(identity_rcvd);
- router = router_get_by_nickname(nickname, 0);
- if (router && /* we know this nickname */
- router->is_named && /* make sure it's the right guy */
- memcmp(digest_rcvd, router->cache_info.identity_digest,DIGEST_LEN) !=0) {
- log_fn(severity, LD_OR,
- "Identity key not as expected for peer claiming to be "
- "'%s' (%s:%d)",
- nickname, safe_address, conn->_base.port);
- return -1;
- }
-
if (started_here) {
int as_advertised = 1;
+ tor_assert(has_cert);
+ tor_assert(has_identity);
if (memcmp(digest_rcvd, conn->identity_digest, DIGEST_LEN)) {
/* I was aiming for a particular digest. I didn't get it! */
char seen[HEX_DIGEST_LEN+1];
@@ -637,8 +657,8 @@
base16_encode(expected, sizeof(expected), conn->identity_digest,
DIGEST_LEN);
log_fn(severity, LD_OR,
- "Identity key not as expected for router at %s:%d: wanted %s "
- "but got %s",
+ "Tried connecting to router at %s:%d, but identity key was not "
+ "as expected wanted %s but got %s",
conn->_base.address, conn->_base.port, expected, seen);
entry_guard_register_connect_status(conn->identity_digest,0,time(NULL));
router_set_status(conn->identity_digest, 0);
@@ -677,7 +697,7 @@
int started_here = connection_or_nonopen_was_started_here(conn);
log_debug(LD_OR,"tls handshake done. verifying.");
- if (connection_or_check_valid_handshake(conn, digest_rcvd) < 0)
+ if (connection_or_check_valid_handshake(conn, started_here, digest_rcvd) < 0)
return -1;
if (!started_here) {
Modified: tor/trunk/src/or/or.h
===================================================================
--- tor/trunk/src/or/or.h 2007-02-12 19:56:07 UTC (rev 9567)
+++ tor/trunk/src/or/or.h 2007-02-12 21:39:33 UTC (rev 9568)
@@ -201,7 +201,8 @@
/** DOCDOC */
typedef enum {
CIRC_ID_TYPE_LOWER=0,
- CIRC_ID_TYPE_HIGHER=1
+ CIRC_ID_TYPE_HIGHER=1,
+ CIRC_ID_TYPE_NEITHER=2
} circ_id_type_t;
#define _CONN_TYPE_MIN 3
@@ -772,8 +773,9 @@
typedef struct or_connection_t {
connection_t _base;
- char identity_digest[DIGEST_LEN]; /**< Hash of the public RSA key for
- * the other side's signing key. */
+ /** Hash of the public RSA key for the other side's identity key, or zero if
+ * the other side hasn't shown us a valid identity key. */
+ char identity_digest[DIGEST_LEN];
char *nickname; /**< Nickname of OR on other side (if any). */
tor_tls_t *tls; /**< TLS connection state */
@@ -787,9 +789,6 @@
int read_bucket; /**< When this hits 0, stop receiving. Every second we
* add 'bandwidthrate' to this, capping it at
* bandwidthburst. (OPEN ORs only) */
- circ_id_type_t circ_id_type; /**< When we send CREATE cells along this
- * connection, which half of the space should
- * we use? */
int n_circuits; /**< How many circuits use this connection as p_conn or
* n_conn ? */
struct or_connection_t *next_with_same_id; /**< Next connection with same
@@ -797,6 +796,9 @@
/** Linked list of bridged dirserver connections that can't write until
* this connection's outbuf is less full. */
struct dir_connection_t *blocked_dir_connections;
+ circ_id_type_t circ_id_type:2; /**< When we send CREATE cells along this
+ * connection, which half of the space should
+ * we use? */
uint16_t next_circ_id; /**< Which circ_id do we try to use next on
* this connection? This is always in the
* range 0..1<<15-1. */
More information about the tor-commits
mailing list