]> git.ipfire.org Git - thirdparty/freeradius-server.git/commitdiff
Fixup protected success/failure notifications
authorArran Cudbard-Bell <a.cudbardb@freeradius.org>
Wed, 13 Dec 2017 19:51:41 +0000 (19:51 +0000)
committerArran Cudbard-Bell <a.cudbardb@freeradius.org>
Wed, 13 Dec 2017 19:51:41 +0000 (19:51 +0000)
src/modules/rlm_eap/types/rlm_eap_aka/eap_aka.h
src/modules/rlm_eap/types/rlm_eap_aka/rlm_eap_aka.c
src/modules/rlm_eap/types/rlm_eap_sim/eap_sim.h
src/modules/rlm_eap/types/rlm_eap_sim/rlm_eap_sim.c

index f77a5821137f98de8ba7eec104ccd87ce95d706a..c72bc7c5bdfb35bacd6337d202885b692bbdceda 100644 (file)
@@ -38,13 +38,16 @@ typedef enum {
        EAP_AKA_SERVER_CHALLENGE,                               //!< We've challenged the supplicant.
        EAP_AKA_SERVER_SUCCESS_NOTIFICATION,                    //!< Send success notification.
        EAP_AKA_SERVER_SUCCESS,                                 //!< Authentication completed successfully.
-       EAP_AKA_SERVER_GENERAL_FAILURE_NOTIFICATION,            //!< Send failure notification.
+       EAP_AKA_SERVER_FAILURE_NOTIFICATION,                    //!< Send failure notification.
        EAP_AKA_SERVER_FAILURE,                                 //!< Send an EAP-Failure.
        EAP_AKA_SERVER_MAX_STATES
 } eap_aka_server_state_t;
 
 typedef struct {
        eap_aka_server_state_t          state;                  //!< Current session state.
+       bool                            challenge_success;      //!< Whether we received the correct
+                                                               ///< challenge response.
+
        fr_sim_id_req_type_t            id_req;                 //!< The type of identity we're requesting
                                                                ///< or previously requested.
        fr_sim_keys_t                   keys;                   //!< Various EAP-AKA keys.
index bb443cabe4880cf52a9cffec128c93d85d4c87d5..7bff7b2c7f110f1540fba21b78f30fa9bd9be6e4 100644 (file)
@@ -42,7 +42,7 @@ FR_NAME_NUMBER const aka_state_table[] = {
        { "CHALLENGE",                          EAP_AKA_SERVER_CHALLENGE                        },
        { "SUCCESS-NOTIFICATION",               EAP_AKA_SERVER_SUCCESS_NOTIFICATION             },
        { "SUCCESS",                            EAP_AKA_SERVER_SUCCESS                          },
-       { "GENERAL-FAILURE-NOTIFICATION",       EAP_AKA_SERVER_GENERAL_FAILURE_NOTIFICATION     },
+       { "FAILURE-NOTIFICATION",               EAP_AKA_SERVER_FAILURE_NOTIFICATION             },
        { "FAILURE",                            EAP_AKA_SERVER_FAILURE                          },
        { NULL }
 };
@@ -201,7 +201,7 @@ static int eap_aka_send_identity_request(eap_session_t *eap_session)
  *
  * Challenges will come from one of three places eventually:
  *
- * 1  from attributes like FR_EAP_SIM_RANDx
+ * 1  from attributes like FR_EAP_AKA_RANDx
  *         (these might be retrieved from a database)
  *
  * 2  from internally implemented SIM authenticators
@@ -383,14 +383,17 @@ static int eap_aka_send_challenge(eap_session_t *eap_session)
  */
 static int eap_aka_send_eap_success_notification(eap_session_t *eap_session)
 {
-       REQUEST         *request = eap_session->request;
-       RADIUS_PACKET   *packet = eap_session->request->reply;
-       fr_cursor_t     cursor;
-       VALUE_PAIR      *vp;
+       REQUEST                 *request = eap_session->request;
+       RADIUS_PACKET           *packet = eap_session->request->reply;
+       eap_aka_session_t       *eap_aka_session = talloc_get_type_abort(eap_session->opaque, eap_aka_session_t);
+       fr_cursor_t             cursor;
+       VALUE_PAIR              *vp;
 
        RDEBUG2("Sending AKA-Notification (Success)");
        eap_session->this_round->request->code = FR_EAP_CODE_REQUEST;
 
+       if (!fr_cond_assert(eap_aka_session->challenge_success)) return -1;
+
        fr_cursor_init(&cursor, &packet->vps);
 
        /*
@@ -404,6 +407,13 @@ static int eap_aka_send_eap_success_notification(eap_session_t *eap_session)
        vp->vp_uint32 = FR_EAP_AKA_NOTIFICATION_VALUE_SUCCESS;
        fr_cursor_append(&cursor, vp);
 
+       /*
+        *      Need to include an AT_MAC attribute so that it will get
+        *      calculated.
+        */
+       vp = fr_pair_afrom_child_num(packet, dict_aka_root, FR_EAP_AKA_MAC);
+       fr_pair_replace(&packet->vps, vp);
+
        /*
         *      Encode the packet
         */
@@ -446,16 +456,34 @@ static int eap_aka_send_eap_success(eap_session_t *eap_session)
  */
 static int eap_aka_send_eap_failure_notification(eap_session_t *eap_session)
 {
-       REQUEST         *request = eap_session->request;
-       RADIUS_PACKET   *packet = eap_session->request->reply;
-       fr_cursor_t     cursor;
-       VALUE_PAIR      *vp;
-
-       RDEBUG2("Sending AKA-Notification (General-Failure)");
-       eap_session->this_round->request->code = FR_EAP_CODE_REQUEST;
+       REQUEST                 *request = eap_session->request;
+       RADIUS_PACKET           *packet = eap_session->request->reply;
+       fr_cursor_t             cursor;
+       VALUE_PAIR              *vp;
+       eap_aka_session_t       *eap_aka_session = talloc_get_type_abort(eap_session->opaque, eap_aka_session_t);
 
        fr_cursor_init(&cursor, &packet->vps);
 
+       vp = fr_pair_find_by_child_num(packet->vps, dict_sim_root, FR_EAP_AKA_NOTIFICATION, TAG_ANY);
+       if (!vp) {
+               vp = fr_pair_afrom_child_num(packet, dict_sim_root, FR_EAP_AKA_NOTIFICATION);
+               vp->vp_uint16 = FR_EAP_AKA_NOTIFICATION_VALUE_GENERAL_FAILURE;
+               fr_cursor_append(&cursor, vp);
+       }
+
+       /*
+        *      Change the failure notification depending where
+        *      we are in the state machine.
+        */
+       if (eap_aka_session->challenge_success) {
+               vp->vp_uint16 &= ~0x40000;      /* Unset phase bit */
+       } else {
+               vp->vp_uint16 |= 0x40000;       /* Set phase bit */
+       }
+
+       RDEBUG2("Sending AKA-Notification (%pV)", &vp->data);
+       eap_session->this_round->request->code = FR_EAP_CODE_REQUEST;
+
        /*
         *      Set the subtype to notification
         */
@@ -463,9 +491,15 @@ static int eap_aka_send_eap_failure_notification(eap_session_t *eap_session)
        vp->vp_uint32 = FR_EAP_AKA_SUBTYPE_VALUE_AKA_NOTIFICATION;
        fr_cursor_append(&cursor, vp);
 
-       vp = fr_pair_afrom_child_num(packet, dict_aka_root, FR_EAP_AKA_NOTIFICATION);
-       vp->vp_uint32 = FR_EAP_AKA_NOTIFICATION_VALUE_GENERAL_FAILURE;
-       fr_cursor_append(&cursor, vp);
+       /*
+        *      If we're after the challenge phase
+        *      then we need to include a MAC to
+        *      protect notifications.
+        */
+       if (eap_aka_session->challenge_success) {
+               vp = fr_pair_afrom_child_num(packet, dict_sim_root, FR_EAP_AKA_MAC);
+               fr_pair_replace(&packet->vps, vp);
+       }
 
        /*
         *      Encode the packet
@@ -515,7 +549,7 @@ static void eap_aka_state_enter(eap_session_t *eap_session, eap_aka_server_state
        case EAP_AKA_SERVER_IDENTITY:
                if (eap_aka_send_identity_request(eap_session) < 0) {
                notify_failure:
-                       eap_aka_state_enter(eap_session, EAP_AKA_SERVER_GENERAL_FAILURE_NOTIFICATION);
+                       eap_aka_state_enter(eap_session, EAP_AKA_SERVER_FAILURE_NOTIFICATION);
                        return;
                }
                break;
@@ -544,7 +578,7 @@ static void eap_aka_state_enter(eap_session_t *eap_session, eap_aka_server_state
        /*
         *      Send a general failure notification
         */
-       case EAP_AKA_SERVER_GENERAL_FAILURE_NOTIFICATION:
+       case EAP_AKA_SERVER_FAILURE_NOTIFICATION:
                if (eap_aka_send_eap_failure_notification(eap_session) < 0) {   /* Fallback to EAP-Failure */
                        eap_aka_state_enter(eap_session, EAP_AKA_SERVER_FAILURE);
                }
@@ -559,7 +593,7 @@ static void eap_aka_state_enter(eap_session_t *eap_session, eap_aka_server_state
 
        default:
                rad_assert(0);  /* Invalid transition */
-               eap_aka_state_enter(eap_session, EAP_AKA_SERVER_GENERAL_FAILURE_NOTIFICATION);
+               eap_aka_state_enter(eap_session, EAP_AKA_SERVER_FAILURE_NOTIFICATION);
                return;
        }
 }
@@ -621,7 +655,7 @@ static int process_eap_aka_identity(eap_session_t *eap_session, VALUE_PAIR *vps)
        case SIM_PERMANENT_ID_REQ:
                eap_aka_state_enter(eap_session, EAP_AKA_SERVER_CHALLENGE);
 //             REDEBUG2("Failed to negotiate a usable identity");
-//             eap_aka_state_enter(eap_session, eap_aka_session, EAP_AKA_SERVER_GENERAL_FAILURE_NOTIFICATION);
+//             eap_aka_state_enter(eap_session, eap_aka_session, EAP_AKA_SERVER_FAILURE_NOTIFICATION);
                break;
 
        case SIM_NO_ID_REQ:
@@ -728,6 +762,8 @@ static int process_eap_aka_challenge(eap_session_t *eap_session, VALUE_PAIR *vps
 
        RDEBUG2("EAP-AKA-RES matches XRES");
 
+       eap_aka_session->challenge_success = true;
+
        /*
         *      If the peer wants a Success notification, then
         *      send a success notification, otherwise send a
@@ -789,7 +825,7 @@ static rlm_rcode_t mod_process(UNUSED void *arg, eap_session_t *eap_session)
         */
        if (ret < 0) {
                RPEDEBUG2("Failed decoding EAP-AKA attributes");
-               eap_aka_state_enter(eap_session, EAP_AKA_SERVER_GENERAL_FAILURE_NOTIFICATION);
+               eap_aka_state_enter(eap_session, EAP_AKA_SERVER_FAILURE_NOTIFICATION);
                return RLM_MODULE_HANDLED;      /* We need to process more packets */
        }
 
@@ -802,7 +838,7 @@ static rlm_rcode_t mod_process(UNUSED void *arg, eap_session_t *eap_session)
        subtype_vp = fr_pair_find_by_child_num(vps, dict_aka_root, FR_EAP_AKA_SUBTYPE, TAG_ANY);
        if (!subtype_vp) {
                REDEBUG("Missing EAP-AKA-Subtype");
-               eap_aka_state_enter(eap_session, EAP_AKA_SERVER_GENERAL_FAILURE_NOTIFICATION);
+               eap_aka_state_enter(eap_session, EAP_AKA_SERVER_FAILURE_NOTIFICATION);
                return RLM_MODULE_HANDLED;                              /* We need to process more packets */
        }
        subtype = subtype_vp->vp_uint32;
@@ -816,7 +852,7 @@ static rlm_rcode_t mod_process(UNUSED void *arg, eap_session_t *eap_session)
                switch (subtype) {
                case EAP_AKA_IDENTITY:
                        if (process_eap_aka_identity(eap_session, vps) == 0) return RLM_MODULE_HANDLED;
-                       eap_aka_state_enter(eap_session, EAP_AKA_SERVER_GENERAL_FAILURE_NOTIFICATION);
+                       eap_aka_state_enter(eap_session, EAP_AKA_SERVER_FAILURE_NOTIFICATION);
                        return RLM_MODULE_HANDLED;      /* We need to process more packets */
 
                /*
@@ -852,7 +888,7 @@ static rlm_rcode_t mod_process(UNUSED void *arg, eap_session_t *eap_session)
                        vp = fr_pair_afrom_child_num(vps, dict_aka_root, FR_EAP_AKA_NOTIFICATION);
                        if (!vp) {
                                REDEBUG2("Received AKA-Notification with no notification code");
-                               eap_aka_state_enter(eap_session, EAP_AKA_SERVER_GENERAL_FAILURE_NOTIFICATION);
+                               eap_aka_state_enter(eap_session, EAP_AKA_SERVER_FAILURE_NOTIFICATION);
                                return RLM_MODULE_HANDLED;                      /* We need to process more packets */
                        }
 
@@ -882,7 +918,7 @@ static rlm_rcode_t mod_process(UNUSED void *arg, eap_session_t *eap_session)
                         *      send an EAP-Failure in this case.
                         */
                        REDEBUG("Unexpected subtype %pV", &subtype_vp->data);
-                       eap_aka_state_enter(eap_session, EAP_AKA_SERVER_GENERAL_FAILURE_NOTIFICATION);
+                       eap_aka_state_enter(eap_session, EAP_AKA_SERVER_FAILURE_NOTIFICATION);
                        return RLM_MODULE_HANDLED;                              /* We need to process more packets */
                }
 
@@ -895,12 +931,12 @@ static rlm_rcode_t mod_process(UNUSED void *arg, eap_session_t *eap_session)
                switch (subtype) {
                case EAP_AKA_CHALLENGE:
                        if (process_eap_aka_challenge(eap_session, vps) == 0) return RLM_MODULE_HANDLED;
-                       eap_aka_state_enter(eap_session, EAP_AKA_SERVER_GENERAL_FAILURE_NOTIFICATION);
+                       eap_aka_state_enter(eap_session, EAP_AKA_SERVER_FAILURE_NOTIFICATION);
                        return RLM_MODULE_HANDLED;                              /* We need to process more packets */
 
                case EAP_AKA_SYNCHRONIZATION_FAILURE:
                        REDEBUG("EAP-AKA Peer synchronization failure");        /* We can't handle these yet */
-                       eap_aka_state_enter(eap_session, EAP_AKA_SERVER_GENERAL_FAILURE_NOTIFICATION);
+                       eap_aka_state_enter(eap_session, EAP_AKA_SERVER_FAILURE_NOTIFICATION);
                        return RLM_MODULE_HANDLED;                              /* We need to process more packets */
 
                /*
@@ -940,7 +976,7 @@ static rlm_rcode_t mod_process(UNUSED void *arg, eap_session_t *eap_session)
        /*
         *      Peer acked our failure
         */
-       case EAP_AKA_SERVER_GENERAL_FAILURE_NOTIFICATION:
+       case EAP_AKA_SERVER_FAILURE_NOTIFICATION:
                switch (subtype) {
                case EAP_AKA_NOTIFICATION:
                        RDEBUG2("AKA-Notification ACKed, sending EAP-Failure");
@@ -956,7 +992,7 @@ static rlm_rcode_t mod_process(UNUSED void *arg, eap_session_t *eap_session)
         */
        default:
                rad_assert(0);
-               eap_aka_state_enter(eap_session, EAP_AKA_SERVER_GENERAL_FAILURE_NOTIFICATION);
+               eap_aka_state_enter(eap_session, EAP_AKA_SERVER_FAILURE_NOTIFICATION);
                return RLM_MODULE_HANDLED;                              /* We need to process more packets */
        }
 }
index 3c5380bdbb7e2849d26cddfcd4ed9e9809830a68..70080fb655f3ce600c2ad7916653ebb7d04d423f 100644 (file)
@@ -37,17 +37,21 @@ typedef enum {
        EAP_SIM_SERVER_CHALLENGE,
        EAP_SIM_SERVER_SUCCESS_NOTIFICATION,
        EAP_SIM_SERVER_SUCCESS,
-       EAP_SIM_SERVER_GENERAL_FAILURE_NOTIFICATION,
+       EAP_SIM_SERVER_FAILURE_NOTIFICATION,
        EAP_SIM_SERVER_FAILURE,
        EAP_SIM_SERVER_MAX_STATES
 } eap_sim_server_state_t;
 
-typedef struct eap_aka_session {
+typedef struct {
        eap_sim_server_state_t          state;                  //!< Current session state.
+       bool                            challenge_success;      //!< Whether we received the correct
+                                                               ///< challenge response.
+
        fr_sim_keys_t                   keys;                   //!< Various EAP-AKA keys.
        fr_sim_id_req_type_t            id_req;                 //!< The type of identity we're requesting
                                                                ///< or previously requested.
 
+
        bool                            request_identity;       //!< Always send an identity request before a
                                                                ///< challenge.
 
index 627a57ee3494bf74ca3ec0ca5ae431f60eb0ab33..71cc71ef085064f417bdca6958d65d5bcf9eb7f2 100644 (file)
@@ -44,7 +44,7 @@ FR_NAME_NUMBER const sim_state_table[] = {
        { "CHALLENGE",                          EAP_SIM_SERVER_CHALLENGE                        },
        { "SUCCESS-NOTIFICATION",               EAP_SIM_SERVER_SUCCESS_NOTIFICATION             },
        { "SUCCESS",                            EAP_SIM_SERVER_SUCCESS                          },
-       { "GENERAL-FAILURE-NOTIFICATION",       EAP_SIM_SERVER_GENERAL_FAILURE_NOTIFICATION     },
+       { "FAILURE-NOTIFICATION",               EAP_SIM_SERVER_FAILURE_NOTIFICATION             },
        { "FAILURE",                            EAP_SIM_SERVER_FAILURE                          },
        { NULL }
 };
@@ -57,7 +57,7 @@ static CONF_PARSER submodule_config[] = {
 /*
  *     build a reply to be sent.
  */
-static int eap_sim_compose(eap_session_t *eap_session)
+static int eap_sim_compose(eap_session_t *eap_session, uint8_t const *hmac_extra, size_t hmac_extra_len)
 {
        eap_sim_session_t       *eap_sim_session = talloc_get_type_abort(eap_session->opaque, eap_sim_session_t);
        vp_cursor_t             cursor;
@@ -74,8 +74,8 @@ static int eap_sim_compose(eap_session_t *eap_session)
 
                                        .hmac_md = EVP_sha1(),
                                        .eap_packet = eap_session->this_round->request,
-                                       .hmac_extra = eap_sim_session->keys.gsm.nonce_mt,
-                                       .hmac_extra_len = sizeof(eap_sim_session->keys.gsm.nonce_mt)
+                                       .hmac_extra = hmac_extra,
+                                       .hmac_extra_len = hmac_extra_len
                                };
 
        ssize_t                 ret;
@@ -175,7 +175,7 @@ static int eap_sim_send_start(eap_session_t *eap_session)
        /*
         *      Encode the packet
         */
-       if (eap_sim_compose(eap_session) < 0) {
+       if (eap_sim_compose(eap_session, NULL, 0) < 0) {
                fr_pair_list_free(&packet->vps);
                return -1;
        }
@@ -201,8 +201,6 @@ static int eap_sim_send_start(eap_session_t *eap_session)
  */
 static int eap_sim_send_challenge(eap_session_t *eap_session)
 {
-       static uint8_t          hmac_zero[16] = { 0x00 };
-
        REQUEST                 *request = eap_session->request;
        eap_sim_session_t       *eap_sim_session = talloc_get_type_abort(eap_session->opaque, eap_sim_session_t);
        VALUE_PAIR              **to_peer, *vp;
@@ -271,13 +269,13 @@ static int eap_sim_send_challenge(eap_session_t *eap_session)
         *      calculated.
         */
        vp = fr_pair_afrom_child_num(packet, dict_sim_root, FR_EAP_SIM_MAC);
-       fr_pair_value_memcpy(vp, hmac_zero, sizeof(hmac_zero));
        fr_pair_replace(to_peer, vp);
 
        /*
         *      Encode the packet
         */
-       if (eap_sim_compose(eap_session) < 0) {
+       if (eap_sim_compose(eap_session,
+                           eap_sim_session->keys.gsm.nonce_mt, sizeof(eap_sim_session->keys.gsm.nonce_mt)) < 0) {
                fr_pair_list_free(&packet->vps);
                return -1;
        }
@@ -290,14 +288,17 @@ static int eap_sim_send_challenge(eap_session_t *eap_session)
  */
 static int eap_sim_send_eap_success_notification(eap_session_t *eap_session)
 {
-       REQUEST         *request = eap_session->request;
-       RADIUS_PACKET   *packet = eap_session->request->reply;
-       fr_cursor_t     cursor;
-       VALUE_PAIR      *vp;
+       REQUEST                 *request = eap_session->request;
+       RADIUS_PACKET           *packet = eap_session->request->reply;
+       eap_sim_session_t       *eap_sim_session = talloc_get_type_abort(eap_session->opaque, eap_sim_session_t);
+       fr_cursor_t             cursor;
+       VALUE_PAIR              *vp;
 
        RDEBUG2("Sending SIM-Notification (Success)");
        eap_session->this_round->request->code = FR_EAP_CODE_REQUEST;
 
+       if (!fr_cond_assert(eap_sim_session->challenge_success)) return -1;
+
        fr_cursor_init(&cursor, &packet->vps);
 
        /*
@@ -311,10 +312,17 @@ static int eap_sim_send_eap_success_notification(eap_session_t *eap_session)
        vp->vp_uint32 = FR_EAP_SIM_NOTIFICATION_VALUE_SUCCESS;
        fr_cursor_append(&cursor, vp);
 
+       /*
+        *      Need to include an AT_MAC attribute so that it will get
+        *      calculated.
+        */
+       vp = fr_pair_afrom_child_num(packet, dict_sim_root, FR_EAP_SIM_MAC);
+       fr_pair_replace(&packet->vps, vp);
+
        /*
         *      Encode the packet
         */
-       if (eap_sim_compose(eap_session) < 0) {
+       if (eap_sim_compose(eap_session, NULL, 0) < 0) {
                fr_pair_list_free(&packet->vps);
                return -1;
        }
@@ -350,16 +358,34 @@ static int eap_sim_send_eap_success(eap_session_t *eap_session)
  */
 static int eap_sim_send_eap_failure_notification(eap_session_t *eap_session)
 {
-       REQUEST         *request = eap_session->request;
-       RADIUS_PACKET   *packet = eap_session->request->reply;
-       fr_cursor_t     cursor;
-       VALUE_PAIR      *vp;
-
-       RDEBUG2("Sending SIM-Notification (General-Failure)");
-       eap_session->this_round->request->code = FR_EAP_CODE_REQUEST;
+       REQUEST                 *request = eap_session->request;
+       RADIUS_PACKET           *packet = eap_session->request->reply;
+       fr_cursor_t             cursor;
+       VALUE_PAIR              *vp;
+       eap_sim_session_t       *eap_sim_session = talloc_get_type_abort(eap_session->opaque, eap_sim_session_t);
 
        fr_cursor_init(&cursor, &packet->vps);
 
+       vp = fr_pair_find_by_child_num(packet->vps, dict_sim_root, FR_EAP_SIM_NOTIFICATION, TAG_ANY);
+       if (!vp) {
+               vp = fr_pair_afrom_child_num(packet, dict_sim_root, FR_EAP_SIM_NOTIFICATION);
+               vp->vp_uint16 = FR_EAP_SIM_NOTIFICATION_VALUE_GENERAL_FAILURE;
+               fr_cursor_append(&cursor, vp);
+       }
+
+       /*
+        *      Change the failure notification depending where
+        *      we are in the state machine.
+        */
+       if (eap_sim_session->challenge_success) {
+               vp->vp_uint16 &= ~0x40000;      /* Unset phase bit */
+       } else {
+               vp->vp_uint16 |= 0x40000;       /* Set phase bit */
+       }
+
+       RDEBUG2("Sending SIM-Notification (%pV)", &vp->data);
+       eap_session->this_round->request->code = FR_EAP_CODE_REQUEST;
+
        /*
         *      Set the subtype to notification
         */
@@ -367,14 +393,20 @@ static int eap_sim_send_eap_failure_notification(eap_session_t *eap_session)
        vp->vp_uint32 = FR_EAP_SIM_SUBTYPE_VALUE_SIM_NOTIFICATION;
        fr_cursor_append(&cursor, vp);
 
-       vp = fr_pair_afrom_child_num(packet, dict_sim_root, FR_EAP_SIM_NOTIFICATION);
-       vp->vp_uint32 = FR_EAP_SIM_NOTIFICATION_VALUE_GENERAL_FAILURE;
-       fr_cursor_append(&cursor, vp);
+       /*
+        *      If we're after the challenge phase
+        *      then we need to include a MAC to
+        *      protect notifications.
+        */
+       if (eap_sim_session->challenge_success) {
+               vp = fr_pair_afrom_child_num(packet, dict_sim_root, FR_EAP_SIM_MAC);
+               fr_pair_replace(&packet->vps, vp);
+       }
 
        /*
         *      Encode the packet
         */
-       if (eap_sim_compose(eap_session) < 0) {
+       if (eap_sim_compose(eap_session, NULL, 0) < 0) {
                fr_pair_list_free(&packet->vps);
                return -1;
        }
@@ -419,7 +451,7 @@ static void eap_sim_state_enter(eap_session_t *eap_session, eap_sim_server_state
        case EAP_SIM_SERVER_START:
                if (eap_sim_send_start(eap_session) < 0) {
                notify_failure:
-                       eap_sim_state_enter(eap_session, EAP_SIM_SERVER_GENERAL_FAILURE_NOTIFICATION);
+                       eap_sim_state_enter(eap_session, EAP_SIM_SERVER_FAILURE_NOTIFICATION);
                        return;
                }
                break;
@@ -448,7 +480,7 @@ static void eap_sim_state_enter(eap_session_t *eap_session, eap_sim_server_state
        /*
         *      Send a general failure notification
         */
-       case EAP_SIM_SERVER_GENERAL_FAILURE_NOTIFICATION:
+       case EAP_SIM_SERVER_FAILURE_NOTIFICATION:
                if (eap_sim_send_eap_failure_notification(eap_session) < 0) {   /* Fallback to EAP-Failure */
                        eap_sim_state_enter(eap_session, EAP_SIM_SERVER_FAILURE);
                }
@@ -463,7 +495,7 @@ static void eap_sim_state_enter(eap_session_t *eap_session, eap_sim_server_state
 
        default:
                rad_assert(0);  /* Invalid transition */
-               eap_sim_state_enter(eap_session, EAP_SIM_SERVER_GENERAL_FAILURE_NOTIFICATION);
+               eap_sim_state_enter(eap_session, EAP_SIM_SERVER_FAILURE_NOTIFICATION);
                return;
        }
 }
@@ -563,7 +595,7 @@ static int process_eap_sim_start(eap_session_t *eap_session, VALUE_PAIR *vps)
        case SIM_PERMANENT_ID_REQ:
                eap_sim_state_enter(eap_session, EAP_SIM_SERVER_CHALLENGE);
 //             REDEBUG2("Failed to negotiate a usable identity");
-//             eap_sim_state_enter(eap_session, eap_sim_session, EAP_SIM_SERVER_GENERAL_FAILURE_NOTIFICATION);
+//             eap_sim_state_enter(eap_session, eap_sim_session, EAP_SIM_SERVER_FAILURE_NOTIFICATION);
                break;
        }
 
@@ -625,6 +657,8 @@ static int process_eap_sim_challenge(eap_session_t *eap_session, VALUE_PAIR *vps
                return -1;
        }
 
+       eap_sim_session->challenge_success = true;
+
        /*
         *      If the peer wants a Success notification, then
         *      send a success notification, otherwise send a
@@ -680,7 +714,7 @@ static rlm_rcode_t mod_process(UNUSED void *arg, eap_session_t *eap_session)
         */
        if (ret < 0) {
                RPEDEBUG2("Failed decoding EAP-SIM attributes");
-               eap_sim_state_enter(eap_session, EAP_SIM_SERVER_GENERAL_FAILURE_NOTIFICATION);
+               eap_sim_state_enter(eap_session, EAP_SIM_SERVER_FAILURE_NOTIFICATION);
                return RLM_MODULE_HANDLED;      /* We need to process more packets */
        }
 
@@ -693,7 +727,7 @@ static rlm_rcode_t mod_process(UNUSED void *arg, eap_session_t *eap_session)
        subtype_vp = fr_pair_find_by_child_num(from_peer, dict_sim_root, FR_EAP_SIM_SUBTYPE, TAG_ANY);
        if (!subtype_vp) {
                REDEBUG("Missing EAP-SIM-Subtype");
-               eap_sim_state_enter(eap_session, EAP_SIM_SERVER_GENERAL_FAILURE_NOTIFICATION);
+               eap_sim_state_enter(eap_session, EAP_SIM_SERVER_FAILURE_NOTIFICATION);
                return RLM_MODULE_HANDLED;                              /* We need to process more packets */
        }
        subtype = subtype_vp->vp_uint32;
@@ -707,7 +741,7 @@ static rlm_rcode_t mod_process(UNUSED void *arg, eap_session_t *eap_session)
                switch (subtype) {
                case EAP_SIM_START:
                        if (process_eap_sim_start(eap_session, from_peer) == 0) return RLM_MODULE_HANDLED;
-                       eap_sim_state_enter(eap_session, EAP_SIM_SERVER_GENERAL_FAILURE_NOTIFICATION);
+                       eap_sim_state_enter(eap_session, EAP_SIM_SERVER_FAILURE_NOTIFICATION);
                        return RLM_MODULE_HANDLED;      /* We need to process more packets */
 
                /*
@@ -743,7 +777,7 @@ static rlm_rcode_t mod_process(UNUSED void *arg, eap_session_t *eap_session)
                        vp = fr_pair_afrom_child_num(from_peer, dict_sim_root, FR_EAP_SIM_NOTIFICATION);
                        if (!vp) {
                                REDEBUG2("Received SIM-Notification with no notification code");
-                               eap_sim_state_enter(eap_session, EAP_SIM_SERVER_GENERAL_FAILURE_NOTIFICATION);
+                               eap_sim_state_enter(eap_session, EAP_SIM_SERVER_FAILURE_NOTIFICATION);
                                return RLM_MODULE_HANDLED;                      /* We need to process more packets */
                        }
 
@@ -772,7 +806,7 @@ static rlm_rcode_t mod_process(UNUSED void *arg, eap_session_t *eap_session)
                         *      send an EAP-Failure in this case.
                         */
                        REDEBUG("Unexpected subtype %pV", &subtype_vp->data);
-                       eap_sim_state_enter(eap_session, EAP_SIM_SERVER_GENERAL_FAILURE_NOTIFICATION);
+                       eap_sim_state_enter(eap_session, EAP_SIM_SERVER_FAILURE_NOTIFICATION);
                        return RLM_MODULE_HANDLED;                              /* We need to process more packets */
                }
                }
@@ -814,7 +848,7 @@ static rlm_rcode_t mod_process(UNUSED void *arg, eap_session_t *eap_session)
        /*
         *      Peer acked our failure
         */
-       case EAP_SIM_SERVER_GENERAL_FAILURE_NOTIFICATION:
+       case EAP_SIM_SERVER_FAILURE_NOTIFICATION:
                switch (subtype) {
                case EAP_SIM_NOTIFICATION:
                        RDEBUG2("SIM-Notification ACKed, sending EAP-Failure");
@@ -830,7 +864,7 @@ static rlm_rcode_t mod_process(UNUSED void *arg, eap_session_t *eap_session)
         */
        default:
                rad_assert(0);
-               eap_sim_state_enter(eap_session, EAP_SIM_SERVER_GENERAL_FAILURE_NOTIFICATION);
+               eap_sim_state_enter(eap_session, EAP_SIM_SERVER_FAILURE_NOTIFICATION);
                return RLM_MODULE_HANDLED;                              /* We need to process more packets */
        }