Elliott Hughes | e2e3bd1 | 2017-05-15 10:59:29 -0700 | [diff] [blame] | 1 | Some Information for Contributors |
| 2 | --------------------------------- |
Elliott Hughes | 820eced | 2021-08-20 18:00:50 -0700 | [diff] [blame] | 3 | Thank you for considering to make a contribution to tcpdump! Please use the |
| 4 | guidelines below to achieve the best results and experience for everyone. |
Elliott Hughes | e2e3bd1 | 2017-05-15 10:59:29 -0700 | [diff] [blame] | 5 | |
Elliott Hughes | 9a98642 | 2017-12-19 14:49:10 -0800 | [diff] [blame] | 6 | |
| 7 | How to report bugs and other problems |
| 8 | ------------------------------------- |
| 9 | To report a security issue (segfault, buffer overflow, infinite loop, arbitrary |
| 10 | code execution etc) please send an e-mail to security@tcpdump.org, do not use |
| 11 | the bug tracker! |
| 12 | |
| 13 | To report a non-security problem (failure to compile, incorrect output in the |
| 14 | protocol printout, missing support for a particular protocol etc) please check |
| 15 | first that it reproduces with the latest stable release of tcpdump and the latest |
| 16 | stable release of libpcap. If it does, please check that the problem reproduces |
| 17 | with the current git master branch of tcpdump and the current git master branch of |
| 18 | libpcap. If it does (and it is not a security-related problem, otherwise see |
| 19 | above), please navigate to https://github.com/the-tcpdump-group/tcpdump/issues |
| 20 | and check if the problem has already been reported. If it has not, please open |
| 21 | a new issue and provide the following details: |
| 22 | |
| 23 | * tcpdump and libpcap version (tcpdump --version) |
| 24 | * operating system name and version and any other details that may be relevant |
| 25 | (uname -a, compiler name and version, CPU type etc.) |
Elliott Hughes | 820eced | 2021-08-20 18:00:50 -0700 | [diff] [blame] | 26 | * custom configure/CMake flags, if any |
Elliott Hughes | 9a98642 | 2017-12-19 14:49:10 -0800 | [diff] [blame] | 27 | * statement of the problem |
| 28 | * steps to reproduce |
| 29 | |
| 30 | Please note that if you know exactly how to solve the problem and the solution |
| 31 | would not be too intrusive, it would be best to contribute some development time |
Elliott Hughes | 820eced | 2021-08-20 18:00:50 -0700 | [diff] [blame] | 32 | and to open a pull request instead as discussed below. |
Elliott Hughes | 9a98642 | 2017-12-19 14:49:10 -0800 | [diff] [blame] | 33 | |
Elliott Hughes | 820eced | 2021-08-20 18:00:50 -0700 | [diff] [blame] | 34 | Still not sure how to do? Feel free to [subscribe](https://www.tcpdump.org/#mailing-lists) |
Elliott Hughes | 9a98642 | 2017-12-19 14:49:10 -0800 | [diff] [blame] | 35 | to the mailing list tcpdump-workers@lists.tcpdump.org and ask! |
| 36 | |
| 37 | |
| 38 | How to add new code and to update existing code |
| 39 | ----------------------------------------------- |
| 40 | |
| 41 | 0) Check that there isn't a pull request already opened for the changes you |
| 42 | intend to make. |
| 43 | |
Elliott Hughes | e2e3bd1 | 2017-05-15 10:59:29 -0700 | [diff] [blame] | 44 | 1) Fork the Tcpdump repository on GitHub from |
| 45 | https://github.com/the-tcpdump-group/tcpdump |
| 46 | (See https://help.github.com/articles/fork-a-repo/) |
| 47 | |
Elliott Hughes | 820eced | 2021-08-20 18:00:50 -0700 | [diff] [blame] | 48 | 2) The easiest way to test your changes on multiple operating systems and |
| 49 | architectures is to let the upstream CI test your pull request (more on |
| 50 | this below). |
Elliott Hughes | e2e3bd1 | 2017-05-15 10:59:29 -0700 | [diff] [blame] | 51 | |
Elliott Hughes | 9a98642 | 2017-12-19 14:49:10 -0800 | [diff] [blame] | 52 | 3) Setup your git working copy |
Elliott Hughes | e2e3bd1 | 2017-05-15 10:59:29 -0700 | [diff] [blame] | 53 | git clone https://github.com/<username>/tcpdump.git |
Elliott Hughes | 9a98642 | 2017-12-19 14:49:10 -0800 | [diff] [blame] | 54 | cd tcpdump |
| 55 | git remote add upstream https://github.com/the-tcpdump-group/tcpdump |
| 56 | git fetch upstream |
Elliott Hughes | e2e3bd1 | 2017-05-15 10:59:29 -0700 | [diff] [blame] | 57 | |
| 58 | 4) Do a 'touch .devel' in your working directory. |
| 59 | Currently, the effect is |
| 60 | a) add (via configure, in Makefile) some warnings options ( -Wall |
| 61 | -Wmissing-prototypes -Wstrict-prototypes, ...) to the compiler if it |
| 62 | supports these options, |
| 63 | b) have the Makefile support "make depend" and the configure script run it. |
| 64 | |
| 65 | 5) Configure and build |
| 66 | ./configure && make -s && make check |
| 67 | |
Elliott Hughes | 820eced | 2021-08-20 18:00:50 -0700 | [diff] [blame] | 68 | 6) Add/update tests |
| 69 | The tests directory contains regression tests of the dissection of captured |
| 70 | packets. Those captured packets were saved running tcpdump with option "-w |
| 71 | sample.pcap". Additional options, such as "-n", are used to create relevant |
| 72 | and reproducible output; "-#" is used to indicate which particular packets |
| 73 | have output that differs. The tests are run with the TZ environment |
| 74 | variable set to GMT0, so that UTC, rather than the local time where the |
| 75 | tests are being run, is used when "local time" values are printed. The |
| 76 | actual test compares the current text output with the expected result |
| 77 | (sample.out) saved from a previous version. |
Elliott Hughes | e2e3bd1 | 2017-05-15 10:59:29 -0700 | [diff] [blame] | 78 | |
| 79 | Any new/updated fields in a dissector must be present in a sample.pcap file |
| 80 | and the corresponding output file. |
| 81 | |
| 82 | Configuration is set in tests/TESTLIST. |
| 83 | Each line in this file has the following format: |
| 84 | test-name sample.pcap sample.out tcpdump-options |
| 85 | |
Elliott Hughes | 820eced | 2021-08-20 18:00:50 -0700 | [diff] [blame] | 86 | The sample.out file can be produced as follows: |
| 87 | (cd tests && TZ=GMT0 ../tcpdump -# -n -r sample.pcap tcpdump-options > sample.out) |
| 88 | |
| 89 | Or, for convenience, use "./update-test.sh test-name" |
Elliott Hughes | e2e3bd1 | 2017-05-15 10:59:29 -0700 | [diff] [blame] | 90 | |
| 91 | It is often useful to have test outputs with different verbosity levels |
| 92 | (none, -v, -vv, -vvv, etc.) depending on the code. |
| 93 | |
| 94 | 7) Test with 'make check' |
| 95 | Don't send a pull request if 'make check' gives failed tests. |
| 96 | |
Elliott Hughes | 9a98642 | 2017-12-19 14:49:10 -0800 | [diff] [blame] | 97 | 8) Try to rebase your commits to keep the history simple. |
| 98 | git rebase upstream/master |
| 99 | (If the rebase fails and you cannot resolve, issue "git rebase --abort" |
| 100 | and ask for help in the pull request comment.) |
Elliott Hughes | e2e3bd1 | 2017-05-15 10:59:29 -0700 | [diff] [blame] | 101 | |
Elliott Hughes | 9a98642 | 2017-12-19 14:49:10 -0800 | [diff] [blame] | 102 | 9) Once 100% happy, put your work into your forked repository. |
| 103 | git push |
| 104 | |
| 105 | 10) Initiate and send a pull request |
Elliott Hughes | 820eced | 2021-08-20 18:00:50 -0700 | [diff] [blame] | 106 | (See https://help.github.com/articles/using-pull-requests/) |
| 107 | This will trigger the upstream repository CI tests. |
Elliott Hughes | e2e3bd1 | 2017-05-15 10:59:29 -0700 | [diff] [blame] | 108 | |
Elliott Hughes | 9a98642 | 2017-12-19 14:49:10 -0800 | [diff] [blame] | 109 | |
| 110 | Code style and generic remarks |
| 111 | ------------------------------ |
Elliott Hughes | e2e3bd1 | 2017-05-15 10:59:29 -0700 | [diff] [blame] | 112 | a) A thorough reading of some other printers code is useful. |
| 113 | |
| 114 | b) Put the normative reference if any as comments (RFC, etc.). |
| 115 | |
Elliott Hughes | 9a98642 | 2017-12-19 14:49:10 -0800 | [diff] [blame] | 116 | c) Put the format of packets/headers/options as comments if there is no |
| 117 | published normative reference. |
Elliott Hughes | e2e3bd1 | 2017-05-15 10:59:29 -0700 | [diff] [blame] | 118 | |
| 119 | d) The printer may receive incomplete packet in the buffer, truncated at any |
| 120 | random position, for example by capturing with '-s size' option. |
Elliott Hughes | 820eced | 2021-08-20 18:00:50 -0700 | [diff] [blame] | 121 | If your code reads and decodes every byte of the protocol packet, then to |
| 122 | ensure proper and complete bounds checks it would be sufficient to read all |
| 123 | packet data using the GET_*() macros, typically: |
| 124 | GET_U_1(p) |
| 125 | GET_S_1(p) |
| 126 | GET_BE_U_n(p), n in { 2, 3, 4, 5, 6, 7, 8 } |
| 127 | GET_BE_S_n(p), n in { 2, 3, 4, 5, 6, 7, 8 } |
| 128 | If your code uses the macros above only on some packet data, then the gaps |
| 129 | would have to be bounds-checked using the ND_TCHECK_*() macros: |
| 130 | ND_TCHECK_n(p), n in { 1, 2, 3, 4, 5, 6, 7, 8, 16 } |
| 131 | ND_TCHECK_SIZE(p) |
| 132 | ND_TCHECK_LEN(p, l) |
| 133 | For the ND_TCHECK_* macros (if not already done): |
| 134 | Assign: ndo->ndo_protocol = "protocol"; |
| 135 | Define: ND_LONGJMP_FROM_TCHECK before including netdissect.h |
| 136 | Make sure that the intersection of GET_*() and ND_TCHECK_*() is minimal, |
| 137 | but at the same time their union covers all packet data in all cases. |
Elliott Hughes | e2e3bd1 | 2017-05-15 10:59:29 -0700 | [diff] [blame] | 138 | You can test the code via: |
| 139 | sudo ./tcpdump -s snaplen [-v][v][...] -i lo # in a terminal |
| 140 | sudo tcpreplay -i lo sample.pcap # in another terminal |
| 141 | You should try several values for snaplen to do various truncation. |
| 142 | |
| 143 | e) Do invalid packet checks in code: Think that your code can receive in input |
| 144 | not only a valid packet but any arbitrary random sequence of octets (packet |
| 145 | - built malformed originally by the sender or by a fuzz tester, |
Elliott Hughes | 820eced | 2021-08-20 18:00:50 -0700 | [diff] [blame] | 146 | - became corrupted in transit or for some other reason). |
| 147 | Print with: nd_print_invalid(ndo); /* to print " (invalid)" */ |
Elliott Hughes | e2e3bd1 | 2017-05-15 10:59:29 -0700 | [diff] [blame] | 148 | |
| 149 | f) Use 'struct tok' for indexed strings and print them with |
| 150 | tok2str() or bittok2str() (for flags). |
| 151 | |
| 152 | g) Avoid empty lines in output of printers. |
| 153 | |
| 154 | h) A commit message must have: |
| 155 | First line: Capitalized short summary in the imperative (70 chars or less) |
| 156 | |
| 157 | Body: Detailed explanatory text, if necessary. Fold it to approximately |
| 158 | 72 characters. There must be an empty line separating the summary from |
| 159 | the body. |
| 160 | |
| 161 | i) Avoid non-ASCII characters in code and commit messages. |
| 162 | |
| 163 | j) Use the style of the modified sources. |
| 164 | |
| 165 | k) Don't mix declarations and code |
| 166 | |
| 167 | l) Don't use // for comments |
| 168 | Not all C compilers accept C++/C99 comments by default. |
| 169 | |
| 170 | m) Avoid trailing tabs/spaces |