Ticket #3131: investigate security issue

THIS IS A SECURITY ISSUE and is NOT FOR PUBLIC DISCUSSION

everything that follows is referencing a pool that uses only host-based authorization with no authentication.

when conodor receives a connection, it decides if the connection is authorized by looking at an ALLOW_X list (for example, ALLOW_ADMINISTRATOR). this list can contain IP addresses, hostnames, and wildcards in either.

if the IP address has a reverse DNS entry that maps the IP to, for example, "condor.cs.wisc.edu", then an attacker may connect from that IP address and actually be authorized as if they were coming from a different IP address.

proposed fix: after doing a reverse lookup, also do a forward lookup to verify that the IP that initiated the connection is a valid IP for that DNS hostname.

[Append remarks]

Remarks:

2012-Jul-17 13:54:37 by zmiller:
CONFIRMED

i actually tested this by setting up a "naughty" reverse DNS for exec-1.batlab.org:

% host exec-1.batlab.org
exec-1.batlab.org has address 128.104.103.1
% host 128.104.103.1
1.103.104.128.in-addr.arpa domain name pointer exec-1.batlab.org.
1.103.104.128.in-addr.arpa domain name pointer astro.cs.wisc.edu.

and here is the authorization list on bio.cs.wisc.edu:

% condor_config_val hostallow_administrator
tonic.cs.wisc.edu, chopin.cs.wisc.edu, astro.cs.wisc.edu, stork.cs.wisc.edu, pinguino.cs.wisc.edu, cobalt.cs.wisc.edu nation.cs.wisc.edu

i then logged in to exec-1.batlab.org and was able to issue a condor_off command to bio.cs.wisc.edu. here is the master log (notice the IP/hostname correlation):

07/17/12 13:49:07 Calling Handler <DaemonCommandProtocol::WaitForSocketData> (2)
07/17/12 13:49:07 DC_AUTHENTICATE: received DC_AUTHENTICATE from <128.104.103.1:49786>
07/17/12 13:49:07 DC_AUTHENTICATE: added incoming session id bio:4051:1342550947:7 to cache for 80 seconds (lease is 3620s, return address is unknown).
07/17/12 13:49:07 DC_AUTHENTICATE: Success.
07/17/12 13:49:07 IPVERIFY: matched user unauthenticated@unmapped from astro.cs.wisc.edu to allow list
07/17/12 13:49:07 Adding to resolved authorization table: unauthenticated@unmapped/128.104.103.1: ADMINISTRATOR
07/17/12 13:49:07 PERMISSION GRANTED to unauthenticated@unmapped from host 128.104.103.1 for command 60005 (DC_OFF_GRACEFUL), access level ADMINISTRATOR: reason: ADMINISTRATOR authorization policy allows hostname astro.cs.wisc.edu; identifiers used for this remote host: 128.104.103.1,astro.cs.wisc.edu
07/17/12 13:49:07 Received TCP command 60005 (DC_OFF_GRACEFUL) from unauthenticated@unmapped <128.104.103.1:49786>, access level ADMINISTRATOR
07/17/12 13:49:07 Calling HandleReq <handle_off_graceful()> (0) for command 60005 (DC_OFF_GRACEFUL) from unauthenticated@unmapped <128.104.103.1:49786>
07/17/12 13:49:07 Return from HandleReq <handle_off_graceful()> (handler: 0.000s, sec: 0.004s, payload: 0.000s)
07/17/12 13:49:07 Return from Handler <DaemonCommandProtocol::WaitForSocketData> 0.0041s
07/17/12 13:49:07 Got SIGTERM. Performing graceful shutdown.


2012-Jul-17 14:48:05 by tannenba:
Thoughts on a patch:

When Condor does the reverse dns for host based authentication, it needs to implement forward-confirmed reverse dns.

At a high level, when the server receives a connection from the client, Condor should do an inverse lookup to get a list of hostnames associated with that address. For each hostname, it should do a forward lookup, and discard any entry in the list where a forward lookup does not return the same address.

This should be a simple patch, but things are more complicated now with the IPv6 code. With the addition of IPv6, it is not clear the host alias functionality is intact, and also not clear that all code paths funnel through condor_gethostbyaddr() anymore like they did before IPv6.


2012-Jul-18 11:56:33 by tannenba:
Thoughts on a workaround:

Until folks upgrade their binaries, I folks would be safe if they changed their host allow/deny settings to only reference IP addresses (no hostnames). For instance, instead of

   hostallow_write = *.cs.wisc.edu

instead do

   hostallow_write = 128.105.*
----
_2012-Aug-23 13:52:45 by zmiller:_ 
Bulk change of target version from v070802 to v070803 using ./ticket-target-mover. ---- _2012-Aug-23 14:11:43 by zmiller:_
Bulk change of target version from v070802 to v070803 using ./ticket-target-mover.
[Append remarks]

Properties:

Type: defect           Last Change: 2013-Jun-17 14:47
Status: resolved          Created: 2012-Jul-17 11:12
Fixed Version: v070802           Broken Version: v070000 
Priority:          Subsystem: Security 
Assigned To: zmiller           Derived From:  
Creator: zmiller  Rust:  
Customer Group: chtc  Visibility: public 
Notify: matt@cs.wisc.edu, tstclair@redhat.com  Due Date:  

Related Check-ins:

2012-Aug-15 13:21   Check-in [33067]: move in fix for #3131 (By Zach Miller )
2012-Aug-15 11:24   Check-in [33061]: move in fix for #3131 (By Zach Miller )
2012-Aug-15 09:56   Check-in [33052]: fix issue #3131 (By Zach Miller )