tests and fixes for issue #69#71
Conversation
|
Added the new fix. Let me know what you think. |
pires
left a comment
There was a problem hiding this comment.
The code mostly LGTM. Other than two comments I kindly request you tidy up the commit log.
I'd also like to compensate you for this contribution as it seems a serious security issue. Can you please reach out over Twitter (@therealpires) or email pjpires at googlemail?
Thanks a ton!
|
cc @emersion @mschneider82 @TimWolla @jefferai (I wish I remembered everyone's name/handle). Someone at Snyk is also willing to help getting a CVE for this which will be a first for me. Thanks everybody out there paying attention and willing to help. |
|
Thanks for the ping! I'm afraid I won't be much of a help here, though. I'm not even a go programmer. My contribution regarding the unique ID TLV was motivated by the fact that I was the person that added this TLV to the PROXYv2 spec (I'm somewhat involved with upstream HAProxy) and I just searched for proxy protocol libraries to make sure they are up to date. |
|
@pires Thanks for pinging me! This generally looks good, left a couple of comments. |
Any suggestions how I tidy up the commit log? I agree needs to be done, just no idea how to without new pull request. If that's what it takes, more than happy to. |
Use an interactive rebase |
Thanks. Learn something new everyday :) |
476d66a to
a368adc
Compare
|
@pires can you tag a new release? |
|
Working on it. Want to get Go 1.16 in and some fixes to tests I spotted while doing so. |
|
i do a pull req for traefik, where i used this lib to provide proxy protocol support, but traefik still depends on go 1.15, so requiring 1.16 here would break it. I will stick on last commit for now |
|
There's no such thing as enforcing Go 1.16 requirement. CI compiles and tests for Go 1.14 and Go 1.15, and now want to add 1.16, that's all. |
|
Ok fine. The traefik maintainers requested a new version tag |
|
OK the tests fixing will have to wait as I need more time to find a reasonable strategy while also refactoring to reduce lots of duplicated code. |
2As suggested in PR#70, a new pull request which captures just the tests proving issue #69, with some refinements:
The second issue above can be used for DoS by consuming server process sockets.
Will then submit fixes - further testing shows that pull request #70 does not solve the problem properly.
Fixes #69