Message ID | 20180702215004.16311-1-qiuyu.xiao.qyx@gmail.com |
---|---|
State | RFC |
Headers | show |
Series | [ovs-dev,RFC] OVN: native support for tunnel encryption | expand |
Bleep bloop. Greetings Qiuyu Xiao, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: ERROR: No signatures found. Lines checked: 237, Warnings: 0, Errors: 1 Please check this out. If you feel there has been an error, please email aconole@bytheb.org Thanks, 0-day Robot
On Mon, Jul 02, 2018 at 02:50:04PM -0700, Qiuyu Xiao wrote: > This patch adds IPsec support for OVN tunnel. Basically, OVN offers a > binary option to its user for encryption configuration. If the IPsec > option is turned on, all tunnels will be encrypted. Otherwise, no tunnel > will be encrypted. > > The changes are summarized as below: > 1) Added a ipsec column on the NB_Global table and SB_Global table. The > value of ipsec column is propagated by ovn-northd from NB_Global to > SB_Global. > > 2) ovn-controller monitors the ipsec column in SB_Global. If the ipsec > value is true, ovn-controller sets options of the tunnel interface by > specifying "options:pki=ca_auth options:local_name=<local_chassis_name> > options:remote_name=<remote_chassis_name>". If the ipsec value is false, > ovn-controller removes these options. > > 3) ovs-monitor-ipsec daemon > (https://mail.openvswitch.org/pipermail/ovs-dev/2018-June/348701.html) > monitors the tunnel interface options and configures IKE daemon > accordingly for IPsec encryption. This is much simpler than I expected. Great. Would you mind adding something, probably to the ovn-architecture document, that explains the purpose for encrypted tunnels and the threat model? You posted a document earlier that might be a good place to start. The ovn-architecture document is in ovn/ovn-architecture.7.xml.
Thanks for the review! I will on adding this documentation soon. -Qiuyu On Tue, Jul 3, 2018 at 1:13 PM, Ben Pfaff <blp@ovn.org> wrote: > On Mon, Jul 02, 2018 at 02:50:04PM -0700, Qiuyu Xiao wrote: >> This patch adds IPsec support for OVN tunnel. Basically, OVN offers a >> binary option to its user for encryption configuration. If the IPsec >> option is turned on, all tunnels will be encrypted. Otherwise, no tunnel >> will be encrypted. >> >> The changes are summarized as below: >> 1) Added a ipsec column on the NB_Global table and SB_Global table. The >> value of ipsec column is propagated by ovn-northd from NB_Global to >> SB_Global. >> >> 2) ovn-controller monitors the ipsec column in SB_Global. If the ipsec >> value is true, ovn-controller sets options of the tunnel interface by >> specifying "options:pki=ca_auth options:local_name=<local_chassis_name> >> options:remote_name=<remote_chassis_name>". If the ipsec value is false, >> ovn-controller removes these options. >> >> 3) ovs-monitor-ipsec daemon >> (https://mail.openvswitch.org/pipermail/ovs-dev/2018-June/348701.html) >> monitors the tunnel interface options and configures IKE daemon >> accordingly for IPsec encryption. > > This is much simpler than I expected. Great. > > Would you mind adding something, probably to the ovn-architecture > document, that explains the purpose for encrypted tunnels and the > threat model? You posted a document earlier that might be a good place > to start. > > The ovn-architecture document is in ovn/ovn-architecture.7.xml.
On Tue, Jul 03, 2018 at 01:13:05PM -0700, Ben Pfaff wrote: > On Mon, Jul 02, 2018 at 02:50:04PM -0700, Qiuyu Xiao wrote: > > This patch adds IPsec support for OVN tunnel. Basically, OVN offers a > > binary option to its user for encryption configuration. If the IPsec > > option is turned on, all tunnels will be encrypted. Otherwise, no tunnel > > will be encrypted. > > > > The changes are summarized as below: > > 1) Added a ipsec column on the NB_Global table and SB_Global table. The > > value of ipsec column is propagated by ovn-northd from NB_Global to > > SB_Global. > > > > 2) ovn-controller monitors the ipsec column in SB_Global. If the ipsec > > value is true, ovn-controller sets options of the tunnel interface by > > specifying "options:pki=ca_auth options:local_name=<local_chassis_name> > > options:remote_name=<remote_chassis_name>". If the ipsec value is false, > > ovn-controller removes these options. > > > > 3) ovs-monitor-ipsec daemon > > (https://mail.openvswitch.org/pipermail/ovs-dev/2018-June/348701.html) > > monitors the tunnel interface options and configures IKE daemon > > accordingly for IPsec encryption. > > This is much simpler than I expected. Great. > > Would you mind adding something, probably to the ovn-architecture > document, that explains the purpose for encrypted tunnels and the > threat model? You posted a document earlier that might be a good place > to start. > > The ovn-architecture document is in ovn/ovn-architecture.7.xml. There was a new suggestion in the OVN meeting morning, which is that it would be valuable to document good ways to verify that encryption is actually working and in use. I suggested using tcpdump or wireshark to see that IPSEC traffic is really flowing, but there may be other or better ways.
Sure. I will document this. "ip xfrm state" also shows whether encryption is taking effect in the kernel. -Qiuyu On Thu, Jul 5, 2018 at 11:11 AM, Ben Pfaff <blp@ovn.org> wrote: > On Tue, Jul 03, 2018 at 01:13:05PM -0700, Ben Pfaff wrote: >> On Mon, Jul 02, 2018 at 02:50:04PM -0700, Qiuyu Xiao wrote: >> > This patch adds IPsec support for OVN tunnel. Basically, OVN offers a >> > binary option to its user for encryption configuration. If the IPsec >> > option is turned on, all tunnels will be encrypted. Otherwise, no tunnel >> > will be encrypted. >> > >> > The changes are summarized as below: >> > 1) Added a ipsec column on the NB_Global table and SB_Global table. The >> > value of ipsec column is propagated by ovn-northd from NB_Global to >> > SB_Global. >> > >> > 2) ovn-controller monitors the ipsec column in SB_Global. If the ipsec >> > value is true, ovn-controller sets options of the tunnel interface by >> > specifying "options:pki=ca_auth options:local_name=<local_chassis_name> >> > options:remote_name=<remote_chassis_name>". If the ipsec value is false, >> > ovn-controller removes these options. >> > >> > 3) ovs-monitor-ipsec daemon >> > (https://mail.openvswitch.org/pipermail/ovs-dev/2018-June/348701.html) >> > monitors the tunnel interface options and configures IKE daemon >> > accordingly for IPsec encryption. >> >> This is much simpler than I expected. Great. >> >> Would you mind adding something, probably to the ovn-architecture >> document, that explains the purpose for encrypted tunnels and the >> threat model? You posted a document earlier that might be a good place >> to start. >> >> The ovn-architecture document is in ovn/ovn-architecture.7.xml. > > There was a new suggestion in the OVN meeting morning, which is that it > would be valuable to document good ways to verify that encryption is > actually working and in use. I suggested using tcpdump or wireshark to > see that IPSEC traffic is really flowing, but there may be other or > better ways.
diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c index fde017586..d122e7c9b 100644 --- a/ovn/controller/encaps.c +++ b/ovn/controller/encaps.c @@ -79,7 +79,8 @@ tunnel_create_name(struct tunnel_ctx *tc, const char *chassis_id) } static void -tunnel_add(struct tunnel_ctx *tc, const char *new_chassis_id, +tunnel_add(struct tunnel_ctx *tc, const struct sbrec_sb_global *sbg, + const char *local_chassis_id, const char *new_chassis_id, const struct sbrec_encap *encap) { struct smap options = SMAP_INITIALIZER(&options); @@ -89,6 +90,12 @@ tunnel_add(struct tunnel_ctx *tc, const char *new_chassis_id, if (csum && (!strcmp(csum, "true") || !strcmp(csum, "false"))) { smap_add(&options, "csum", csum); } + /* Add auth info if ipsec is enabled. */ + if (sbg->ipsec) { + smap_add(&options, "pki", "ca_auth"); + smap_add(&options, "local_name", local_chassis_id); + smap_add(&options, "remote_name", new_chassis_id); + } /* If there's an existing chassis record that does not need any change, * keep it. Otherwise, create a new record (if there was an existing @@ -157,7 +164,8 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, const struct ovsrec_bridge_table *bridge_table, const struct ovsrec_bridge *br_int, const struct sbrec_chassis_table *chassis_table, - const char *chassis_id) + const char *chassis_id, + const struct sbrec_sb_global *sbg) { if (!ovs_idl_txn || !br_int) { return; @@ -209,7 +217,7 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, VLOG_INFO("No supported encaps for '%s'", chassis_rec->name); continue; } - tunnel_add(&tc, chassis_rec->name, encap); + tunnel_add(&tc, sbg, chassis_id, chassis_rec->name, encap); } } diff --git a/ovn/controller/encaps.h b/ovn/controller/encaps.h index 054bdfa78..da12bfc3b 100644 --- a/ovn/controller/encaps.h +++ b/ovn/controller/encaps.h @@ -23,13 +23,16 @@ struct ovsdb_idl_txn; struct ovsrec_bridge; struct ovsrec_bridge_table; struct sbrec_chassis_table; +struct sbrec_sb_global; void encaps_register_ovs_idl(struct ovsdb_idl *); void encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, const struct ovsrec_bridge_table *, const struct ovsrec_bridge *br_int, const struct sbrec_chassis_table *, - const char *chassis_id); + const char *chassis_id, + const struct sbrec_sb_global *); + bool encaps_cleanup(struct ovsdb_idl_txn *ovs_idl_txn, const struct ovsrec_bridge *br_int); diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index 6ee72a9fa..10fbc879c 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -679,7 +679,8 @@ main(int argc, char *argv[]) chassis_id, br_int); encaps_run(ovs_idl_txn, ovsrec_bridge_table_get(ovs_idl_loop.idl), br_int, - sbrec_chassis_table_get(ovnsb_idl_loop.idl), chassis_id); + sbrec_chassis_table_get(ovnsb_idl_loop.idl), chassis_id, + sbrec_sb_global_first(ovnsb_idl_loop.idl)); bfd_calculate_active_tunnels(br_int, &active_tunnels); binding_run(ovnsb_idl_txn, ovs_idl_txn, sbrec_chassis_by_name, sbrec_datapath_binding_by_key, diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index 74eefc6ca..51f1671cd 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -6606,8 +6606,8 @@ ovnnb_db_run(struct northd_context *ctx, } hmap_destroy(&ports); - /* Copy nb_cfg from northbound to southbound database. - * + /* Sync ipsec configuration. + * Copy nb_cfg from northbound to southbound database. * Also set up to update sb_cfg once our southbound transaction commits. */ const struct nbrec_nb_global *nb = nbrec_nb_global_first(ctx->ovnnb_idl); if (!nb) { @@ -6617,6 +6617,9 @@ ovnnb_db_run(struct northd_context *ctx, if (!sb) { sb = sbrec_sb_global_insert(ctx->ovnsb_txn); } + if (nb->ipsec != sb->ipsec) { + sbrec_sb_global_set_ipsec(sb, nb->ipsec); + } sbrec_sb_global_set_nb_cfg(sb, nb->nb_cfg); sb_loop->next_cfg = nb->nb_cfg; @@ -7120,6 +7123,7 @@ main(int argc, char *argv[]) ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_sb_global); add_column_noalert(ovnsb_idl_loop.idl, &sbrec_sb_global_col_nb_cfg); + add_column_noalert(ovnsb_idl_loop.idl, &sbrec_sb_global_col_ipsec); ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_logical_flow); add_column_noalert(ovnsb_idl_loop.idl, diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema index 8e6ddec46..528614efa 100644 --- a/ovn/ovn-nb.ovsschema +++ b/ovn/ovn-nb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Northbound", - "version": "5.11.0", - "cksum": "1149260021 18713", + "version": "5.12.0", + "cksum": "3682343791 18759", "tables": { "NB_Global": { "columns": { @@ -19,7 +19,8 @@ "ssl": { "type": {"key": {"type": "uuid", "refTable": "SSL"}, - "min": 0, "max": 1}}}, + "min": 0, "max": 1}}, + "ipsec": {"type": "boolean"}}, "maxRows": 1, "isRoot": true}, "Logical_Switch": { diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml index 6aed6102a..ddee98cd1 100644 --- a/ovn/ovn-nb.xml +++ b/ovn/ovn-nb.xml @@ -80,6 +80,12 @@ Global SSL configuration. </column> </group> + <group title="Security Configurations"> + <column name="ipsec"> + Tunnel encryption configuration. If this column is set to be true, all + OVN tunnels will be encrypted with IPsec. + </column> + </group> </table> <table name="Logical_Switch" title="L2 logical switch"> diff --git a/ovn/ovn-sb.ovsschema b/ovn/ovn-sb.ovsschema index 9e271d433..9c5c2ef29 100644 --- a/ovn/ovn-sb.ovsschema +++ b/ovn/ovn-sb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Southbound", - "version": "1.15.0", - "cksum": "1839738004 13639", + "version": "1.16.0", + "cksum": "3309568193 13685", "tables": { "SB_Global": { "columns": { @@ -17,7 +17,8 @@ "ssl": { "type": {"key": {"type": "uuid", "refTable": "SSL"}, - "min": 0, "max": 1}}}, + "min": 0, "max": 1}}, + "ipsec": {"type": "boolean"}}, "maxRows": 1, "isRoot": true}, "Chassis": { diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml index b17110e48..4090581fc 100644 --- a/ovn/ovn-sb.xml +++ b/ovn/ovn-sb.xml @@ -174,6 +174,12 @@ Global SSL configuration. </column> </group> + <group title="Security Configurations"> + <column name="ipsec"> + Tunnel encryption configuration. If this column is set to be true, all + OVN tunnels will be encrypted with IPsec. + </column> + </group> </table> <table name="Chassis" title="Physical Network Hypervisor and Gateway Information">