Commit Graph

17 Commits

Author SHA1 Message Date
Neels Hofmeyr 5314c513f2 vty: fix use-after-free and memleaks in is_cmd_ambiguous()
vty_test: add test against ambiguous cmd causing use-after-free and memory
leaks. Add this test along with the fix, because the new test triggers the
memory use-after-free and leaks, causing build failures.

Add cmd_deopt_with_ctx() to allow passing a specific talloc ctx.

is_cmd_ambiguous(): keep all cmd_deopt() allocations until the function exits.
Add a comment explaining why. Before this, if a command matched an optional
"[arg]" with square brackets, we would keep it in local var 'matched', but we
would free the string it points to at the end of that loop iteration; upon
encountering another match, we would attempt to strcmp against the freed
'matched'. Instead of adding hard-to-read and -verify free/alloc dances to keep
the 'matched' accurately freed/non-freed/..., just keep all cmd_deopt() string
allocated until done.

Needless to say that this should have been implemented on a lower level upon
inventing optional args, but at least this is fixing a program crash.

Related: OS#33903390
Change-Id: Ia71ba742108b5ff020997bfb612ad5eb30d04fcd
2018-07-11 15:47:08 +02:00
Neels Hofmeyr a829b45c85 use osmo_init_logging2() with proper talloc ctx
Ironically, when deprecating osmo_init_logging() in
I216837780e9405fdaec8059c63d10699c695b360, I forgot to change the callers
within libosmocore itself, i.e. in the various regression tests.

Change-Id: Ia36c248f99353d5baaa2533f46a2f60a8579bdf8
2018-04-06 04:37:50 +02:00
Harald Welte e08da97570 Fix/Update copyright notices; Add SPDX annotation
Let's fix some erroneous/accidential references to wrong license,
update copyright information where applicable and introduce a
SPDX-License-Identifier to all files.

Change-Id: I39af26c6aaaf5c926966391f6565fc5936be21af
2017-11-13 01:35:12 +09:00
Neels Hofmeyr f4f23bd682 vty: install 'exit', 'end',... commands on *all* nodes
In many callers of the VTY API, we are lacking the vty_install_default() step
at certain node levels. This creates nodes that lack the 'exit' command, and
hence the only way to exit such a node is to restart the telnet session.

Historically, the VTY looked for missing commands on the immediate parent node,
and hence possibly found the parent's 'exit' command when the local node was
missing it. That is why we so far did not notice the missing default commands.

Furthermore, some callers call install_default() instead of
vty_install_default(). Only vty_install_default() also includes the 'exit' and
'end' commands. There is no reason why there are two sets of default commands.

To end this confusion, to catch all missing 'exit' commands and to prevent this
from re-appearing in the future, simply *always* install all default commands
implicitly when calling install_node().

In cmd_init(), there are some top-level nodes that apparently do not want the
default commands installed. Keep those the way they are, by changing the
invocation to new install_node_bare() ({VIEW,AUTH,AUTH_ENABLE}_NODE).

Make both install_default() and vty_install_default() no-ops so that users of
the API may still call them without harm. Do not yet deprecate yet, which
follows in Icf5d83f641e838cebcccc635a043e94ba352abff.

Drop all invocations to these two functions found in libosmocore.

Change-Id: I5021c64a787b63314e0f2f1cba0b8fc7bff4f09b
2017-09-27 14:04:09 +00:00
Neels Hofmeyr b022c867e8 vty_test: add artificial node levels for better testing
In vty_test, add three levels of parent nodes (level1, level2, level3) with
each having a leaf child (child1, child2, child3).

Use these to enhance the vty_test cfg files and test more diverse situations.

The current VTY code expects a go_parent_cb() to be present, otherwise it will
bump right back to the CONFIG_NODE, which will not work with more than one node
level below the CONFIG_NODE. Hence provide a minimal go_parent_cb().

Change-Id: Ib9bcf58b655fbd85e196f363fb7d8305d7dfc997
2017-09-20 03:32:24 +02:00
Neels Hofmeyr 430636328c fix vty regression: empty parent node
The recent exit-by-indent patch breaks a VTY case where a node is entered but
directly followed by a sibling or ancestor without listing any child nodes.
Regression introduced by I24cbb3f6de111f2d31110c3c484c066f1153aac9.

An example is a common usage in osmo-bts, where 'phy N' / 'instance N' is a
parent node that is commonly left empty:

	phy 0
	 instance 0
	bts 0
	 band 1800

Before this patch, this case produces the error:

	There is no such command.
	Error occurred during reading the below line:
	bts 0

Fix indentation parsing logic in command.c to accomodate this case.

Add a unit test for empty parent node.

Change-Id: Ia0880a17ae55accb092ae8585cc3a1bec9986891
2017-09-20 00:57:33 +02:00
Neels Hofmeyr 4a31ffa2f0 VTY: implicit node exit by de-indenting, not parent lookup
Note: This will break users' config files if they do not use consistent
indenting. (see below for a definition of "consistent".)

When reading VTY commands from a file, use indenting as means to implicitly
exit child nodes. Do not look for commands in the parent node implicitly.

The VTY so far implies 'exit' commands if a VTY line cannot be parsed on the
current node, but succeeds on the parent node. That is the mechanism by which
our VTY config files do not need 'exit' at the end of each child node.

We've hit problems with this in the following scenarios, which will show
improved user experience after this patch:

*) When both a parent and its child node have commands with identical names:

  cs7 instace 0
   point-code 1.2.3
   sccp-address osmo-msc
    point-code 0.0.1

If I put the parent's command below the child, it is still interpreted in the
context of the child node:

  cs7 instace 0
   sccp-address osmo-msc
    point-code 0.0.1
   point-code 1.2.3

Though the indenting lets me assume I am setting the cs7 instance's global PC
to 1.2.3, I'm actually overwriting osmo-msc's PC with 1.2.3 and discarding the
0.0.1.

*) When a software change moves a VTY command from a child to a parent. Say
'timezone' moved from 'bts' to 'network' level:

  network
   timezone 1 2

Say a user still has an old config file with 'timezone' on the child level:

  network
   bts 0
    timezone 1 2
    trx 0

The user would expect an error message that 'timezone' is invalid on the 'bts'
level. Instead, the VTY finds the parent node's 'timezone', steps out of 'bts'
to the 'network' level, and instead says that the 'trx' command does not exist.

Format:

Consistent means that two adjacent indenting lines have the exact
same indenting characters for the common length:

Weird mix if you ask me, but correct and consistent:

  ROOT
  <space>PARENT
  <space><tab><space>CHILD
  <space><tab><space><tab><tab>GRANDCHILD
  <space><tab><space><tab><tab>GRANDCHILD2
  <space>SIBLING

Inconsistent:

  ROOT
  <space>PARENT
  <tab><space>CHILD
  <space><space><tab>GRANDCHILD
  <space><tab><tab>GRANDCHILD2
  <tab>SIBLING

Also, when going back to a parent level, the exact same indenting must be used
as before in that node:

Incorrect:

  ROOT
  <tab>PARENT
  <tab><tab><tab>CHILD
  <tab><tab>SIBLING

As not really intended side effect, it is also permitted to indent the entire
file starting from the root level. We could guard against it but there's no
harm:

Correct and consistent:

  <tab>ROOT
  <tab><tab>PARENT
  <tab><tab><tab><tab>CHILD
  <tab><tab>SIBLING

Implementation:

Track parent nodes state: whenever a command enters a child node, push a parent
node onto an llist to remember the exact indentation characters used for that
level.

As soon as the first line on a child node is parsed, remember this new
indentation (which must have a longer strlen() than its parent level) to apply
to all remaining child siblings and grandchildren.

If the amount of spaces that indent a following VTY command are less than this
expected indentation, call vty_go_parent() until it matches up.

At any level, if the common length of indentation characters mismatch, abort
parsing in error.

Transitions to child node are spread across VTY implementations and are hard to
change. But transitions to the parent node are all handled by vty_go_parent().
By popping a parent from the list of parents in vty_go_parent(), we can also
detect that a command has changed the node without changing the parent, hence
it must have stepped into a child node, and we can push a parent frame.

The behavior on the interactive telnet VTY remains unchanged.

Change-Id: I24cbb3f6de111f2d31110c3c484c066f1153aac9
2017-09-19 01:35:30 +00:00
Neels Hofmeyr d64b6aed23 VTY: interactive: never look for matching commands on parent node
For interactive telnet VTY, remove the implicit move up to the parent node when
a command did not succeed on the current node level.

When reading config files, this behavior was useful to allow skipping explicit
'exit' commands. (A different patch deals with that.)

In the telnet VTY, this behavior was never necessary. Explicit 'exit' commands
can move to the parent node, and typically uninformed users expect to require
that.

On a telnet VTY, counting indents like for reading config files is not an
option: a user will always type from the first column or may paste some leading
spaces without intended meaning.

After this patch, it is thus no longer possible to paste a complete config
across several node levels directly to a telnet session, unless it contains
'exit' commands.

Change-Id: Id73cba2dd34676bad8a130e9c45e67a272f19588
2017-09-08 23:45:52 +00:00
Max c65c5b4ea0 vty: cleanup logging functions
* remove unused parameter from logging_vty_add_cmds()
* mark log level descriptors static
* change internal static function int check_log_to_target() to more
  appropriate bool should_log_to_target()
* deprecate log_vty_command_*() from public API as it should only be
  used by logging_vty_add_cmds()

Change-Id: I0e9ddd7ba3ce211302d99a3494eb408907a2916e
Related: OS#71
2017-05-09 09:11:05 +00:00
Jacob Erlbeck be37fb7db4 stats/test: Add tests to check VTY configuration
This commit adds tests to verify the stats related VTY configuration
commands.

Sponsored-by: On-Waves ehf
2015-08-22 01:34:05 +00:00
Jacob Erlbeck ca6602f476 vty/test: Refactor vty creation/deletion into separate functions
Currently this is part of the only test function that uses the
vty directly.

In preperation for more such test cases, this commit moves this code
into separate functions.

Sponsored-by: On-Waves ehf
2015-08-22 01:31:31 +00:00
Jacob Erlbeck adc900e0e3 stats/vty: Add stats configuration
This commit provides stats configuration similar to the log
configuration.

The following vty commands are added to the config node:
  stats reporter statsd          Create/Modify a statsd reporter
  no stats reporter statsd       Remove a statsd reporter

To actually configure a reporter, the config-stats node is entered
when the "stats reporter" command has succeeded. The following new
vty commands are available there:
  local-ip ADDR          Set the IP address to which we bind locally
  no local-ip            Do not bind to a certain IP address
  remote-ip ADDR         Set the remote IP address to which we connect
  remote-port <1-65535>  Set the remote port to which we connect
  prefix PREFIX          Set the item/counter name prefix
  no prefix              Do not use a prefix
  enable                 Enable the reporter
  disable                Disable the reporter

Sponsored-by: On-Waves ehf
2015-10-29 01:10:06 +01:00
Holger Hans Peter Freyther 2c9168cf34 vty: Make vty_event dispatch signals and use it in the testcase
The testcase didn't work on Ubuntu 12.04 because vty_create will
directly call vty_event (e.g. not through the plt). This means
that the approach to override vty_event in the testcase failed.

Use the signal interface of libosmocore and make the testcase
use it. The signals can be generally useful as well.
2013-10-10 20:21:33 +02:00
Holger Hans Peter Freyther 6ef71b062b vty: Fix compiler warning in the test 2013-09-10 11:17:46 +02:00
Jacob Erlbeck 0c987bd83b vty: Add vty_install_default() and use for the vty nodes
This adds the vty_install_default() function that is basically the
install_default() function plus the registration of the commands
'exit' and 'end'. The latter is only provided in subnodes of
ENABLED_NODE and CONFIG_NONE.

The VTY test program is extended to check these commands.

Ticket: OW#952
2013-09-08 10:49:52 +02:00
Jacob Erlbeck cd195fa267 vty: Support multi-char separators and end strings
In vty_cmd_string_from_valstr() include the real string lengths of
the sep and end arguments into the buffer size calculation.
2013-08-06 14:56:35 +02:00
Jacob Erlbeck ae15a2cac1 vty: Fix misusage of snprintf in vty/utils.c
Compiled with ubuntu 1204 (precise), where -Wformat-security is enabled by
-Wall.

Test yields ok, but the current implementation doesn't properly support
multi-character separators and end strings. So the test output is truncated.

Addresses:
utils.c: In function 'vty_cmd_string_from_valstr':
utils.c:84:2: warning: format not a string literal and no format arguments [-Wformat-security]
utils.c:84:2: warning: format not a string literal and no format arguments [-Wformat-security]
utils.c:108:2: warning: format not a string literal and no format arguments [-Wformat-security]
utils.c:108:2: warning: format not a string literal and no format arguments [-Wformat-security]
2013-08-06 14:56:30 +02:00