TS-3485: Support ip_allow config for HTTP2 #614
Conversation
proxy/http2/Http2SessionAccept.cc
Outdated
| ats_ip_nptop(client_ip, ipb, sizeof(ipb)), netvc->attributes); | ||
| } | ||
|
|
||
| // XXX Allocate a Http2ClientSession |
There was a problem hiding this comment.
Looks like this "XXX" is done now?
|
Where is the check on the method? Each individual stream would need to be verified. You can do a higher level check on the session if you check that all methods are being denied. |
|
The method checks are done in HttpTransact. The acl_record is stored on the ProxyClientTransaction object for that later check. |
|
Don't you need to check what is being denied at the higher level? |
|
Just pushed another commit that moves the IP-based ip allow checking to the SessionAccept superclass. Verified for IP-based and method-based policies. The method-based denies will return an error header and body. The IP-based denies just closed the connection. @bryancall I'm not following your question about checking what is denied at a higher level. |
| Warning("HTTP/2 client '%s' prohibited by ip-allow policy", ats_ip_ntop(client_ip, ipb, sizeof(ipb))); | ||
| netvc->do_io_close(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
There is an extra space at the end of this line.
|
Looks like it needs to be clang-formatted there are a few formatting issues. |
|
Yes, planning on running clang-format and squashing before mergin. |
|
Besides the formatting it looks good. I was meaning from above that you would have to check the allowed methods at the at the session level. I missed the isEmpty() before. |
Conflicts: lib/luajit proxy/http2/Http2ClientSession.cc proxy/http2/Http2SessionAccept.cc
TS-3485: Support ip_allow config for HTTP2. This closes apache#614.
Fix previous H2 cherry-pick which pulled in extra stuff
…o carry the plugin responses to jsonrpc messages. (apache#614) This needs to be clear up after use otherwise the state may hold the old one which can end up giving the wrong response to rpc requests.
…o carry the plugin responses to jsonrpc messages. (apache#614) This needs to be clear up after use otherwise the state may hold the old one which can end up giving the wrong response to rpc requests.
…o carry the plugin responses to jsonrpc messages. (apache#614) This needs to be clear up after use otherwise the state may hold the old one which can end up giving the wrong response to rpc requests.
Actually enforce the ip-level ACL checks for HTTP2 and move to the accept logic to make the decision before creating the session object.