]> git.ipfire.org Git - thirdparty/strongswan.git/commitdiff
ike: Treat action_t as flags so 'start' and 'trap' can be combined
authorTobias Brunner <tobias@strongswan.org>
Thu, 16 Dec 2021 15:34:37 +0000 (16:34 +0100)
committerTobias Brunner <tobias@strongswan.org>
Thu, 14 Apr 2022 16:42:01 +0000 (18:42 +0200)
While combining the actions could cause duplicates (while the SA is
initiated, traffic might trigger the trap and the initiation of another
CHILD_SA), the previous commit should avoid most duplicates.  If reuse_ikesa
is disabled, duplicates can't be prevented, though.

src/charon-nm/nm/nm_service.c
src/frontends/android/app/src/main/jni/libandroidbridge/backend/android_service.c
src/libcharon/config/child_cfg.c
src/libcharon/config/child_cfg.h
src/libcharon/plugins/stroke/stroke_config.c
src/libcharon/plugins/vici/vici_config.c
src/libcharon/processing/jobs/start_action_job.c
src/libcharon/sa/ike_sa.c
src/libcharon/sa/ikev1/tasks/quick_delete.c
src/libcharon/sa/ikev2/tasks/child_delete.c

index 6fae7bf9a375d40531f08a6e57a062ca96bc9bd0..09107a76b13d1fc82c99a6f50f465dfea510cdfa 100644 (file)
@@ -635,8 +635,8 @@ static gboolean connect_(NMVpnServicePlugin *plugin, NMConnection *connection,
                        },
                },
                .mode = MODE_TUNNEL,
-               .dpd_action = ACTION_RESTART,
-               .close_action = ACTION_RESTART,
+               .dpd_action = ACTION_START,
+               .close_action = ACTION_START,
        };
 
        /**
index a99d039f8e2365451ac30311304999a633f4337a..77decfd80707db0bcccc3e5f0eb9df2ef49751ef 100644 (file)
@@ -801,8 +801,8 @@ static job_requeue_t initiate(private_android_service_t *this)
                        },
                },
                .mode = MODE_TUNNEL,
-               .dpd_action = ACTION_RESTART,
-               .close_action = ACTION_RESTART,
+               .dpd_action = ACTION_START,
+               .close_action = ACTION_START,
        };
        char *type, *remote_id;
 
index d4fb37cdcac0c580f2ce11ff28c0699f3afa8775..9ea6186c3a37cd3c74199f63eea671d4c0cfad17 100644 (file)
 
 #include <daemon.h>
 
-ENUM(action_names, ACTION_NONE, ACTION_RESTART,
-       "clear",
-       "hold",
-       "restart",
+ENUM_FLAGS(action_names, ACTION_TRAP, ACTION_START,
+       "none",
+       "trap",
+       "start",
 );
 
 /** Default replay window size, if not set using charon.replay_window */
index b176d67bd920300b964992ce905e286c10faeafe..4de978826e0ee057948325e05e749db903777a3e 100644 (file)
@@ -40,11 +40,11 @@ typedef struct child_cfg_create_t child_cfg_create_t;
  */
 enum action_t {
        /** No action */
-       ACTION_NONE,
-       /** Route config to establish or reestablish on demand */
-       ACTION_ROUTE,
-       /** Start or restart config immediately */
-       ACTION_RESTART,
+       ACTION_NONE = 0,
+       /** Install trap policy to (re-)establish on demand */
+       ACTION_TRAP = (1<<0),
+       /** Start or restart immediately */
+       ACTION_START = (1<<1),
 };
 
 /**
index 175b6b5495e29d80312a941dd70a408f2b8f1d38..a15c9f5a45a88d80caab57b229f5c80e132664d1 100644 (file)
@@ -1042,9 +1042,9 @@ static action_t map_action(int starter_action)
        switch (starter_action)
        {
                case 2: /* =hold */
-                       return ACTION_ROUTE;
+                       return ACTION_TRAP;
                case 3: /* =restart */
-                       return ACTION_RESTART;
+                       return ACTION_START;
                default:
                        return ACTION_NONE;
        }
