From 341c4e59f50a842a2974d06e448a57af372a7edd Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 5 Sep 2012 10:35:20 -0700 Subject: [PATCH] ovsdb: Enforce immutability of immutable columns. OVSDB has always had the ability to mark a column as "immutable", so that its value cannot be changed in a given row after that row is initially inserted. However, we discovered recently that ovsdb-server has never enforced this constraint. This commit implements enforcement. Reported-by: Paul Ingram Signed-off-by: Ben Pfaff Acked-by: Kyle Mestery --- NEWS | 2 ++ lib/ovsdb-idl-provider.h | 3 ++- ovsdb/execution.c | 18 ++++++++++++++++- ovsdb/mutation.c | 8 +++++++- ovsdb/ovsdb-idlc.in | 5 +++++ tests/ovs-vsctl.at | 12 +++++++++++ tests/ovsdb-execution.at | 43 ++++++++++++++++++++++++++++++++++++++++ tests/ovsdb-mutation.at | 12 +++++++++++ utilities/ovs-vsctl.c | 22 ++++++++++++++++++-- 9 files changed, 120 insertions(+), 5 deletions(-) diff --git a/NEWS b/NEWS index 872f8d0d8..cbc5c5862 100644 --- a/NEWS +++ b/NEWS @@ -18,6 +18,8 @@ post-v1.8.0 are true, but because we do not know of any users for this feature it seems better on balance to remove it. (The ovs-pki-cgi program was not included in distribution packaging.) + - ovsdb-server now enforces the immutability of immutable columns. This + was not enforced in earlier versions due to an oversight. - Stable bond mode is deprecated and will be removed no earlier than February 2013. Please email dev@openvswitch.org with concerns. - The autopath action is deprecated and will be removed no earlier than diff --git a/lib/ovsdb-idl-provider.h b/lib/ovsdb-idl-provider.h index 546acbb84..7ea5ce690 100644 --- a/lib/ovsdb-idl-provider.h +++ b/lib/ovsdb-idl-provider.h @@ -1,4 +1,4 @@ -/* Copyright (c) 2009, 2010, 2011 Nicira, Inc. +/* Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -41,6 +41,7 @@ struct ovsdb_idl_row { struct ovsdb_idl_column { char *name; struct ovsdb_type type; + bool mutable; void (*parse)(struct ovsdb_idl_row *, const struct ovsdb_datum *); void (*unparse)(struct ovsdb_idl_row *); }; diff --git a/ovsdb/execution.c b/ovsdb/execution.c index 1aff0c51e..300c247a3 100644 --- a/ovsdb/execution.c +++ b/ovsdb/execution.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2009, 2010, 2011 Nicira, Inc. +/* Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -434,6 +434,22 @@ ovsdb_execute_update(struct ovsdb_execution *x, struct ovsdb_parser *parser, if (!error) { error = parse_row(row_json, table, x->symtab, &row, &columns); } + if (!error) { + size_t i; + + for (i = 0; i < columns.n_columns; i++) { + const struct ovsdb_column *column = columns.columns[i]; + + if (!column->mutable) { + error = ovsdb_syntax_error(parser->json, + "constraint violation", + "Cannot update immutable column %s " + "in table %s.", + column->name, table->schema->name); + break; + } + } + } if (!error) { error = ovsdb_condition_from_json(table->schema, where, x->symtab, &condition); diff --git a/ovsdb/mutation.c b/ovsdb/mutation.c index 0dcd16fec..5fd983a4b 100644 --- a/ovsdb/mutation.c +++ b/ovsdb/mutation.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2009, 2010, 2011 Nicira, Inc. +/* Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -95,6 +95,12 @@ ovsdb_mutation_from_json(const struct ovsdb_table_schema *ts, "No column %s in table %s.", column_name, ts->name); } + if (!m->column->mutable) { + return ovsdb_syntax_error(json, "constraint violation", + "Cannot mutate immutable column %s in " + "table %s.", column_name, ts->name); + } + ovsdb_type_clone(&m->type, &m->column->type); mutator_name = json_string(array->elems[1]); diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in index 0b1933b35..478109af6 100755 --- a/ovsdb/ovsdb-idlc.in +++ b/ovsdb/ovsdb-idlc.in @@ -577,11 +577,16 @@ static void\n%s_columns_init(void) for columnName, column in sorted(table.columns.iteritems()): cs = "%s_col_%s" % (structName, columnName) d = {'cs': cs, 'c': columnName, 's': structName} + if column.mutable: + mutable = "true" + else: + mutable = "false" print print " /* Initialize %(cs)s. */" % d print " c = &%(cs)s;" % d print " c->name = \"%(c)s\";" % d print column.type.cInitType(" ", "c->type") + print " c->mutable = %s;" % mutable print " c->parse = %(s)s_parse_%(c)s;" % d print " c->unparse = %(s)s_unparse_%(c)s;" % d print "}" diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at index 84cc272d7..e90361915 100644 --- a/tests/ovs-vsctl.at +++ b/tests/ovs-vsctl.at @@ -736,6 +736,18 @@ AT_CHECK([RUN_OVS_VSCTL([clear netflow `cat netflow-uuid` targets])], AT_CHECK([RUN_OVS_VSCTL([destroy b br2])], [1], [], [ovs-vsctl: no row "br2" in table Bridge ], [OVS_VSCTL_CLEANUP]) +AT_CHECK([RUN_OVS_VSCTL([add i br1 name x])], + [1], [], [ovs-vsctl: cannot modify read-only column name in table Interface +], [OVS_VSCTL_CLEANUP]) +AT_CHECK([RUN_OVS_VSCTL([set port br1 name br2])], + [1], [], [ovs-vsctl: cannot modify read-only column name in table Port +], [OVS_VSCTL_CLEANUP]) +AT_CHECK([RUN_OVS_VSCTL([remove b br1 name br1])], + [1], [], [ovs-vsctl: cannot modify read-only column name in table Bridge +], [OVS_VSCTL_CLEANUP]) +AT_CHECK([RUN_OVS_VSCTL([clear b br1 name])], + [1], [], [ovs-vsctl: cannot modify read-only column name in table Bridge +], [OVS_VSCTL_CLEANUP]) OVS_VSCTL_CLEANUP AT_CLEANUP diff --git a/tests/ovsdb-execution.at b/tests/ovsdb-execution.at index 480f61016..6a3b5d157 100644 --- a/tests/ovsdb-execution.at +++ b/tests/ovsdb-execution.at @@ -111,6 +111,15 @@ gc_schema () { "isRoot": false}}} EOF } + +immutable_schema () { + cat <<'EOF' +{"name": "immutable", + "tables": { + "a": { + "columns": {"i": {"type": "integer", "mutable": false}}}}} +EOF +} ] m4_divert_pop([PREPARE_TESTS]) @@ -908,6 +917,40 @@ OVSDB_CHECK_EXECUTION([weak references], [{"rows":[{"_uuid":["uuid","<3>"],"b":2,"b2a":["set",[]]},{"_uuid":["uuid","<4>"],"b":3,"b2a":["set",[]]}]}] ]]) +OVSDB_CHECK_EXECUTION([immutable columns], + [immutable_schema], + [[[["immutable", + {"op": "insert", + "table": "a", + "row": {"i": 5}, + "uuid-name": "row1"}]]], + [[["immutable", + {"op": "update", + "table": "a", + "row": {"i": 10}, + "where": []}]]], + [[["immutable", + {"op": "update", + "table": "a", + "row": {"i": 5}, + "where": []}]]], + [[["immutable", + {"op": "mutate", + "table": "a", + "where": [], + "mutations": [["i", "-=", 5]]}]]], + [[["immutable", + {"op": "mutate", + "table": "a", + "where": [], + "mutations": [["i", "*=", 1]]}]]]], + [[[{"uuid":["uuid","<0>"]}] +[{"details":"Cannot update immutable column i in table a.","error":"constraint violation","syntax":"{\"op\":\"update\",\"row\":{\"i\":10},\"table\":\"a\",\"where\":[]}"}] +[{"details":"Cannot update immutable column i in table a.","error":"constraint violation","syntax":"{\"op\":\"update\",\"row\":{\"i\":5},\"table\":\"a\",\"where\":[]}"}] +[{"details":"Cannot mutate immutable column i in table a.","error":"constraint violation","syntax":"[\"i\",\"-=\",5]"}] +[{"details":"Cannot mutate immutable column i in table a.","error":"constraint violation","syntax":"[\"i\",\"*=\",1]"}] +]]) + OVSDB_CHECK_EXECUTION([garbage collection], [gc_schema], [dnl Check that inserting a row without any references is a no-op. diff --git a/tests/ovsdb-mutation.at b/tests/ovsdb-mutation.at index 3d753176d..fc898b56d 100644 --- a/tests/ovsdb-mutation.at +++ b/tests/ovsdb-mutation.at @@ -99,6 +99,18 @@ test-ovsdb: syntax "["u","delete",["uuid","9179ca6d-6d65-400a-b455-3ad92783a099" ]]) AT_CLEANUP +AT_SETUP([disallowed mutations on immutable columns]) +AT_KEYWORDS([ovsdb negative mutation]) +AT_CHECK([[test-ovsdb parse-mutations \ + '{"columns": + {"i": {"type": "integer", "mutable": false}}}' \ + '[["i", "+=", 1]]' +]], + [1], [], + [[test-ovsdb: syntax "["i","+=",1]": constraint violation: Cannot mutate immutable column i in table mytable. +]]) +AT_CLEANUP + OVSDB_CHECK_POSITIVE([mutations on sets], [[parse-mutations \ '{"columns": diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c index 0a7c2c64e..e6bd63bd2 100644 --- a/utilities/ovs-vsctl.c +++ b/utilities/ovs-vsctl.c @@ -2763,7 +2763,7 @@ error: return error; } -static void +static const struct ovsdb_idl_column * pre_parse_column_key_value(struct vsctl_context *ctx, const char *arg, const struct vsctl_table_class *table) @@ -2780,6 +2780,18 @@ pre_parse_column_key_value(struct vsctl_context *ctx, pre_get_column(ctx, table, column_name, &column); free(column_name); + + return column; +} + +static void +check_mutable(const struct vsctl_table_class *table, + const struct ovsdb_idl_column *column) +{ + if (!column->mutable) { + vsctl_fatal("cannot modify read-only column %s in table %s", + column->name, table->class->name); + } } static void @@ -3112,7 +3124,10 @@ pre_cmd_set(struct vsctl_context *ctx) table = pre_get_table(ctx, table_name); for (i = 3; i < ctx->argc; i++) { - pre_parse_column_key_value(ctx, ctx->argv[i], table); + const struct ovsdb_idl_column *column; + + column = pre_parse_column_key_value(ctx, ctx->argv[i], table); + check_mutable(table, column); } } @@ -3195,6 +3210,7 @@ pre_cmd_add(struct vsctl_context *ctx) table = pre_get_table(ctx, table_name); pre_get_column(ctx, table, column_name, &column); + check_mutable(table, column); } static void @@ -3251,6 +3267,7 @@ pre_cmd_remove(struct vsctl_context *ctx) table = pre_get_table(ctx, table_name); pre_get_column(ctx, table, column_name, &column); + check_mutable(table, column); } static void @@ -3316,6 +3333,7 @@ pre_cmd_clear(struct vsctl_context *ctx) const struct ovsdb_idl_column *column; pre_get_column(ctx, table, ctx->argv[i], &column); + check_mutable(table, column); } } -- 2.43.0