Modify

Ticket #1030 (closed defect: fixed)

Opened 10 years ago

Last modified 10 years ago

Better handling of floating cert-authenticated clients

Reported by: https://www.google.com/accounts/o8/id?id=AItOawkfHvWdYf7g8kSZA32s7dhK0Xig9JKo_CA Owned by: desai
Priority: major Milestone: Bcfg2 1.2.0 Release
Component: bcfg2-client Version: 1.0
Keywords: floating, ssl, cert, reverse dns Cc:

Description

A floating, cert-authenticated client can be not recognized properly by hostname if it resolves to an arbitrary name in reverse DNS.

Background: Metadata.resolve_client, called from @exposed Core's methods, falls back to reverse DNS lookup for client's name, because the name is not preserved thanks to bailing off early from Metadata.AuthenticateConnection.

(This issue can be related to #936.)

Here comes a patch which enables caching of client names for cert-based floating clients. The patch is against modified 1.2.0pre3-HEAD-9070f86, tested with Python 2.7. Line numbers may be slightly off.

diff --git a/src/lib/Server/Plugins/Metadata.py b/src/lib/Server/Plugins/Metadata.py
--- a/src/lib/Server/Plugins/Metadata.py
+++ b/src/lib/Server/Plugins/Metadata.py
@@ -568,15 +568,28 @@
                                       'Client', name=client,
                                       profile=profile)
         self.clients[client] = profile
         self.clients_xml.write()

-    def resolve_client(self, addresspair):
+    def resolve_client(self, addresspair, cleanup_cache=False):
         """Lookup address locally or in DNS to get a hostname."""
         if addresspair in self.session_cache:
+            # client _was_ cached, so there can be some expired entries
+            # we need to clean them up to avoid potentially infinite memory swell
+            cache_ttl = 90
+            if cleanup_cache:
+                # remove entries for this client's IP address with _any_ port numbers
+                # - perhaps a priority queue could be faster?
+                curtime = time.time()
+                for addrpair in self.session_cache.keys():
+                     if addresspair[0] == addrpair[0]:
+                         (stamp, _) = self.session_cache[addrpair]
+                         if curtime - stamp > cache_ttl:
+                             del self.session_cache[addrpair]
+            # return the cached data
             (stamp, uuid) = self.session_cache[addresspair]
-            if time.time() - stamp < 90:
+            if time.time() - stamp < cache_ttl:
                 return self.session_cache[addresspair][1]
         address = addresspair[0]
         if address in self.addresses:
             if len(self.addresses[address]) != 1:
                 self.logger.error("Address %s has multiple reverse assignments; a uuid must be used" % (address))
@@ -738,10 +751,13 @@

         if not addr_is_valid:
             return False

         if id_method == 'cert' and auth_type != 'cert+password':
+            # remember the cert-derived client name for this connection
+            if client in self.floating:
+                self.session_cache[address] = (time.time(), client)
             # we are done if cert+password not required
             return True

         if client not in self.passwords:
             if client in self.secure:
diff --git a/src/lib/Server/Core.py b/src/lib/Server/Core.py
--- a/src/lib/Server/Core.py
+++ b/src/lib/Server/Core.py
@@ -349,11 +349,11 @@
     @exposed
     def GetProbes(self, address):
         """Fetch probes for a particular client."""
         resp = lxml.etree.Element('probes')
         try:
-            name = self.metadata.resolve_client(address)
+            name = self.metadata.resolve_client(address, cleanup_cache=True)
             meta = self.build_metadata(name)

             for plugin in [p for p in list(self.plugins.values()) \
                            if isinstance(p, Bcfg2.Server.Plugin.Probing)]:
                 for probe in plugin.GetProbes(meta):

/mkd

Attachments

Change History

comment:1 Changed 10 years ago by solj

  • Status changed from new to closed
  • Resolution set to fixed

Applied in 5fe3867a3b75aff04eeb7ba8c910ee6939c1680f. Thanks for the patch!

WARNING! You need to establish a session before you can create or edit tickets. Otherwise the ticket will get treated as spam.

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.