Bcfg2 / bcfg2

Git clone of the Bcfg2 repository
http://bcfg2.org
Other
93 stars 58 forks source link

Connecting clients ips are not matched against actual client names with per client passwords #417

Open xschlef opened 5 years ago

xschlef commented 5 years ago

In clients.xml the documentation states:

name: Hostname of client. This needs to be the name (probably FQDN) returned by a reverse lookup on the connecting IP address.

This is not checked if you use password authentication with per client passwords.

    def AuthenticateConnection(self, cert, user, password, address):
        ...
        if cert:
            id_method = 'cert'
            certinfo = dict([x[0] for x in cert['subject']])
            # look at cert.cN
            client = certinfo['commonName']
            self.debug_log("Got cN %s; using as client name" % client)
        elif user == 'root':
            id_method = 'address'
            try:
                client = self.resolve_client(address)
            except Bcfg2.Server.Plugin.MetadataConsistencyError:
                err = sys.exc_info()[1]
                self.logger.error("Client %s failed to resolve: %s" %
                                  (address[0], err))
                return False
        else:
            id_method = 'uuid'
            # user maps to client
            if user not in self.uuid:
                client = user
                self.uuid[user] = user
            else:
                client = self.uuid[user]

This will switch to id_method = 'uuid' where die client name is simply set to the value of the user provided username. The actual hostname of the connecting ip is never resolved, it only has to pass ip-acl based restrictions.

The implication of this is quite low, as you need to know a username and a password to bind as a client. But I think the documentation is misleading, as it states that the connecting ip will be looked up and validated.

A proper fix would look up the connecting clients hostname either way and validate it before anything else unless the client is floating.

solj commented 5 years ago

An example of a use case where you would actually want this behavior is when you are running a bunch of clients behind a NAT. Doing a reverse lookup of the public address wouldn't add any additional security in that situation. Perhaps we need to clarify in the documentation?

xschlef commented 5 years ago

Well I could simply set the floating flag for those clients to circumvent the reverse lookup or do a matching based on uuid? Because at the moment you gain nothing by the host lookup itself in this scenario.