cleanups and fixes

diff --git a/raddb/certs/realms/README.md b/raddb/certs/realms/README.md
index f4dee04dc00b29a222caa588e0f61bb6a5a399a8..0922403f24fe7d4ed84a3f095f5e1c0e69bfd604 100644 (file)
--- a/raddb/certs/realms/README.md
+++ b/raddb/certs/realms/README.md
@@ -10,12 +10,12 @@ network access, the server can present an `example.com` certificate.
 On the other hand, when a user `doug@example.org` requests network
 access, the server cna present an `example.org` certificate.
 
-This functionaltiy means that it is possible to configure only one
+This functionality means that it is possible to configure only one
 `eap` module, and then use multiple certificate chains. Previous
 versions of the server required that the administrator configure
 multiple EAP modules, one for each certificate being used.
 
-This functionality can be used in one of two ways.  First, the
+The selection can be performed in one of two ways.  First, the
 certificates can be loaded dynamically at run-time.  Second, the
 certificates can be pre-loaded for speed.
 
@@ -195,3 +195,20 @@ a file which taken from
 `realm_dir` portion of the filename is configurable.  The SNI portion
 is taken from the TLS messages, and the `.pem` suffix is hard-coded in
 the source code.
+
+Taking the filename from an untrusted source is fine here.  The
+standard (RFC 6066 Section 3) says that the Server Name Indication
+field is a DNS "A label".  Which means that there are a limited number
+of characters allowed:
+
+* `.`, `-`, `a-Z`, `A-Z`, `0-9`
+
+If the SNI contain anything else, the TLS connection is rejected.
+
+Note that if session resumption is enabled for RadSec, the session
+cache *must* also cache the `TLS-Server-Name-Indication` attribute.
+The SNI is sent on resumption for TLS 1.2 and earlier, but it is not
+sent for TLS 1.3.  As such, the only way to select the right policy on
+resumption is to check the value of the cached
+TLS-Server-Name-Indication attribute.
+

diff --git a/src/main/listen.c b/src/main/listen.c
index bce82801b2c809fbe28c661e638d630eba02d2da..57a627aa6dab1b5cf5b585240a30ef148eded74a 100644 (file)
--- a/src/main/listen.c
+++ b/src/main/listen.c
@@ -656,17 +656,27 @@ static int tls_sni_callback(SSL *ssl, UNUSED int *al, void *arg)
                if ((*p >= '0') && (*p <= '9')) continue;
 
                /*
-                *      Anything else, ignore it.
+                *      Anything else, ignore fail.
                 */
-               return SSL_TLSEXT_ERR_OK;
+               return SSL_TLSEXT_ERR_ALERT_FATAL;
        }
 
+       /*
+        *      Too long, fail.
+        */
+       if ((p - name) > 255) return SSL_TLSEXT_ERR_ALERT_FATAL;
+
        snprintf(buffer, sizeof(buffer), "%s/%s.pem", conf->realm_dir, name);
 
        my_r.name = buffer;
        r = fr_hash_table_finddata(conf->realms, &my_r);
-       if (!r) return SSL_TLSEXT_ERR_OK;
 
+       /*
+        *      If found, switch certs.  Otherwise use the default
+        *      one.
+        */
+       if (r) (void) SSL_set_SSL_CTX(ssl, r->ctx);
+               
        /*
         *      Set an attribute saying which server has been selected.
         */
@@ -675,7 +685,6 @@ static int tls_sni_callback(SSL *ssl, UNUSED int *al, void *arg)
                (void) pair_make_config("TLS-Server-Name-Indication", name, T_OP_SET);
        }
 
-       (void) SSL_set_SSL_CTX(ssl, r->ctx);
        return SSL_TLSEXT_ERR_OK;
 }
 #endif