You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Chug Rolke <cr...@redhat.com> on 2014/07/08 00:18:35 UTC
Review Request 23322: Add ACL connection white/black lists to lock down
hosts from which a user connects
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23322/
-----------------------------------------------------------
Review request for qpid and Andrew Stitcher.
Summary (updated)
-----------------
Add ACL connection white/black lists to lock down hosts from which a user connects
Bugs: QPID-4947
https://issues.apache.org/jira/browse/QPID-4947
Repository: qpid
Description (updated)
-------
This patch adds:
1. ACL file syntax:
acl result user create connection host=<host> | <host,host>
group pepboys manny@QPID moe@QPID jack@QPID
acl allow chuck create connection host=localhost
acl allow chuck create connection host=[::1],[::1:0]
acl deny bob create connection host=1.1.1.1
acl deny all create connection host=10.0.0.0,10.255.255.255
acl deny all create connection host=192.168.0.0,192.168.255.255
acl deny all create connection host=localhost
acl allow pepboys create connection host=pepboys.com
acl deny pepboys create connection host=127.0.0.1
2. The connection denial is implemented in ConnectionCounter where there is an existing approver.
3. Connection denial in implemented in two loops:
a. Loop 1 is for user 'all'. This check could be placed at the socket level long before the user is known.
b. Loop 2 is for named users. This is a more normal check and should stay in ConnectionCounter
These checks reject the connection before any of the max connection limit checks are tested.
4. AclReader intercepts the create connection rules and keeps them in separate lists for the ConnectionCounter to check.
These rules aren't checked with the acl Lookup functions.
5. The binary address range checks are done in SocketAddress.
The implementation of the posix and windows changes to SocketAddress are identical.
The range matching logic has some non-obvious rules:
1. When a single host is named in the acl rule then that host may resolve to multiple addrinfo structures. This is fine because the host data is duplicated into the (low, high) range pair and the check is just for equality down the list.
2. When two hosts are named then each host must resolve to exactly one addrinfo struct. This because some hosts may resolve to multiple IPv4 or IPv6 addresses and comparing ranges in list like that is unpredictable. The host may resolve by name (as in example.com) or by numeric IP but only one address is allowed.
3. It is presumed that the incoming host being matched against the acl ranges is a numeric IP that resolves to one address.
4. Non-IP hosts are not subjected to nor denied by range checks.
Diffs (updated)
-----
trunk/qpid/cpp/src/CMakeLists.txt 1608612
trunk/qpid/cpp/src/qpid/AclHost.h PRE-CREATION
trunk/qpid/cpp/src/qpid/AclHost.cpp PRE-CREATION
trunk/qpid/cpp/src/qpid/acl/Acl.h 1608612
trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1608612
trunk/qpid/cpp/src/qpid/acl/AclConnectionCounter.h 1608612
trunk/qpid/cpp/src/qpid/acl/AclConnectionCounter.cpp 1608612
trunk/qpid/cpp/src/qpid/acl/AclData.h 1608612
trunk/qpid/cpp/src/qpid/acl/AclData.cpp 1608612
trunk/qpid/cpp/src/qpid/acl/AclReader.h 1608612
trunk/qpid/cpp/src/qpid/acl/AclReader.cpp 1608612
trunk/qpid/cpp/src/qpid/broker/AclModule.h 1608612
trunk/qpid/cpp/src/qpid/sys/SocketAddress.h 1608612
trunk/qpid/cpp/src/qpid/sys/posix/SocketAddress.cpp 1608612
trunk/qpid/cpp/src/qpid/sys/windows/SocketAddress.cpp 1608612
trunk/qpid/cpp/src/tests/AclHost.cpp PRE-CREATION
trunk/qpid/cpp/src/tests/CMakeLists.txt 1608612
Diff: https://reviews.apache.org/r/23322/diff/
Testing (updated)
-------
A unit test for the Acl parsing and matching functions is added.
Tests for acl.py are coming.
Thanks,
Chug Rolke
Re: Review Request 23322: Add ACL connection white/black lists to lock down
hosts from which a user connects
Posted by Chug Rolke <cr...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23322/
-----------------------------------------------------------
(Updated July 10, 2014, 12:59 p.m.)
Review request for qpid and Andrew Stitcher.
Changes
-------
This improves a few things. Most notable is that the SocketAddress objects only ever call ::getaddrinfo once ever. After that the .addrInfo and .currentAddrInfo pointers are used for iterating the addrinfo structures. The new firstAddress() member function makes this possible. Debug log output shows how the example acl file is processed.
2014-07-10 08:33:10 [Security] notice ACL: Read file "acl.acl"
2014-07-10 08:33:10 [Security] debug ACL: Group list: 1 groups found:
2014-07-10 08:33:10 [Security] debug ACL: "pepboys": jack@QPID manny@QPID moe@QPID
2014-07-10 08:33:10 [Security] debug ACL: name list: 6 names found:
2014-07-10 08:33:10 [Security] debug ACL: * bob chuck jack@QPID manny@QPID moe@QPID
2014-07-10 08:33:10 [Security] debug ACL: Rule list: 8 ACL rules found:
2014-07-10 08:33:10 [Security] debug ACL: 1 allow [chuck] create connection host=localhost
2014-07-10 08:33:10 [Security] debug ACL: 2 allow [chuck] create connection host=[::1],[::1:0]
2014-07-10 08:33:10 [Security] debug ACL: 3 deny [bob] create connection host=1.1.1.1
2014-07-10 08:33:10 [Security] debug ACL: 4 deny [*] create connection host=10.0.0.0,10.255.255.255
2014-07-10 08:33:10 [Security] debug ACL: 5 deny [*] create connection host=192.168.0.0,192.168.255.255
2014-07-10 08:33:10 [Security] debug ACL: 6 deny [*] create connection host=localhost
2014-07-10 08:33:10 [Security] debug ACL: 7 allow [jack@QPID, manny@QPID, moe@QPID] create connection host=pepboys.com
2014-07-10 08:33:10 [Security] debug ACL: 8 deny [jack@QPID, manny@QPID, moe@QPID] create connection host=127.0.0.1
2014-07-10 08:33:10 [Security] debug ACL: connections quota: 0 rules found:
2014-07-10 08:33:10 [Security] debug ACL: queues quota: 0 rules found:
2014-07-10 08:33:10 [Security] debug ACL: Load Rules
2014-07-10 08:33:10 [Security] debug ACL: Processing 8 deny [jack@QPID, manny@QPID, moe@QPID] create connection host=127.0.0.1
2014-07-10 08:33:10 [Security] debug ACL: Processing 7 allow [jack@QPID, manny@QPID, moe@QPID] create connection host=pepboys.com
2014-07-10 08:33:10 [Security] debug ACL: Processing 6 deny [*] create connection host=localhost
2014-07-10 08:33:10 [Security] debug ACL: Processing 5 deny [*] create connection host=192.168.0.0,192.168.255.255
2014-07-10 08:33:10 [Security] debug ACL: Processing 4 deny [*] create connection host=10.0.0.0,10.255.255.255
2014-07-10 08:33:10 [Security] debug ACL: Processing 3 deny [bob] create connection host=1.1.1.1
2014-07-10 08:33:10 [Security] debug ACL: Processing 2 allow [chuck] create connection host=[::1],[::1:0]
2014-07-10 08:33:10 [Security] debug ACL: Processing 1 allow [chuck] create connection host=localhost
2014-07-10 08:33:10 [Security] debug ACL: global Connection Rule list : 3 rules found :
2014-07-10 08:33:10 [Security] debug ACL: 1 [ruleMode = deny {(10.0.0.0,10.255.255.255)}
2014-07-10 08:33:10 [Security] debug ACL: 2 [ruleMode = deny {(192.168.0.0,192.168.255.255)}
2014-07-10 08:33:10 [Security] debug ACL: 3 [ruleMode = deny {([::1],[::1]),(127.0.0.1,127.0.0.1)}
2014-07-10 08:33:10 [Security] debug ACL: User Connection Rule lists : 5 user lists found :
2014-07-10 08:33:10 [Security] debug ACL: bob Connection Rule list : 1 rules found :
2014-07-10 08:33:10 [Security] debug ACL: 1 [ruleMode = deny {(1.1.1.1,1.1.1.1)}
2014-07-10 08:33:10 [Security] debug ACL: chuck Connection Rule list : 2 rules found :
2014-07-10 08:33:10 [Security] debug ACL: 1 [ruleMode = allow {([::1],[::1]),(127.0.0.1,127.0.0.1)}
2014-07-10 08:33:10 [Security] debug ACL: 2 [ruleMode = allow {([::1],[::0.1.0.0])}
2014-07-10 08:33:10 [Security] debug ACL: jack@QPID Connection Rule list : 2 rules found :
2014-07-10 08:33:10 [Security] debug ACL: 1 [ruleMode = allow {(166.78.227.116,166.78.227.116)}
2014-07-10 08:33:10 [Security] debug ACL: 2 [ruleMode = deny {(127.0.0.1,127.0.0.1)}
2014-07-10 08:33:10 [Security] debug ACL: manny@QPID Connection Rule list : 2 rules found :
2014-07-10 08:33:10 [Security] debug ACL: 1 [ruleMode = allow {(166.78.227.116,166.78.227.116)}
2014-07-10 08:33:10 [Security] debug ACL: 2 [ruleMode = deny {(127.0.0.1,127.0.0.1)}
2014-07-10 08:33:10 [Security] debug ACL: moe@QPID Connection Rule list : 2 rules found :
2014-07-10 08:33:10 [Security] debug ACL: 1 [ruleMode = allow {(166.78.227.116,166.78.227.116)}
2014-07-10 08:33:10 [Security] debug ACL: 2 [ruleMode = deny {(127.0.0.1,127.0.0.1)}
2014-07-10 08:33:10 [Security] info ACL Plugin loaded
Bugs: QPID-4947
https://issues.apache.org/jira/browse/QPID-4947
Repository: qpid
Description
-------
This patch adds:
1. ACL file syntax:
acl result user create connection host=<host> | <host,host>
group pepboys manny@QPID moe@QPID jack@QPID
acl allow chuck create connection host=localhost
acl allow chuck create connection host=[::1],[::1:0]
acl deny bob create connection host=1.1.1.1
acl deny all create connection host=10.0.0.0,10.255.255.255
acl deny all create connection host=192.168.0.0,192.168.255.255
acl deny all create connection host=localhost
acl allow pepboys create connection host=pepboys.com
acl deny pepboys create connection host=127.0.0.1
2. The connection denial is implemented in ConnectionCounter where there is an existing approver.
3. Connection denial in implemented in two loops:
a. Loop 1 is for user 'all'. This check could be placed at the socket level long before the user is known.
b. Loop 2 is for named users. This is a more normal check and should stay in ConnectionCounter
These checks reject the connection before any of the max connection limit checks are tested.
4. AclReader intercepts the create connection rules and keeps them in separate lists for the ConnectionCounter to check.
These rules aren't checked with the acl Lookup functions.
5. The binary address range checks are done in SocketAddress.
The implementation of the posix and windows changes to SocketAddress are identical.
The range matching logic has some non-obvious rules:
1. When a single host is named in the acl rule then that host may resolve to multiple addrinfo structures. This is fine because the host data is duplicated into the (low, high) range pair and the check is just for equality down the list.
2. When two hosts are named then each host must resolve to exactly one addrinfo struct. This because some hosts may resolve to multiple IPv4 or IPv6 addresses and comparing ranges in list like that is unpredictable. The host may resolve by name (as in example.com) or by numeric IP but only one address is allowed.
3. It is presumed that the incoming host being matched against the acl ranges is a numeric IP that resolves to one address.
4. Non-IP hosts are not subjected to nor denied by range checks.
Diffs (updated)
-----
trunk/qpid/cpp/src/CMakeLists.txt 1609448
trunk/qpid/cpp/src/qpid/acl/Acl.h 1609448
trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1609448
trunk/qpid/cpp/src/qpid/acl/AclConnectionCounter.h 1609448
trunk/qpid/cpp/src/qpid/acl/AclConnectionCounter.cpp 1609448
trunk/qpid/cpp/src/qpid/acl/AclData.h 1609448
trunk/qpid/cpp/src/qpid/acl/AclData.cpp 1609448
trunk/qpid/cpp/src/qpid/acl/AclReader.h 1609448
trunk/qpid/cpp/src/qpid/acl/AclReader.cpp 1609448
trunk/qpid/cpp/src/qpid/broker/AclModule.h 1609448
trunk/qpid/cpp/src/qpid/sys/SocketAddress.h 1609448
trunk/qpid/cpp/src/qpid/sys/posix/SocketAddress.cpp 1609448
trunk/qpid/cpp/src/qpid/sys/windows/SocketAddress.cpp 1609448
trunk/qpid/cpp/src/tests/CMakeLists.txt 1609448
Diff: https://reviews.apache.org/r/23322/diff/
Testing
-------
A unit test for the Acl parsing and matching functions is added.
Tests for acl.py are coming.
Thanks,
Chug Rolke