It might be better to replace find_library everywhere. I just found out that it is causing AVCs.

type=AVC msg=audit(1492697538.114:2820): avc:  denied  { execute } for  pid=17848 comm="ipa-custodia" name="ldconfig" dev="dm-0" ino=4793 scontext=system_u:system_r:ipa_custodia_t:s0 tcontext=system_u:object_r:ldconfig_exec_t:s0 tclass=file permissive=1
type=AVC msg=audit(1492697538.114:2821): avc:  denied  { read open } for  pid=17848 comm="ipa-custodia" path="/usr/sbin/ldconfig" dev="dm-0" ino=4793 scontext=system_u:system_r:ipa_custodia_t:s0 tcontext=system_u:object_r:ldconfig_exec_t:s0 tclass=file permissive=1
type=AVC msg=audit(1492697538.114:2822): avc:  denied  { execute_no_trans } for  pid=17848 comm="ipa-custodia" path="/usr/sbin/ldconfig" dev="dm-0" ino=4793 scontext=system_u:system_r:ipa_custodia_t:s0 tcontext=system_u:object_r:ldconfig_exec_t:s0 tclass=file permissive=1

I'm not sure I like hardcoding the soname, how about a pure-python dumbed-down solution (doesn't honor ld.so.conf nor setuid/gid for LD_LIBRARY_PATH). This could be behind a platform conditional.

It would also allow the krb5 call to change to:

ctypes.CDLL(find_library("krb5"))

This may need more tuning but it does the basic job:

import os

def find_library(name):
    # extra hacky, need to parse ld.so.conf
    if os.path.exists('/lib64'):
        search_path="/lib64:/usr/lib64"
    else:
        search_path="/lib:/usr/lib"

    ld_library_path = os.environ.get('LD_LIBRARY_PATH')
    if ld_library_path:
        search_path=ld_library_path + ':' + search_path

    for directory in search_path.split(os.pathsep):
        possible = [name,
                    'lib%s.so' % name,
                    '%s.so' % name]
        for fname in possible:
            pname = os.path.join(directory, fname)
            if os.path.isfile(pname):
                return pname
    return None

assert(find_library("crypto") == "/lib64/libcrypto.so")
assert(find_library("libcrypto.so") == "/lib64/libcrypto.so")
assert(find_library("libcrypto.so.10") == "/lib64/libcrypto.so.10")
assert(find_library("/lib64/libcrypto.so") == "/lib64/libcrypto.so")

This looks like a good solution for a platform module. May be we should put this find_library implementation there in ipaplatform/base. Then a platform-specific variant would apply consistently.

Rob, would you take this to a PR or should I?

That makes sense to me. As for who does the work, I'm ok either way.

The trick is whether to parse ld.so.conf or not. This code wouldn't work at all on Ubuntu, for example. There could be a distro-specific default search-path i suppose.

Right, that's the point of the platform-specific (re-)implementation.

My point is if you parse ld.so.conf then you don't need to, for *nix anyway. For MacOS you'd still need to call their own find_library.

I don't think we need to parse anything. Just run 'ldconfig -N -p' and grep through the output for 'libcrypto.so ' line (with terminating space). This will give you content on the ld.so.cache as ld.so sees it, without any parsing of the ld.so.conf and friends.

This call is really cheap because ldconfig just reads the cache and its .mo files (for translations).

I considered that as well but I was trying to avoid the fork.

The ldconfig method is exactly what @cheimes spelled out in the original request and is what he is trying to avoid.

I think the argument instead should be: how expensive can we afford?

Are people going to be generating their own CSRs at such a rate that any overhead will be measurable?

By passing in a library without a soname version and path is going to cause cffi to do the same or similar search for the actual library anyway so in the end I think it is a wash.

Calling out for ldconfig is not expensive. I did the checks myself -- it reads ld.so.cache and that's all. Given that ld.so.cache is always mmap()-ed, It is much less expensive than other alternatives.

FreeIPA 4.6.3 has been released, moving to FreeIPA 4.6.4 milestone

Hi,

I put together an initial ctypes.util.find_library() replacement at ipaplatform/base/find_library.py... it needs work so guidance would be greatly appreciated

many thanks

https://github.com/m3gat0nn4ge/freeipa/tree/find_library

PR for review: https://github.com/freeipa/freeipa/pull/1773

thanks

@abbra find_library is two magnitudes slower even on a fast system with pre-warmed caches. The ipa command loads at least two external libraries with find_library, which slows down the command by at least 5 to 10ms for each call.

 python3 -m timeit -s "from ctypes.util import find_library; from ctypes import CDLL" "CDLL('libkrb5.so.3')"
100000 loops, best of 3: 13.6 usec per loop
$ python3 -m timeit -s "from ctypes.util import find_library; from ctypes import CDLL" "CDLL(find_library('krb5'))"
100 loops, best of 3: 2.75 msec per loop

@rcritten Your example script doesn't work unless development libraries are installed. libkrb5.so is provided by krb5-devel. libkrb5.so.3 is provided by krb5-libs.

@megatonnage Thanks for your PR. You are still using a subprocess and ldconfig to locate the library. You are even using more programs and shell=True, which is a security risk and most likely slower than find_library.

We could use a different approach and defer both the import of csrgen_ffi and loading of _librpm = _ffi.dlopen(find_library("rpm")). That would address the issue of slow startup in the default case. ipaplatform.redhat.tasks could use ctypes directly, too. There is no need to use FFI for a simple function call here.

@cheimes oh I know my attempt was sort of lousy which is why I didn't pursue it further :-)

https://github.com/freeipa/freeipa/pull/1774 takes care of librpm in ipaplatform.tasks.redhat.

Issue #7484 and https://github.com/freeipa/freeipa/pull/1779 handle ipaclient.csrgen_ffi

master:

• b82a229 Load librpm on demand for IPAVersion

All performance relevant uses of find_library have been replaced or now use deferred loading.