On Wed, Feb 27, 2013 at 09:50:22AM -0500, Simo Sorce wrote: > On Wed, 2013-02-27 at 09:13 -0500, Simo Sorce wrote: > > On Wed, 2013-02-27 at 11:24 +0100, Jakub Hrozek wrote: > > > On Mon, Jan 14, 2013 at 11:33:46AM +0100, Pavel Březina wrote: > > > > On 01/12/2013 09:27 PM, Simo Sorce wrote: > > > > >Rebased on top of current master. > > > > > > > > > >Simo. > > > > > > > > > >Simo Sorce (2): > > > > > krb5_child style fix > > > > > Refactor krb5 child > > > > > > > > > > src/providers/krb5/krb5_child.c | 512 > > > > > +++++++++++++++++---------------------- > > > > > 1 files changed, 217 insertions(+), 295 deletions(-) > > > > > > > > Patch 1 ack. > > > > > > > > Patch 2 nack: > > > > > > > > src/providers/krb5/krb5_child.c: In function ‘main’: > > > > src/providers/krb5/krb5_child.c:2013:8: warning: ‘ret’ may be used > > > > uninitialized in this function [-Wmaybe-uninitialized] > > > > > > > > $ echo $CFLAGS > > > > -m64 -mtune=generic -O3 -fstack-protector-all -Wall -Wextra > > > > -Wno-sign-compare -Wno-unused-parameter > > > > > > > > > > Nack, the new krb5_child allows login with *any* password. > > > > How lovely ... > > Maybe I should test with a wrong password next time :-) > > > > > Seems like we're > > > not handling error codes properly: > > > > > > [[sssd[krb5_child[15317]]]] [sss_child_krb5_trace_cb] (0x4000): [15317] > > > 1361959672.153511: Received error from KDC: -1765328353/Decrypt integrity > > > check failed > > > [[sssd[krb5_child[15317]]]] [get_and_save_tgt] (0x0020): 941: > > > [-1765328353][Decrypt integrity check failed] > > > [[sssd[krb5_child[15317]]]] [kerr_handle_error] (0x0020): 994: > > > [-1765328353][Decrypt integrity check failed] > > > [[sssd[krb5_child[15317]]]] [prepare_response_message] (0x0400): Building > > > response for result [0] > > > [[sssd[krb5_child[15317]]]] [pack_response_packet] (0x2000): response > > > packet size: [54] > > > [[sssd[krb5_child[15317]]]] [k5c_send_data] (0x4000): Response sent. > > > [[sssd[krb5_child[15317]]]] [main] (0x0400): krb5_child completed > > > successfully > > > [sssd[be[IPA]]] [read_pipe_handler] (0x0400): EOF received, client > > > finished > > > [sssd[be[IPA]]] [parse_krb5_child_response] (0x1000): child response > > > [0][3][42]. > > > ^^^^^^ > > > The response should not be 0 here > > > [sssd[be[IPA]]] [fo_set_port_status] (0x0100): Marking port 0 of server > > > 'vm-101.idm.lab.bos.redhat.com' as 'working' > > > [sssd[be[IPA]]] [set_server_common_status] (0x0100): Marking server > > > 'vm-101.idm.lab.bos.redhat.com' as 'working' > > > [sssd[be[IPA]]][cc_dir_cache_for_princ] (0x0400): No principal for > > > u...@idm.lab.bos.redhat.com in DIR:/run/user/782400060/krb5cc > > > [sssd[be[IPA]]] [krb5_auth_done] (0x0020): No ccache for > > > u...@idm.lab.bos.redhat.com in DIR:/run/user/782400060/krb5cc? > > > > > > etc..but the auth succeeds. I have also tested your branch with the > > > new Kerberos auth codes and there access is denied properly. But I > > > really don't want to push a security problem to master knowingly, sorry. > > > > Not a problem, let me check what is going on and send revised patches. > > > > Thanks a lot for thorough testing, it would be nice if we could find > > this type of errors using the new cmocka test stuff. > > > > Simo. > > > > Ok there was an error in which return variable was used in some > functions which was canceled out by the refactoring of the error code > returns. > > Attached find rebased patch that fixes this problem. > > Simo.
Ack to both now. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
