0
0
Fork 0
mirror of https://github.com/matrix-construct/construct synced 2024-09-30 04:38:52 +02:00

authd: wait until the ssl connection is "open" before reading

It's useful to allow authd to run in parallel with ssl negotiation,
but if the ssld connection has plaintext data ready for reading
there's a race condition between authd calling read_packet() and
ssl_process_certfp() storing the certificate fingerprint. This
scenario would be bad for a server connecting because fingerprint
verification will fail.

Allow either operation to complete first, but wait until
ssl_process_open_fd() calls the ssl open callback before calling
read_packet().
This commit is contained in:
Simon Arlott 2016-04-25 21:35:58 +01:00
parent 53789fddda
commit 762468f85d
No known key found for this signature in database
GPG key ID: C8975F2043CA5D24
4 changed files with 57 additions and 14 deletions

View file

@ -70,7 +70,8 @@ void restart_authd(void);
void rehash_authd(void);
void check_authd(void);
void authd_initiate_client(struct Client *);
void authd_initiate_client(struct Client *, bool defer);
void authd_deferred_client(struct Client *);
void authd_accept_client(struct Client *client_p, const char *ident, const char *host);
void authd_reject_client(struct Client *client_p, const char *ident, const char *host, char cause, const char *data, const char *reason);
void authd_abort_client(struct Client *);

View file

@ -294,6 +294,9 @@ struct LocalUser
time_t sasl_next_retry;
};
#define AUTHC_F_DEFERRED 0x01
#define AUTHC_F_COMPLETE 0x02
struct AuthClient
{
uint32_t cid; /* authd id */
@ -302,6 +305,7 @@ struct AuthClient
char cause; /* rejection cause */
char *data; /* reason data */
char *reason; /* reason we were rejected */
int flags;
};
struct PreClient

View file

@ -417,9 +417,15 @@ generate_cid(void)
* gonna accept the client and ignore authd's suggestion.
*
* --Elizafox
*
* If this is an SSL connection we must defer handing off the client for
* reading until it is open and we have the certificate fingerprint, otherwise
* it's possible for the client to immediately send data before authd completes
* and before the status of the connection is communicated via ssld. This data
* could then be processed too early by read_packet().
*/
void
authd_initiate_client(struct Client *client_p)
authd_initiate_client(struct Client *client_p, bool defer)
{
char client_ipaddr[HOSTIPLEN+1];
char listen_ipaddr[HOSTIPLEN+1];
@ -442,15 +448,35 @@ authd_initiate_client(struct Client *client_p)
listen_port = ntohs(GET_SS_PORT(&client_p->preClient->lip));
client_port = ntohs(GET_SS_PORT(&client_p->localClient->ip));
if(defer)
client_p->preClient->auth.flags |= AUTHC_F_DEFERRED;
/* Add a bit of a fudge factor... */
client_p->preClient->auth.timeout = rb_current_time() + ConfigFileEntry.connect_timeout + 10;
rb_helper_write(authd_helper, "C %x %s %hu %s %hu", authd_cid, listen_ipaddr, listen_port, client_ipaddr, client_port);
}
static inline void
authd_read_client(struct Client *client_p)
{
/*
* When a client has auth'ed, we want to start reading what it sends
* us. This is what read_packet() does.
* -- adrian
*
* Above comment was originally in s_auth.c, but moved here with below code.
* --Elizafox
*/
rb_dlinkAddTail(client_p, &client_p->node, &global_client_list);
read_packet(client_p->localClient->F, client_p);
}
/* When this is called we have a decision on client acceptance.
*
* After this point authd no longer "owns" the client.
* After this point authd no longer "owns" the client, but if
* it's flagged as deferred then we're still waiting for a call
* to authd_deferred_client().
*/
static inline void
authd_decide_client(struct Client *client_p, const char *ident, const char *host, bool accept, char cause, const char *data, const char *reason)
@ -478,16 +504,17 @@ authd_decide_client(struct Client *client_p, const char *ident, const char *host
client_p->preClient->auth.reason = (reason == NULL ? NULL : rb_strdup(reason));
client_p->preClient->auth.cid = 0;
/*
* When a client has auth'ed, we want to start reading what it sends
* us. This is what read_packet() does.
* -- adrian
*
* Above comment was originally in s_auth.c, but moved here with below code.
* --Elizafox
*/
rb_dlinkAddTail(client_p, &client_p->node, &global_client_list);
read_packet(client_p->localClient->F, client_p);
client_p->preClient->auth.flags |= AUTHC_F_COMPLETE;
if((client_p->preClient->auth.flags & AUTHC_F_DEFERRED) == 0)
authd_read_client(client_p);
}
void
authd_deferred_client(struct Client *client_p)
{
client_p->preClient->auth.flags &= ~AUTHC_F_DEFERRED;
if(client_p->preClient->auth.flags & AUTHC_F_COMPLETE)
authd_read_client(client_p);
}
/* Convenience function to accept client */

View file

@ -56,6 +56,7 @@ static const struct in6_addr in6addr_any =
static struct Listener *ListenerPollList = NULL;
static int accept_precallback(rb_fde_t *F, struct sockaddr *addr, rb_socklen_t addrlen, void *data);
static void accept_callback(rb_fde_t *F, int status, struct sockaddr *addr, rb_socklen_t addrlen, void *data);
static SSL_OPEN_CB accept_sslcallback;
static struct Listener *
make_listener(struct rb_sockaddr_storage *addr)
@ -455,6 +456,7 @@ static void
add_connection(struct Listener *listener, rb_fde_t *F, struct sockaddr *sai, struct sockaddr *lai)
{
struct Client *new_client;
bool defer = false;
s_assert(NULL != listener);
/*
@ -471,6 +473,8 @@ add_connection(struct Listener *listener, rb_fde_t *F, struct sockaddr *sai, str
free_client(new_client);
return;
}
new_client->localClient->ssl_callback = accept_sslcallback;
defer = true;
new_client->localClient->ssl_ctl = start_ssld_accept(F, xF[1], connid_get(new_client)); /* this will close F for us */
if(new_client->localClient->ssl_ctl == NULL)
{
@ -516,7 +520,14 @@ add_connection(struct Listener *listener, rb_fde_t *F, struct sockaddr *sai, str
++listener->ref_count;
authd_initiate_client(new_client);
authd_initiate_client(new_client, defer);
}
static int
accept_sslcallback(struct Client *client_p, int status)
{
authd_deferred_client(client_p);
return 0; /* use default handler if status != RB_OK */
}
static const char *toofast = "ERROR :Reconnecting too fast, throttled.\r\n";