Skip to content

Conversation

@sescandor
Copy link

@sescandor sescandor commented Mar 25, 2020

Includes fuzz target as well as corpus set.

@sescandor sescandor marked this pull request as ready for review April 2, 2020 11:17
Copy link
Member

@kazuho kazuho left a comment

Choose a reason for hiding this comment

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

Thank you for the PR.

I like the idea of fuzzing our connection startup. That's when many things happen, it's a good way of making sure that we are robust.

The limitation of the PR as is now is that it only fuzzes the decoding of datagram into multiple packets (i.e. quicly_decode_packet) and first few lines of quicly_receive. This is because Initial packets are authenticated using AEAD. All the spoofed Initial packets would end up having an incorrect AEAD tag, and that would lead to quicly_receive discarding those packets before making any use.

It would be great if we could make sure that the spoofed packet being passed to quicly_receive (or quicly_accept) would something that is actually being processed.

I think we can achieve that goal by doing the following:

  • Define and use a special random_bytes callback that returns a constant pattern. Doing so would fix the AEAD key that would be used for authenticating the Initial packets to a constant value.
  • Supply the packet image in decrypted form to the fuzzer. Then, when processing the input from the fuzzer, apply AEAD encryption, then pass that to quicly_receive (or quicly_accept).

WDYT?

quicly_cid_plaintext_t next_cid;
const char* host = "127.0.0.1";
const char* port = "4422";
ptls_iovec_t *resumption_token = (ptls_iovec_t *)malloc(sizeof(ptls_iovec_t));
Copy link
Member

Choose a reason for hiding this comment

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

I do not think we need a pointer (with allocate memory), as the only use of the value in relation to other functions is passing the value of the pointer in quicly_connect. This can be ptls_iovec_t resumption_token = ptls_iovec_init(NULL, 0).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants