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