Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extras: allow options in journal export block #122

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

eqvinox
Copy link
Contributor

@eqvinox eqvinox commented Sep 1, 2022

Can't currently put options on captured log messages... (Also adding a nanosecond timestamp in the next commit.)


Really want to add a comment on some log messages 😄

I'm fully aware wireshark (aka the primary pcap-ng editing tool) doesn't support this yet, I just feel like specifying out things before implementing them. I may code this up for wireshark in a bit. It currently ignores the "trailing garbage" (my generator already tries generating comments because I didn't check that the journal block actually supports options…)

I'm 99.9% sure this shouldn't cause any compatibility problems; any reader or writer unaware of the options would still process the blocks correctly. Even if a writer was previously generating "excessive" padding (0…3 + n*4 zero bytes), it would just be read as end-of-options. Doing it this way should make existing readers just ignore the options.

No idea how to make the cross-document reference to section_opt work 😞

Copy link
Collaborator

@mcr mcr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rebase and fix the TBD?

Can't currently put options on captured log messages...
(Also adding a nanosecond timestamp in the next commit.)
@eqvinox
Copy link
Contributor Author

eqvinox commented Jul 23, 2023

Can you rebase and fix the TBD?

done

@guyharris
Copy link
Collaborator

A URL change needs to be made:

@guyharris
Copy link
Collaborator

If the block contains options, there MUST be at least one byte of zero padding present to mark the end of the journal entry. This only makes a difference if the journal entry is a multiple of four octets long, in this case 4 bytes of zero padding MUST be appended. Blocks without options do not contain any zero padding if the journal entry is a multiple of 4 octets long, therefore readers MUST NOT rely on the presence of a zero byte to terminate the entry.

So what is the algorithm that a pcapng reader would use to find the end of the journal entry?

Search for a zero byte, starting at the beginning of the Journal Entry field and ending just before the beginning of the Block Total Length field.

If a zero byte is not found, the entry ends before the Block Total Length field (as it's not padded).

If a zero byte is found, the entry ends before the zero byte; skip over zero bytes until you're at a 4-byte boundary. If the Block Total Length field follows the entry, there are no options; otherwise, the options begin on that 4-byte boundary and run until the Block Total Length field.

@mcr
Copy link
Collaborator

mcr commented Jul 24, 2023

wish we could put the options first....

@eqvinox
Copy link
Contributor Author

eqvinox commented Jul 24, 2023

A URL change needs to be made: […]

#133

Search for a zero byte, starting at the beginning of the Journal Entry field and ending just before the beginning of the Block Total Length field.

Yes — if there's any zero bytes, options start at the 4-byte boundary following the first zero byte. To be clear, if any options are present, there will be 1 to 4 zero bytes of padding. If without any zero bytes, the entry ends at a 4-byte boundary, there will be 4 bytes of zero padding. Otherwise how would you tell the entry has in fact ended…

wish we could put the options first....

Yup, that'd make things a lot easier 😢 … sadly the format is already established without options…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants