draios / sysdig

Linux system exploration and troubleshooting tool with first class support for containers
http://www.sysdig.com/
Other
7.74k stars 729 forks source link

Please don't use a minor version number of 2 in pcapng files #1444

Open guyharris opened 5 years ago

guyharris commented 5 years ago

Currently, sysdig/userspace/libscap/scap_savefile.h has:

// Major version of the file format supported by this library.
// Must be increased only when if the new version of the software
// is not able anymore to read older captures
#define CURRENT_MAJOR_VERSION   1
// Minor version of the file format supported by this library.
// Must be increased every time we change the capture format
// (e.g. most of the changes in the event table, like adding
// a syscall)
#define CURRENT_MINOR_VERSION   2

If "the file format supported by this library" is pcapng, that is an error, because 1) the minor version of pcapng is determined by the pcapng project, not the sysdig project and 2) the whole point of pcapng's design was to allow as many additions as possible to be made to the file without changing even the minor version number.

To quote the pcapng spec, section 4.1 "Section Header Block":

The current minor version for pcapng is 0, not 2.

Is the format of the blocks you're using is such that most changes to the event table mean that blocks written after the change cannot be read by code written before the change, and it's impossible to avoid this problem? If so, then either:

  1. if you wish to continue to use pcapng as your file format, do not change the major or minor version number in violation of the pcapng specification and, instead, add version numbers to your blocks, so that if the program reading pcapng file has code to read your blocks, that code can deal, as best it can, with different versions of those blocks;

  2. otherwise, if you want your files to be read by programs other than your own programs, choose a different byte-order magic number, so that your file format is different form pcapng.

In either case, you must then take some level of responsibility for code to read your blocks in pcapng files, or your file formats in their entirety. At minimum, this means fully documenting the block and file formats, and notifying the developers of code to read those blocks and files of changes so that they can update their code. It could also involve developing and maintaining that code yourself.

See also issue #1060, over a year old, requesting that the pcapng blocks you've added be documented.

guyharris commented 3 years ago

OK, it appears that version 1.2 did NOT make any incompatible changes to the file format (note: a new block type is NOT an incompatible change, as code unfamiliar with that block type can just ignore it and process whatever stuff it can understand in the file), so:

  1. I've updated the SHB section of the pcapng specification to indicate that writers MUST NOT write a version other than 1.0 and that readers MUST treat version 1.2 identically to version 1.0, and to give a detailed discussion of the version field and what future change, if they ever occur, would result in a major or minor version change, including examples of those types of changes;
  2. I've updated libpcap to treat version 1.2 identically to 1.0 (libpcap currently does not support writing pcapng files; hopefully if Apple picks up those changes, they will write only 1.0 SHBs even if they've read 1.2 SHBs - any future changes we make for full pcapng support will write only 1.0 SHBs), and backported that change to the 1.10 branch;
  3. I've approved Loris's change to treat 1.2 identically to 1.0, and to support V2 block formats (which will be supported regardless of the minor version number, so V2 event blocks will work Just Fine in 1.0 files, set it up so that it was merged into the main branch, and backported it to the 3.4 branch.

This means that any future releases of libpcap and Wireshark 3.4.x or latter will handle 1.2 files the same way 1.0 files are handled.

All code that writes 1.2 SHBs should be fixed to conform to the current pcapng spec and write 1.0 SHBs, so that even older libpcaps, older Wiresharks, and any other code not update to handle version 1.2 will be able to read those files without the code being updated.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

guyharris commented 1 year ago

@geraldcombs: libpcap 1.10 and later will treat 1.2 as 1.0 rather than returning an error and, as noted, Wireshark 3.4 and later do so as well. It may be that those versions are common enough now that continuing to write 1.2 SHBs won't be too big a problem, although commonly-used "enterprise" OS versions might still have older versions of either libpcap or Wireshark, and people might have older desktop Linux distributions and older versions of other OSes, and those might have 1.10 or earlier, so it might still be an issue. (macOS Monterey still has 1.9, although Ventura has 1.10.)