Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
xschlef opened this issue Oct 16, 2019 · 2 comments

Comments

@xschlef
Copy link
Contributor

xschlef commented Oct 16, 2019

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
Copy link
Member

solj commented Oct 17, 2019

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
Copy link
Contributor Author

xschlef commented Nov 8, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants