Skip to content
This repository was archived by the owner on Dec 6, 2023. It is now read-only.

Added new data handler "noforwardsecrecy".#98

Open
mkenne11 wants to merge 1 commit intogoogle:devfrom
mkenne11:noforwardsecrecy
Open

Added new data handler "noforwardsecrecy".#98
mkenne11 wants to merge 1 commit intogoogle:devfrom
mkenne11:noforwardsecrecy

Conversation

@mkenne11
Copy link

@mkenne11 mkenne11 commented Dec 6, 2015

The "noforwardsecrecy" data handler detects cipher suites negotiated between client and server (in the Server Hello message) which don't support forward secrecy i.e. Ephemeral Diffie-Hellman methods DHE or ECDHE.

Copy link
Author

Choose a reason for hiding this comment

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

If the TLS record contains no messages do I need to continue keep buffing the response, or can I simply "pass" the error and return the response object?

Note. The same comment applies to the exception handler in the SunsetSHA1 on_response method in PR #78.
https://github.com/mkenne11/nogotofail/blob/sunsetsha1/nogotofail/mitm/connection/handlers/data/ssl.py#l231

@mkenne11
Copy link
Author

Just checking if this PR and #78 are ok to merge? No worries if you are busy :)

Copy link
Contributor

Choose a reason for hiding this comment

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

(Sorry for the late response, holidays are busy :().

This would probably be easier to do using a subclass of the _TlsRecordHandler, that way you don't have to worry about the buffering and state

Copy link
Author

Choose a reason for hiding this comment

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

Thanks Chad that's good idea to subclass the _TlsRecordHandler handler. It will make the code much cleaner also.

The "noforwardsecrecy" data handler detects cipher suites negotiated between client and server (in the Server Hello message) which don't support forward secrecy i.e. Ephemeral Diffie-Hellman methods DHE or ECDHE.
@mkenne11
Copy link
Author

I made the changes recommended:

  • The NoForwardSecrecy handler now inherits from _TlsRecordHandler
  • INFO items are added in the event log when alerts for this handler are raised

I am still raising notifications in the Android client when alerts for this handler are raised. Do you think this OK? (Not sure if it's too spammy/ or if it's application policy to notify on INFO events).

Copy link
Author

Choose a reason for hiding this comment

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

Should I return the TLS record (byte array) each time the method is run?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants