389ds / 389-ds-base

The enterprise-class Open Source LDAP server for Linux
https://www.port389.org/
Other
211 stars 91 forks source link

Internal operation updating the password triggers a crash #2269

Closed 389-ds-bot closed 4 years ago

389-ds-bot commented 4 years ago

Cloned from Pagure issue: https://pagure.io/389-ds-base/issue/49210


Issue Description

The bug is a side effect of https://pagure.io/389-ds-base/issue/49039

(gdb) where
0  0x00007fbd027f03b7 in op_shared_allow_pw_change (pb=pb@entry=0x5634932b8d00, mod=0x5634933d6c20,
    old_pw=old_pw@entry=0x7fbcb97794d8, smods=smods@entry=0x7fbcb97794e0) at ldap/servers/slapd/modify.c:1241
1  0x00007fbd027f223f in modify_internal_pb (pb=pb@entry=0x5634932b8d00) at ldap/servers/slapd/modify.c:573
2  0x00007fbd027f2c93 in slapi_modify_internal_pb (pb=pb@entry=0x5634932b8d00) at ldap/servers/slapd/modify.c:454
3  0x00007fbcf50f9d10 in ipapwd_apply_mods (dn=0x5634932f8e40 "uid=admin,cn=users,cn=accounts,dc=testrelm,dc=test",
    mods=0x5634933d66c0) at common.c:950
4  0x00007fbcf50fa3f9 in ipapwd_SetPassword (krbcfg=krbcfg@entry=0x5634932c8d50, data=data@entry=0x7fbcb97797c0, is_krb=1)
    at common.c:866
5  0x00007fbcf510078f in ipapwd_chpwop (krbcfg=0x5634932c8d50, pb=0x7fbcb9779a50) at ipa_pwd_extop.c:577
6  ipapwd_extop (pb=0x7fbcb9779a50) at ipa_pwd_extop.c:1770
7  0x000056348c5cb451 in do_extended (pb=pb@entry=0x7fbcb9779a50) at ldap/servers/slapd/extendop.c:348
8  0x000056348c5c45e8 in connection_dispatch_operation (pb=0x7fbcb9779a50, op=0x56348f6dfa00, conn=0x563492edf240)
    at ldap/servers/slapd/connection.c:688
9  connection_threadmain () at ldap/servers/slapd/connection.c:1772
10 0x00007fbd00b699bb in _pt_root (arg=0x563492e0ca20) at ../../../nspr/pr/src/pthreads/ptthread.c:216
11 0x00007fbd00509e25 in start_thread (arg=0x7fbcb977a700) at pthread_create.c:308
12 0x00007fbcffdeb34d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113

Package Version and Platform

389-ds-base-1.3.6.1-6.el7.x86_64

Steps to reproduce

  1. It is systematically reproducing using ipa plugin (ipa-pwd-extop) that triggers an internal update of password (see https://bugzilla.redhat.com/show_bug.cgi?id=1438724

  2. I "guess" it is reproducible if using password update EXTOP

Actual results

Crash

Expected results

389-ds-bot commented 4 years ago

Comment from tbordaz (@tbordaz) at 2017-04-04 17:53:21

Metadata Update from @tbordaz:

389-ds-bot commented 4 years ago

Comment from mreynolds (@mreynolds389) at 2017-04-04 18:41:57

Metadata Update from @mreynolds389:

389-ds-bot commented 4 years ago

Comment from mreynolds (@mreynolds389) at 2017-04-05 04:33:56

0001-Issue-49210-Fix-regression-when-checking-is-password.patch

389-ds-bot commented 4 years ago

Comment from mreynolds (@mreynolds389) at 2017-04-05 04:34:01

Metadata Update from @mreynolds389:

389-ds-bot commented 4 years ago

Comment from firstyear (@Firstyear) at 2017-04-05 05:04:18

There are some easier ways to setup TLS/SSL for the test case. Have a look at the RSA/Encryption objects in lib389/config.py, as well as the NSS wrapper. Makes it much simpler. An example is in dirsrvtests/suites/sasl/plain.py

Is the check for needpw because ipa-pwd-extop "gets in the way" of our password change? How does this logic change fix the issue (for my understanding ) :)

Thanks!

389-ds-bot commented 4 years ago

Comment from mreynolds (@mreynolds389) at 2017-04-05 16:36:35

There are some easier ways to setup TLS/SSL for the test case. Have a look at the RSA/Encryption objects in lib389/config.py, as well as the NSS wrapper. Makes it much simpler. An example is in dirsrvtests/suites/sasl/plain.py

Thanks I'll check it out.

Is the check for needpw because ipa-pwd-extop "gets in the way" of our password change? How does this logic change fix the issue (for my understanding ) :)

The issue is that the server is crashing (triggered by an IPA plugin). This fix addresses the null pointer dereference, while maintaining the correct/expected behavior.

Thanks!

389-ds-bot commented 4 years ago

Comment from mreynolds (@mreynolds389) at 2017-04-05 17:07:22

New patch attached that updates the CI test as requested by Firstyear

0001-Issue-49210-Fix-regression-when-checking-is-password.patch

389-ds-bot commented 4 years ago

Comment from firstyear (@Firstyear) at 2017-04-06 01:04:42

Looks great to me, thanks for the changes.

389-ds-bot commented 4 years ago

Comment from firstyear (@Firstyear) at 2017-04-06 01:04:45

Metadata Update from @Firstyear:

389-ds-bot commented 4 years ago

Comment from mreynolds (@mreynolds389) at 2017-04-06 16:58:15

master updated (a96f833 -> f5e4b8a)

389-ds-base-1.3.5 updated (a66484c -> 0ebc21e)

389-ds-base-1.3.4 updated (294ebbd -> bad9197)

389-ds-bot commented 4 years ago

Comment from mreynolds (@mreynolds389) at 2017-04-06 16:58:20

Metadata Update from @mreynolds389: