-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
remove msgio double wrap #1065
remove msgio double wrap #1065
Conversation
There was doublewrapping with an unneeded msgio. given that we use a stream muxer now, msgio is only needed by secureConn -- to signal the boundaries of an encrypted / mac-ed ciphertext. Side note: i think including the varint length in the clear is actually a bad idea that can be exploited by an attacker. it should be encrypted, too. (TODO)
@whyrusleeping CR pls |
LGTM, did you run all the tests? |
@whyrusleeping good point. running on jupiter now |
3nodetest passed on cassi |
yep, passed on jupiter too. |
should we start a |
When do we want this to land? |
with the wire fmt / stream muxer changes. i'll have the spec of what we want done this week, so maybe we could impl next week |
i'd like to work on merging this soon. it fixes #1304 |
@whyrusleeping yeah we'll merge it as we move to interop with ipfs/js-ipfs#13 (comment) -- making the multistream + spdy stuff work will help here. |
being merged in dev0.4.0 |
closing, old pr cleanup time. |
There was doublewrapping with an unneeded msgio. given that we
use a stream muxer now, msgio is only needed by secureConn -- to
signal the boundaries of an encrypted / mac-ed ciphertext.
Side note: i think including the varint length in the clear is
actually a bad idea that can be exploited by an attacker. it should
be encrypted, too. (TODO)
WARNING: THIS PR BREAKS THE PROTOCOL. DO NOT MERGE UNTIL NEXT
PROTOCOL VERSION BUMP.