Open
Conversation
Minimal autotools build system Added basic autoconf and automake (but not libtool) build support. Generated files not included. To use, one should first run "autoreconf -is".
Add unit tests Add tests for core functionality as automake TAP tests. To execute, run "make check". Requires support for the linux "veth" virtual ethernet tunnel and permissions required to manipulate such. Effort has been made to make these safe to run on a live system. Tests will only be executed in a private network namespace. In theory, this means that all changes will be properly issolated, but as always, buyer beware. Systems with support for unprivileged containers (unprivileged_userns_clone=1) will allow tests to be run as non-root. Otherwise, root will be required.
More rigorous string manipulation Fix improper use of bounds-checked strncpy and snprintf (return values were ignored). Replace wth input validation and classic strcpy as needed. Replace hand-written buffer management with asprintf(3) appends. Fix off-by-one error in vhid parsing.
Fix erroneous and non-compliant integer conversions Replace improper pointer cast between integer types of differing width. This fixes a bug affecting LP64 64-bit big-endian architectures. Replace BSD legacy integer type aliases with C prmitives, to comply with scanf requirements.
Input validation enhancements Make sure the interface link type is ethernet, since that's all we support. Add bounds-checking for integer values. Add checks for trailing characters on input parameters. Documentation calls for vhid: mac address to be followed by a hexadecimal number, but implementation parsed it as decimal. Split the difference and support hexadecimal when preceded by "0x" and decimal otherwise.
Additional error checking and reporting Treat and report pidfile creation as a fatal error. Report fatal pcap errors. Check for memory allocation failures. Distinguish general runtime errors from invalid user input using exit codes 1 and 2 respectively, in accordance with the LSB init script conventions.
Fix getopt(3) usage errors Since IP addresses/ranges are excluded with the same character as standard options (dash, '-'), GNU getopt(3) will erroneously interpret these as invalid options, unless the user sets the POSIXLY_CORRECT environment variable. Specifying a '+' prefix ensures that these will be treated as regular arguments. With POSIX getopt, option arguments may or may not be in separate argv[] elements. If the user bundles the option argument, such as -p/run/choparp.pid, the option parsing code would skip over the first non-option argument.
Update manual page choparp(8) Document new pidfile feature. Fix misleading vhid: number format mismatch. Replace IP and MAC addresses under assigned prefixes with RFC5737 "TEST-NET" and RFC7042 reserved values for documentation. General attempts at clarification.
Make compile-time diagnostics available at run time. Uses a conventional accumulating "-v" option. Add a conventional "-h" option to print usage.
OpenBSD getopt can return ':' in cases where other implementations would return '?'. Add a default case as a catch-all to avoid undefined behavior (i.e. bugs).
Run test script using shell selected by autoconf instead of /bin/sh Fix recursion check
Owner
|
I see you closed this pull request, but the patches have not been merged yet. Do you plan to resubmit it? |
Author
|
I closed the pull request when I saw the implications of using "master" on my side and didn't see an obvious way to change it to something more sensible. I'd like to make a few more tweaks before resubmitting. I wasn't quite sure you were still interested, actually :-) |
Owner
|
I do appreciate all contributions, and I look forward to your updated submission
…I closed the pull request when I saw the implications of using "master"
on my side and didn't see an obvious way to change it to something more
sensible. I'd like to make a few more tweaks before resubmitting. I
wasn't quite sure you were still interested, actually :-)
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
#6 (comment)
--
Thomas.
|
Test diagnostics improved. Split linux namespace sanbox setup into distinct stages each in separate files. Multiple logical stages are required and coercing them into one souce file required obscure shell features and compromised comprehension. Keep it simple instead.
Author
|
I finally got around to tidying up the tests. I wasn't happy with the contortions put in place to keep the linux-veth tests in one file and decided it didn't make sense anyway. The tests are still linux-only, but since all the linux-specific bits are restricted to the utility functions, it'd be pretty easy to split those out too. I don't know enough about BSD and MacOS network and isolation tools to tackle those platforms. My next set of changes will pull in the LGPL library libdaemon - not sure if you'd want those. I will avoid adding any LGPL source to the repository. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I'm not sure whether you are still actively maintaining this project, but I have some changes you might find useful if you do.
I am afraid that my attempt to carry my quilt patch series into github went slightly awry: please pardon the mess.
unshareandveth