index aa6fbafbeb15cfa0fef4238592282811bf12c9fb..2c570c69b66655e1c1deccda87458d9d0b05bfa5 100644 (file)
@@ -1004,10 +1004,10 @@ CALLBACK(parse_action, bool,
        action_t *out, chunk_t v)
 {
        enum_map_t map[] = {
-               { "start",              ACTION_RESTART  },
-               { "restart",    ACTION_RESTART  },
-               { "route",              ACTION_ROUTE    },
-               { "trap",               ACTION_ROUTE    },
+               { "start",              ACTION_START    },
+               { "restart",    ACTION_START    },
+               { "route",              ACTION_TRAP             },
+               { "trap",               ACTION_TRAP             },
                { "none",               ACTION_NONE             },
                { "clear",              ACTION_NONE             },
        };
@@ -2143,30 +2143,33 @@ CALLBACK(peer_sn, bool,
 static void run_start_action(private_vici_config_t *this, peer_cfg_t *peer_cfg,
                                                         child_cfg_t *child_cfg)
 {
-       switch (child_cfg->get_start_action(child_cfg))
+       action_t action;
+
+       action = child_cfg->get_start_action(child_cfg);
+
+       if (action & ACTION_TRAP)
+       {
+               DBG1(DBG_CFG, "installing '%s'", child_cfg->get_name(child_cfg));
+               switch (child_cfg->get_mode(child_cfg))
+               {
+                       case MODE_PASS:
+                       case MODE_DROP:
+                               charon->shunts->install(charon->shunts,
+                                                                               peer_cfg->get_name(peer_cfg), child_cfg);
+                               /* no need to check for ACTION_START */
+                               return;
+                       default:
+                               charon->traps->install(charon->traps, peer_cfg, child_cfg);
+                               break;
+               }
+       }
+
+       if (action & ACTION_START)
        {
-               case ACTION_RESTART:
-                       DBG1(DBG_CFG, "initiating '%s'", child_cfg->get_name(child_cfg));
-                       charon->controller->initiate(charon->controller,
+               DBG1(DBG_CFG, "initiating '%s'", child_cfg->get_name(child_cfg));
+               charon->controller->initiate(charon->controller,
                                        peer_cfg->get_ref(peer_cfg), child_cfg->get_ref(child_cfg),
                                        NULL, NULL, 0, FALSE);
-                       break;
-               case ACTION_ROUTE:
-                       DBG1(DBG_CFG, "installing '%s'", child_cfg->get_name(child_cfg));
-                       switch (child_cfg->get_mode(child_cfg))
-                       {
-                               case MODE_PASS:
-                               case MODE_DROP:
-                                       charon->shunts->install(charon->shunts,
-                                                                       peer_cfg->get_name(peer_cfg), child_cfg);
-                                       break;
-                               default:
-                                       charon->traps->install(charon->traps, peer_cfg, child_cfg);
-                                       break;
-                       }
-                       break;
-               default:
-                       break;
        }
 }
 
@@ -2181,100 +2184,101 @@ static void clear_start_action(private_vici_config_t *this, char *peer_name,
        ike_sa_t *ike_sa;
        uint32_t id = 0, others;
        array_t *ids = NULL, *ikeids = NULL;
+       action_t action;
        char *name;
 
        name = child_cfg->get_name(child_cfg);
+       action = child_cfg->get_start_action(child_cfg);
+
+       if (action & ACTION_TRAP)
+       {
+               DBG1(DBG_CFG, "uninstalling '%s'", name);
+               switch (child_cfg->get_mode(child_cfg))
+               {
+                       case MODE_PASS:
+                       case MODE_DROP:
+                               charon->shunts->uninstall(charon->shunts, peer_name, name);
+                               /* no need to check for ACTION_START */
+                               return;
+                       default:
+                               charon->traps->uninstall(charon->traps, peer_name, name);
+                               break;
+               }
+       }
 
-       switch (child_cfg->get_start_action(child_cfg))
+       if (action & ACTION_START)
        {
-               case ACTION_RESTART:
-                       enumerator = charon->controller->create_ike_sa_enumerator(
+               enumerator = charon->controller->create_ike_sa_enumerator(
                                                                                                        charon->controller, TRUE);
-                       while (enumerator->enumerate(enumerator, &ike_sa))
+               while (enumerator->enumerate(enumerator, &ike_sa))
+               {
+                       if (!streq(ike_sa->get_name(ike_sa), peer_name))
                        {
-                               if (!streq(ike_sa->get_name(ike_sa), peer_name))
-                               {
-                                       continue;
-                               }
-                               others = id = 0;
-                               children = ike_sa->create_child_sa_enumerator(ike_sa);
-                               while (children->enumerate(children, &child_sa))
+                               continue;
+                       }
+                       others = id = 0;
+                       children = ike_sa->create_child_sa_enumerator(ike_sa);
+                       while (children->enumerate(children, &child_sa))
+                       {
+                               if (child_sa->get_state(child_sa) != CHILD_DELETING &&
+                                       child_sa->get_state(child_sa) != CHILD_DELETED)
                                {
-                                       if (child_sa->get_state(child_sa) != CHILD_DELETING &&
-                                               child_sa->get_state(child_sa) != CHILD_DELETED)
+                                       if (streq(name, child_sa->get_name(child_sa)))
                                        {
-                                               if (streq(name, child_sa->get_name(child_sa)))
-                                               {
-                                                       id = child_sa->get_unique_id(child_sa);
-                                               }
-                                               else
-                                               {
-                                                       others++;
-                                               }
+                                               id = child_sa->get_unique_id(child_sa);
                                        }
-                               }
-                               children->destroy(children);
-
-                               if (!ike_sa->get_child_count(ike_sa) || (id && !others))
-                               {
-                                       /* found no children or only matching, delete IKE_SA */
-                                       id = ike_sa->get_unique_id(ike_sa);
-                                       array_insert_create_value(&ikeids, sizeof(id),
-                                                                                         ARRAY_TAIL, &id);
-                               }
-                               else
-                               {
-                                       children = ike_sa->create_child_sa_enumerator(ike_sa);
-                                       while (children->enumerate(children, &child_sa))
+                                       else
                                        {
-                                               if (streq(name, child_sa->get_name(child_sa)))
-                                               {
-                                                       id = child_sa->get_unique_id(child_sa);
-                                                       array_insert_create_value(&ids, sizeof(id),
-                                                                                                         ARRAY_TAIL, &id);
-                                               }
+                                               others++;
                                        }
-                                       children->destroy(children);
                                }
                        }
-                       enumerator->destroy(enumerator);
+                       children->destroy(children);
 
-                       if (array_count(ids))
+                       if (!ike_sa->get_child_count(ike_sa) || (id && !others))
                        {
-                               while (array_remove(ids, ARRAY_HEAD, &id))
-                               {
-                                       DBG1(DBG_CFG, "closing '%s' #%u", name, id);
-                                       charon->controller->terminate_child(charon->controller,
-                                                                                                               id, NULL, NULL, 0);
-                               }
-                               array_destroy(ids);
+                               /* found no children or only matching, delete IKE_SA */
+                               id = ike_sa->get_unique_id(ike_sa);
+                               array_insert_create_value(&ikeids, sizeof(id),
+                                                                                 ARRAY_TAIL, &id);
                        }
-                       if (array_count(ikeids))
+                       else
                        {
-                               while (array_remove(ikeids, ARRAY_HEAD, &id))
+                               children = ike_sa->create_child_sa_enumerator(ike_sa);
+                               while (children->enumerate(children, &child_sa))
                                {
-                                       DBG1(DBG_CFG, "closing IKE_SA #%u", id);
-                                       charon->controller->terminate_ike(charon->controller, id,
-                                                                                                         FALSE, NULL, NULL, 0);
+                                       if (streq(name, child_sa->get_name(child_sa)))
+                                       {
+                                               id = child_sa->get_unique_id(child_sa);
+                                               array_insert_create_value(&ids, sizeof(id),
+                                                                                                 ARRAY_TAIL, &id);
+                                       }
                                }
-                               array_destroy(ikeids);
+                               children->destroy(children);
                        }
-                       break;
-               case ACTION_ROUTE:
-                       DBG1(DBG_CFG, "uninstalling '%s'", name);
-                       switch (child_cfg->get_mode(child_cfg))
+               }
+               enumerator->destroy(enumerator);
+
+               if (array_count(ids))
+               {
+                       while (array_remove(ids, ARRAY_HEAD, &id))
                        {
-                               case MODE_PASS:
-                               case MODE_DROP:
-                                       charon->shunts->uninstall(charon->shunts, peer_name, name);
-                                       break;
-                               default:
-                                       charon->traps->uninstall(charon->traps, peer_name, name);
-                                       break;
+                               DBG1(DBG_CFG, "closing '%s' #%u", name, id);
+                               charon->controller->terminate_child(charon->controller,
+                                                                                                       id, NULL, NULL, 0);
                        }
-                       break;
-               default:
-                       break;
+                       array_destroy(ids);
+               }
+               if (array_count(ikeids))
+               {
+                       while (array_remove(ikeids, ARRAY_HEAD, &id))
+                       {
+                               DBG1(DBG_CFG, "closing IKE_SA #%u", id);
+                               charon->controller->terminate_ike(charon->controller, id,
+                                                                                                 FALSE, NULL, NULL, 0);
+                       }
+                       array_destroy(ikeids);
+               }
        }
 }
 
index 3a0ed879f53e41aef51dcad2704a55811c61b34e..31e154a770e20c4a274ecc130f3a20cfeecdab47 100644 (file)
@@ -42,8 +42,7 @@ METHOD(job_t, execute, job_requeue_t,
        enumerator_t *enumerator, *children;
        peer_cfg_t *peer_cfg;
        child_cfg_t *child_cfg;
-       ipsec_mode_t mode;
-       char *name;
+       action_t action;
 
        enumerator = charon->backends->create_peer_cfg_enumerator(charon->backends,
                                                                                        NULL, NULL, NULL, NULL, IKE_ANY);
@@ -52,34 +51,39 @@ METHOD(job_t, execute, job_requeue_t,
                children = peer_cfg->create_child_cfg_enumerator(peer_cfg);
                while (children->enumerate(children, &child_cfg))
                {
-                       name = child_cfg->get_name(child_cfg);
+                       action = child_cfg->get_start_action(child_cfg);
+                       if (action == ACTION_NONE)
+                       {
+                               continue;
+                       }
+
+                       DBG1(DBG_JOB, "start action: %N '%s'", action_names, action,
+                                child_cfg->get_name(child_cfg));
 
-                       switch (child_cfg->get_start_action(child_cfg))
+                       if (action & ACTION_TRAP)
                        {
-                               case ACTION_RESTART:
-                                       DBG1(DBG_JOB, "start action: initiate '%s'", name);
-                                       charon->controller->initiate(charon->controller,
-                                                                                                peer_cfg->get_ref(peer_cfg),
-                                                                                                child_cfg->get_ref(child_cfg),
-                                                                                                NULL, NULL, 0, FALSE);
-                                       break;
-                               case ACTION_ROUTE:
-                                       DBG1(DBG_JOB, "start action: route '%s'", name);
-                                       mode = child_cfg->get_mode(child_cfg);
-                                       if (mode == MODE_PASS || mode == MODE_DROP)
-                                       {
+                               switch (child_cfg->get_mode(child_cfg))
+                               {
+                                       case MODE_PASS:
+                                       case MODE_DROP:
                                                charon->shunts->install(charon->shunts,
                                                                                                peer_cfg->get_name(peer_cfg),
                                                                                                child_cfg);
-                                       }
-                                       else
-                                       {
+                                               /* no need to check for ACTION_START */
+                                               continue;
+                                       default:
                                                charon->traps->install(charon->traps, peer_cfg,
                                                                                           child_cfg);
-                                       }
-                                       break;
-                               case ACTION_NONE:
-                                       break;
+                                               break;
+                               }
+                       }
+
+                       if (action & ACTION_START)
+                       {
+                               charon->controller->initiate(charon->controller,
+                                                                                        peer_cfg->get_ref(peer_cfg),
+                                                                                        child_cfg->get_ref(child_cfg),
+                                                                                        NULL, NULL, 0, FALSE);
                        }
                }
                children->destroy(children);
index 3ef5e020b1d8c6eda53cdff25b4e6746b8ed13a4..a9ea02d95512bc94a0b84718f56723a6f813e8e1 100644 (file)
@@ -2060,7 +2060,7 @@ static status_t reestablish_children(private_ike_sa_t *this, ike_sa_t *new,
                }
                if (force)
                {
-                       action = ACTION_RESTART;
+                       action = ACTION_START;
                }
                else
                {       /* only restart CHILD_SAs that are configured accordingly */
@@ -2073,7 +2073,7 @@ static status_t reestablish_children(private_ike_sa_t *this, ike_sa_t *new,
                                action = child_sa->get_dpd_action(child_sa);
                        }
                }
-               if (action == ACTION_RESTART)
+               if (action & ACTION_START)
                {
                        child_init_args_t args = {
                                .reqid = child_sa->get_reqid(child_sa),
@@ -2150,17 +2150,14 @@ METHOD(ike_sa_t, reestablish, status_t,
                        {
                                action = child_sa->get_dpd_action(child_sa);
                        }
-                       switch (action)
+                       if (action & ACTION_TRAP)
                        {
-                               case ACTION_RESTART:
-                                       restart = TRUE;
-                                       break;
-                               case ACTION_ROUTE:
-                                       charon->traps->install(charon->traps, this->peer_cfg,
-                                                                                  child_sa->get_config(child_sa));
-                                       break;
-                               default:
-                                       break;
+                               charon->traps->install(charon->traps, this->peer_cfg,
+                                                                          child_sa->get_config(child_sa));
+                       }
+                       if (action & ACTION_START)
+                       {
+                               restart = TRUE;
                        }
                }
                enumerator->destroy(enumerator);
index ecfc8dda33bf70172aa8239e1e655d67ee0829d9..9d09ee42fd5bda56ff064f5fd9c40e96764ebaaf 100644 (file)
@@ -152,23 +152,21 @@ static status_t delete_child(private_quick_delete_t *this,
                        child_init_args_t args = {
                                .reqid = child_sa->get_reqid(child_sa),
                        };
+                       action_t action;
+
+                       action = child_sa->get_close_action(child_sa);
                        child_cfg = child_sa->get_config(child_sa);
                        child_cfg->get_ref(child_cfg);
-
-                       switch (child_sa->get_close_action(child_sa))
+                       if (action & ACTION_TRAP)
                        {
-                               case ACTION_RESTART:
-                                       child_cfg->get_ref(child_cfg);
-                                       status = this->ike_sa->initiate(this->ike_sa, child_cfg,
-                                                                                                       &args);
-                                       break;
-                               case ACTION_ROUTE:
-                                       charon->traps->install(charon->traps,
-                                                                       this->ike_sa->get_peer_cfg(this->ike_sa),
-                                                                       child_cfg);
-                                       break;
-                               default:
-                                       break;
+                               charon->traps->install(charon->traps,
+                                                                          this->ike_sa->get_peer_cfg(this->ike_sa),
+                                                                          child_cfg);
+                       }
+                       if (action & ACTION_START)
+                       {
+                               child_cfg->get_ref(child_cfg);
+                               status = this->ike_sa->initiate(this->ike_sa, child_cfg, &args);
                        }
                        child_cfg->destroy(child_cfg);
                }
index 5ce251b5ff51f9a22bd0bb33ff795a1f80a11ad1..13cbee3f5bcd1270b2b25988703d7b1119c5738c 100644 (file)
@@ -372,20 +372,16 @@ static status_t destroy_and_reestablish(private_child_delete_t *this)
 
                if (entry->check_delete_action)
                {       /* enforce child_cfg policy if deleted passively */
-                       switch (action)
+                       if (action & ACTION_TRAP)
                        {
-                               case ACTION_RESTART:
-                                       child_cfg->get_ref(child_cfg);
-                                       status = this->ike_sa->initiate(this->ike_sa, child_cfg,
-                                                                                                       &args);
-                                       break;
-                               case ACTION_ROUTE:
-                                       charon->traps->install(charon->traps,
-                                                                       this->ike_sa->get_peer_cfg(this->ike_sa),
-                                                                       child_cfg);
-                                       break;
-                               default:
-                                       break;
+                               charon->traps->install(charon->traps,
+                                                                          this->ike_sa->get_peer_cfg(this->ike_sa),
+                                                                          child_cfg);
+                       }
+                       if (action & ACTION_START)
+                       {
+                               child_cfg->get_ref(child_cfg);
+                               status = this->ike_sa->initiate(this->ike_sa, child_cfg, &args);
                        }
                }
                child_cfg->destroy(child_cfg);