Univention Bugzilla – Bug 49383
Several univention-python-heimdal bugs - SIGSEGV, memory corruption
Last modified: 2020-03-11 14:41:44 CET
While migrating univention-python-heimdal to dh_python2 (Bug #49139) I found several bugs leading to crashes: 1. Dereference pointer instead of comparing it with an uninitialized integer: > keyblock.c:118:24: warning: comparison between pointer and integer > enctype = enctype_obj>enctype; > ^ > keyblock.c:118:24: warning: ‘enctype’ may be used uninitialized in this function [-Wmaybe-uninitialized] > enctype = enctype_obj>enctype; > ~~~~~~~~~~~^~~~~~~~ PATCH: sed -i -e '118s/>/->/' keyblock.c 2. Missing error handling: > creds.c:127:3: warning: ignoring return value of ‘asprintf’, declared with attribute warn_unused_result [-Wunused-result] > asprintf(&s, "unknown (%d)", t.enc_part.etype); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > keytab.c:207:6: warning: variable ‘error’ set but not used [-Wunused-but-set-variable] > int error = 0; > ^~~~~ > keytab.c:235:4: warning: ignoring return value of ‘asprintf’, declared with attribute warn_unused_result [-Wunused-result] > asprintf(&etype, "unknown (%d)", entry.keyblock.keytype); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 3. Wrong type: > context.c:87:32: warning: comparison between ‘krb5_enctype {aka enum ENCTYPE}’ and ‘enum <anonymous>’ [-Wenum-compare] > for (i=0; etypes && etypes[i] != ETYPE_NULL; i++) { > ^~ PATCH: sed -i -e '87:s/ETYPE_NULL/KRB5_ENCTYPE_NULL/' context.c 4. keytab.add() clobbers immutable string: > keytab.c:181 »···»···memset (password_string, 0, strlen(password_string)); python -c 'import heimdal;P="univention";heimdal.keytab(heimdal.context(), "").add("a@B.C", 0, "des-cbc-md5", P, 0, 0);print(P)' Why is this bad: >>> a = "univention" >>> b = "univention" >>> hash(a), hash(b) (6976887264700423245, 6976887264700423245) >>> import heimdal as h;h.keytab(h.context(), "").add("a@B.C", 0, "des-cbc-md5", a, 0, 0); >>> a '\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00' >>> b '\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00' Python internally hashed any string and only ever stores one instance of the text. If the password passed to keytab.add() by pure change is equal to any other string instance (in the process), the string their is clobbered as well. PATCH: sed -i -e '181d' keytab.c 5. dir(keytab) crashed with SIGSEGV: python -c 'import heimdal;dir(heimdal.keytab(heimdal.context(), ""))' Program received signal SIGSEGV, Segmentation fault. Py_FindMethodInChain (chain=0x7fffffffe200, self=<Keytab at remote 0x7ffff7f790b0>, name=0x55555576aee6 "__dict__") at ../Objects/methodobject.c:370 370 ../Objects/methodobject.c: Datei oder Verzeichnis nicht gefunden. (gdb) bt #0 Py_FindMethodInChain (chain=0x7fffffffe200, self=<Keytab at remote 0x7ffff7f790b0>, name=0x55555576aee6 "__dict__") at ../Objects/methodobject.c:370 #1 0x0000555555725419 in Py_FindMethod (methods=<optimized out>, self=<optimized out>, name=<optimized out>) at ../Objects/methodobject.c:389 #2 0x0000555555669e3d in _generic_dir (obj=<Keytab at remote 0x7ffff7f790b0>) at ../Objects/object.c:1863 #3 _dir_object (obj=<Keytab at remote 0x7ffff7f790b0>) at ../Objects/object.c:1940 #4 PyObject_Dir () at ../Objects/object.c:1977 #5 0x0000555555669946 in builtin_dir.lto_priv () at ../Python/bltinmodule.c:598 #6 0x000055555564f84a in call_function (oparg=<optimized out>, pp_stack=0x7fffffffe338) at ../Python/ceval.c:4352 #7 PyEval_EvalFrameEx () at ../Python/ceval.c:2989 #8 0x000055555564d9f5 in PyEval_EvalCodeEx () at ../Python/ceval.c:3584 #9 0x000055555564d7b9 in PyEval_EvalCode (co=<optimized out>, globals=<optimized out>, locals=<optimized out>) at ../Python/ceval.c:669 #10 0x00005555556ad3b6 in run_mod.lto_priv.2003 (arena=0x555555bc9b20, flags=0x7fffffffe52c, locals={'__builtins__': <module at remote 0x7ffff7fa4b08>, '__name__': '__main__', 'heimdal': <module at remote 0x7ffff7f814e8>, '__doc__': None, '__package__': None}, globals={'__builtins__': <module at remote 0x7ffff7fa4b08>, '__name__': '__main__', 'heimdal': <module at remote 0x7ffff7f814e8>, '__doc__': None, '__package__': None}, filename=0x5555557768eb "<string>", mod=<optimized out>) at ../Python/pythonrun.c:1376 #11 PyRun_StringFlags () at ../Python/pythonrun.c:1339 #12 0x00005555556ae07c in PyRun_SimpleStringFlags () at ../Python/pythonrun.c:974 #13 0x00005555556294a9 in Py_Main () at ../Modules/main.c:584 #14 0x00007ffff6f182e1 in __libc_start_main (main=0x5555556290a0 <main>, argc=3, argv=0x7fffffffe6e8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffe6d8) at ../csu/libc-start.c:291 #15 0x0000555555628f9a in _start () 6. Exceptions are named all wrong: python -c 'import heimdal;print(dir(heimdal))' They are all named similar to '(-1765328379L)'; this is due the nested use of C pre-processor macros: PATCH: sed -i '72{h;d};80{p;g;s/\<o\>/e/}' error.c 7. Many more issues already fixed in my WIP git branch: phahn/49139_dhpy2-heimdal
8. Missing type checks: python -c 'import heimdal as h;h.principal("", "")' -> SIGSEGV PyArg_ParseTuple(args, "Os", &context, &enc) is used which accepts any Python object, which is then cast to "krb5_context" without checking if "context" is of the correct type.
git:phahn/49139_dhpy2-heimdal
[4.4-3] bb118a0e46 Bug #50475,#49383,#38736 Heimdal: Merge branch 'phahn/49139_dhpy2-heimdal' into 4.4-3 Package: univention-python-heimdal Version: 9.0.0-3A~4.4.0.201912181749 Branch: ucs_4.4-0 Scope: errata4.4-3 [4.4-3] 6740211353 Bug #50475: univention-python-heimdal 9.0.0-3A~4.4.0.201912181749 doc/errata/staging/univention-python-heimdal.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
QA: There is a unit test "test.py", which you even can execute on an old system. Most tests will crash due to bugs in the old heimdal.so. You have to execute them one-by-one manually like this: python2 test.py TestRefcount.test_context python2 test.py TestRefcount.test_principal python2 test.py TestRefcount.test_creds python2 test.py TestRefcount.test_keytab python2 test.py TestRefcount.test_salt python2 test.py TestRefcount.test_enctype python2 test.py TestRefcount.test_keyblock python2 test.py TestRefcount.test_exception python2 test.py TestContext.test_get_permitted_enctypes python2 test.py TestContext.test_dir python2 test.py TestPrincipal.test_type # FIXED python2 test.py TestPrincipal.test_principal python2 test.py TestPrincipal.test_realm # NEW python2 test.py TestPrincipal.test_dir python2 test.py TestCreds.test_parse python2 test.py TestCreds.test_change_password python2 test.py TestCreds.test_dir # CRASHED python2 test.py TestKeytab.test_keytab_missing python2 test.py TestKeytab.test_keytab_empty python2 test.py TestKeytab.test_keytab_add python2 test.py TestKeytab.test_keytab_remove_missing python2 test.py TestKeytab.test_keytab_remove_existing python2 test.py TestKeytab.test_dir python2 test.py TestSalt.test_salt python2 test.py TestSalt.test_salt_raw python2 test.py TestSalt.test_dir python2 test.py TestEnctype.test_type # FIXED python2 test.py TestEnctype.test_enctype python2 test.py TestEnctype.test_dir python2 test.py TestKeyblock.test_keyblock_principal python2 test.py TestKeyblock.test_keyblock_salt python2 test.py TestKeyblock.test_keyblock_raw python2 test.py TestKeyblock.test_dir python2 test.py TestCcache.test_list python2 test.py TestCcache.test_init # CRASHED python2 test.py TestCcache.test_use_after_destroy # CRASHED python2 test.py TestCcache.test_store_cred python2 test.py TestASN1.test_asn1_decode_key python2 test.py TestASN1.test_asn1_decode_key_with_context # NEW python2 test.py TestASN1.test_asn1_encode_key python2 test.py TestException.test_KRB5KDC_ERR_NONE # FIXED python2 test.py TestException.test_exception # FIXED
This breaks <https://jenkins.knut.univention.de:8181/job/UCS-4.4/job/UCS-4.4-3/job/S4Connector/20/> 52_s4connector.030_sync_with_activated_pwqualitycheck.master071c 52_s4connector.109sync_create_and_modify_ucs_to_ad.master071c 52_s4connector.401check_posix_pwd_expiry_after_ad_pwdchange.master071c 52_s4connector.030_sync_with_activated_pwqualitycheck.master091c 52_s4connector.401check_posix_pwd_expiry_after_ad_pwdchange.master091c
For now I removed the binary packages form ucs_4.4-0-errata4.4-3: Possible cause: Terminating running univention-cli-server processes. Traceback (most recent call last): File "/usr/share/univention-s4-connector/sync_krbtgt", line 190, in <module> main() File "/usr/share/univention-s4-connector/sync_krbtgt", line 184, in main s4.sync_password() File "/usr/share/univention-s4-connector/sync_krbtgt", line 159, in sync_password krb5Key_new = univention.s4connector.s4.password.calculate_krb5key(unicodePwd_attr, supplementalCredentials, int(msDS_KeyVersionNumber)) File "/usr/lib/python2.7/dist-packages/univention/s4connector/s4/password.py", line 72, in calculate_krb5key keys.append(heimdal.asn1_encode_key(key, None, kvno)) TypeError: argument 2 must be heimdal.krb5Salt, not None
[4.4-3] d429b13426 Bug #50475: univention-python-heimdal 9.0.0-3A~4.4.0.202001151023 doc/errata/staging/univention-python-heimdal.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) [4.4-3] 2cd5de4baf Bug #49383 heimdal: Replace magic KRB5_ENCTYPEs base/univention-python-heimdal/module.c | 52 +++++++++++++++++++++++++++++ base/univention-python-heimdal/test.py | 58 ++++++++++++++++----------------- 2 files changed, 81 insertions(+), 29 deletions(-) [4.4-3] 6271592ae7 Bug #49383 heimdal: Initialize variables base/univention-python-heimdal/asn1.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) [4.4-3] 78c4992e35 Bug #49383 heimdal: Fix asn1_encode_key(..., None, ...) base/univention-python-heimdal/asn1.c | 11 +++++++---- base/univention-python-heimdal/test.py | 12 ++++++++++++ 2 files changed, 19 insertions(+), 4 deletions(-) Package: univention-python-heimdal Version: 9.0.0-3A~4.4.0.202001151023 Branch: ucs_4.4-0 Scope: errata4.4-3 FYI: There are still 5 more cleanup patches for S4C sync_krbtgt in my <git:phahn/49139_dhpy2-heimdal> which I created by debugging the last regression.
* Fix reference counting / object ownership for some dict and list elements * Initialize context in asn1_decode_key * Add TestRefcount * Several additions of PyErr_NoMemory * Fix ccache_destroy() and test with TestCcache * Make PyArg_ParseTuple check python method agument types && test with test_type * Initialize structures properly with {0}/NULL/memset(0) * Rename *_destroy to *_dealloc (context, creds, enctype, keyblock, realm, ticket) * Use krb5ContextObject instead of raw krb5_context to implement proper reference counting. * Include krb5_context and krb5_keytab structures directly in krb5KeytabObject structure and let Python handle the memory allocation. * Disable unused functions (realm_from_realm, salt_from_salt) * Explicitly declare methods without arguments as METH_NOARGS * Declare functions as static that are only required internally * Move static variables from header files to c files * Terminate method arrays by {NULL} element * Whitespace cleanup and adjust to "new" german spelling * Don't memset immutable Python strings * Fix code typos (broken enctype_obj>enctype instead of enctype_obj->enctype) * Extend unit test
<http://errata.software-univention.de/ucs/4.4/477.html>