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

AEAD fuzzing support with GCM example #40

Merged
merged 1 commit into from Jul 7, 2016

Conversation

Projects
None yet
2 participants
@blaufish
Copy link
Contributor

blaufish commented Jun 28, 2016

Fuzz sealed (AEAD encrypted) packets.
TLS_RSA_WITH_AES_128_GCM_SHA256 example.

@@ -21,6 +21,9 @@ def main():
"""check if incorrect MAC hash is rejected by server"""
conversations = {}

#aead = True # GCM
aead = False # CBC

This comment has been minimized.

@tomato42

tomato42 Jun 28, 2016

Owner

I would prefer if that was an additional loop, external to the for pos, val

or simply a separate test script

This comment has been minimized.

@tomato42

tomato42 Jun 28, 2016

Owner

a separate script probably would be better idea: the authentication tag is just 16 bytes instead of the 20 bytes of SHA-1 HMAC

This comment has been minimized.

@blaufish

blaufish Jun 28, 2016

Contributor

a separate script is preferable maybe. I'll try that.

b"GET / HTTP/1.0\n\n"),
xors={pos:val}))
node = node.add_child( \
fuzz_seal(ApplicationDataGenerator( b"GET / HTTP/1.0\n\n"), xors={pos:val})

This comment has been minimized.

@tomato42

tomato42 Jun 28, 2016

Owner

why fuzz_message() is not sufficient?

This comment has been minimized.

@tomato42

tomato42 Jun 28, 2016

Owner

duh, it won't work because it is applied pre-encryption, not post encryption

what I wanted to say is, why not apply fuzzing in the send() method of RecordSocket, and make a general fuzz_ciphertext method so that it is also applicable to Encrypt-then-MAC ciphers? fuzz_seal doesn't limit the modification to just the tag portion of the AEAD ciphertext...

This comment has been minimized.

@blaufish

blaufish Jun 28, 2016

Contributor

The script as written now does seem to be working, I can see it connect with GCM to the server and getting bad records when it should.

fuzz_message() not used due to error TypeError: unsupported operand type(s) for ^=: 'str' and 'int' when it attempts to apply xor to a string. So I was assuming fuzz_message() wasn't written for this purpose.

I can try develop a fuzz_ciphertext() as you suggested if this is the best way forward.

This comment has been minimized.

@tomato42

tomato42 Jun 28, 2016

Owner

fuzz_message() not used due to error TypeError: unsupported operand type(s) for ^=: 'str' and 'int' when it attempts to apply xor to a string.

that's weird... scripts/test-fuzzed-finished.py uses fuzz_message and it works just fine in both Py2 and Py3. But as I've said ("duh, it won't work"), fuzz_message won't work for testing AEAD

I can try develop a fuzz_ciphertext() as you suggested if this is the best way forward.

it allows for arbitrary fuzzing of unencrypted messages and kills two birds with one stone (fuzzing EtM and AEAD)

message.data = encrypt(plain + padding) + mac; etm
message.data = encrypt(plain + mac + padding); mte
"""
data = message.data

This comment has been minimized.

@tomato42

tomato42 Jun 29, 2016

Owner

this is not guaranteed to work, this code can get higher level messages (like a CllintHello object), message.write() will work for any object that can be passed to socket

(-12, 0x01),
(-12, 0xff),
(-16, 0x01),
(-16, 0xff), #boundary 128bit between aes and gcm tag?

This comment has been minimized.

@tomato42

tomato42 Jun 29, 2016

Owner

any particular reason to test bytes outside tag in this test?

@@ -764,6 +764,46 @@ def new_calculate_mac(mac, seqnumBytes, contentType, data,

return generator

def fuzz_encrypted_message(generator, substitutions=None, xors=None):
"""Change arbitrary bytes of the AEAD sealed block"""

This comment has been minimized.

@tomato42

tomato42 Jun 29, 2016

Owner

it's general, not limited to AEAD


self.old_send = old_send

def new_send(message, padding, old_send=old_send, substitutions=substitutions,

This comment has been minimized.

@tomato42

tomato42 Jun 29, 2016

Owner

line too long

message.data = aead_encrypt(plain); defined by aead suite
message.data = encrypt(plain + padding) + mac; etm
message.data = encrypt(plain + mac + padding); mte
"""

This comment has been minimized.

@tomato42

tomato42 Jun 29, 2016

Owner

this will not be interpreted as a doc string, needs to be together, with the first line ("Monkey patch...") on its own, empty line and the "message..." following. It should not be indented

from tlsfuzzer.messages import Connect, ClientHelloGenerator, \
ClientKeyExchangeGenerator, ChangeCipherSpecGenerator, \
FinishedGenerator, ApplicationDataGenerator, \
fuzz_mac, fuzz_encrypted_message

This comment has been minimized.

@tomato42

tomato42 Jun 29, 2016

Owner

fuzz_mac is unused

@tomato42

This comment has been minimized.

Copy link
Owner

tomato42 commented Jun 29, 2016

unit tests?

please rebase and squash commits together (makes reading and bisecting commits later much easier)

once that's done, looks good for merging

@blaufish blaufish force-pushed the blaufish:master branch 2 times, most recently from da9cd62 to a07cf8b Jun 30, 2016

ciphertext/mac/tag fuzzing support
including a TLS_RSA_WITH_AES_128_GCM_SHA256 example.

@blaufish blaufish force-pushed the blaufish:master branch from a07cf8b to 9495971 Jun 30, 2016

@blaufish

This comment has been minimized.

Copy link
Contributor

blaufish commented Jul 2, 2016

@tomato42 unit test added 👍

@tomato42

This comment has been minimized.

Copy link
Owner

tomato42 commented Jul 7, 2016

sorry for the delay, was on holiday

looks good! Thanks!

@tomato42 tomato42 merged commit e6c4963 into tomato42:master Jul 7, 2016

4 of 5 checks passed

code-quality/landscape Code quality decreased by -0.06%
Details
QuantifiedCode No new issues found.
Details
codeclimate Code Climate didn't find any new or fixed issues.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.06%) to 96.517%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment