[Swan] XAUTH and uniqueids=yes

Paul Wouters pwouters at redhat.com
Sun Mar 24 03:40:16 EET 2013


Antony found an interesting bug in the XAUTH code,

In initiate.c there is a function ISAKMP_SA_established() that checks
for uniqueids=true. If it is, the code calls release_connection() on
the old connection.

void
ISAKMP_SA_established(struct connection *c, so_serial_t serial)
{
     c->newest_isakmp_sa = serial;

     if (uniqueIDs
#ifdef XAUTH
 	&& (!c->spd.this.xauth_server)


For XAUTH with PSK (or raw RSA but that's not common) this should of
course not be done on the server/responder side. All connections come in
with the "GroupName" which is really the leftid= and the group psk. So
all these connections are identical up to the passing of the xauth
username/password, which happens later on.

However, currently with iphones using XAUTH with certificates, all the
IDs are based on the certificate CN's, so the code _should_ trigger
and kill older connections with the same IDs. It did not so we have
thousands of old connections lingering.

So I'm contemplating changing the code to:

     if (uniqueIDs)
     {
#ifdef XAUTH
         /* XAUTH PSK connections are fully identical IDs with group psk,
          * so we _must_ allow duplicates. For XAUTH RSA/CERT, the IDs
          * _are_
          * different, and we _should_ release the older connection
          */
          if (c->spd.this.xauth_server)
          {
              if (c->policy & POLICY_PSK)
              {
                 DBG(DBG_CONTROLMORE, DBG_log("uniqueids check ignored for XAUTH server and PSK"));
                 return;
              }
              if ((c->policy & POLICY_RSASIG) && c->spd.this.cert.type == CERT_NONE)
              {
                 DBG(DBG_CONTROLMORE, DBG_log("uniqueids check ignored for XAUTH server and RAW RSA"));
                 return;
              }
              DBG(DBG_CONTROLMORE, DBG_log("uniqueids check performed for XAUTH server and RSA CERT"));
          } else {
              DBG(DBG_CONTROLMORE, DBG_log("uniqueids check performed for non-XAUTH server connection "));
          }
#endif


Although the reason for this post is a more generic question. Is there
any good reason why uniqueids=yes|no is a global option instead of a
per-connection option? Thinking about this I see no reason why we cannot
move this option to a per-conn setting. Does anyone see a problem with
doing that? Do we have any defined behaviour for different type of
connections with the same set of IDs? I know one case we currently don't
support is having a single conn be either RSA or PSK. Any other
concerns?

Paul


More information about the Swan mailing list