While looking at the previous patch I saw that we were still using the sss_nss_ prefix for a number of functions that are used by both the pam and nss responders. Changed the prefix to sss_cli_ so it is more clear those functions are not nss specific.
Simo.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 02/07/2011 10:11 AM, Simo Sorce wrote:
While looking at the previous patch I saw that we were still using the sss_nss_ prefix for a number of functions that are used by both the pam and nss responders. Changed the prefix to sss_cli_ so it is more clear those functions are not nss specific.
Nack. If we're going to change these, we should be changing their return types from nss_status to something neutral as well.
- -- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
On Tue, 08 Feb 2011 07:21:46 -0500 Stephen Gallagher sgallagh@redhat.com wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 02/07/2011 10:11 AM, Simo Sorce wrote:
While looking at the previous patch I saw that we were still using the sss_nss_ prefix for a number of functions that are used by both the pam and nss responders. Changed the prefix to sss_cli_ so it is more clear those functions are not nss specific.
Nack. If we're going to change these, we should be changing their return types from nss_status to something neutral as well.
I thought about that when changing the names of the functions. But it would be a much more intrusive patch. Although if you are ok with that I can give it a go.
Simo.
----- Original Message -----
On Tue, 08 Feb 2011 07:21:46 -0500 Stephen Gallagher sgallagh@redhat.com wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 02/07/2011 10:11 AM, Simo Sorce wrote:
While looking at the previous patch I saw that we were still using the sss_nss_ prefix for a number of functions that are used by both the pam and nss responders. Changed the prefix to sss_cli_ so it is more clear those functions are not nss specific.
Nack. If we're going to change these, we should be changing their return types from nss_status to something neutral as well.
I thought about that when changing the names of the functions. But it would be a much more intrusive patch. Although if you are ok with that I can give it a go.
New patch that also changes error codes.
Simo.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 02/08/2011 10:29 AM, Simo Sorce wrote:
----- Original Message -----
On Tue, 08 Feb 2011 07:21:46 -0500 Stephen Gallagher sgallagh@redhat.com wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 02/07/2011 10:11 AM, Simo Sorce wrote:
While looking at the previous patch I saw that we were still using the sss_nss_ prefix for a number of functions that are used by both the pam and nss responders. Changed the prefix to sss_cli_ so it is more clear those functions are not nss specific.
Nack. If we're going to change these, we should be changing their return types from nss_status to something neutral as well.
I thought about that when changing the names of the functions. But it would be a much more intrusive patch. Although if you are ok with that I can give it a go.
New patch that also changes error codes.
Ack.
- -- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 02/10/2011 03:13 PM, Stephen Gallagher wrote:
On 02/08/2011 10:29 AM, Simo Sorce wrote:
----- Original Message -----
On Tue, 08 Feb 2011 07:21:46 -0500 Stephen Gallagher sgallagh@redhat.com wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 02/07/2011 10:11 AM, Simo Sorce wrote:
While looking at the previous patch I saw that we were still using the sss_nss_ prefix for a number of functions that are used by both the pam and nss responders. Changed the prefix to sss_cli_ so it is more clear those functions are not nss specific.
Nack. If we're going to change these, we should be changing their return types from nss_status to something neutral as well.
I thought about that when changing the names of the functions. But it would be a much more intrusive patch. Although if you are ok with that I can give it a go.
New patch that also changes error codes.
Ack.
Revoking my ack. Further testing reveals that this broke something in pam_sss and auth is now failing.
- -- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
On Fri, 11 Feb 2011 09:02:37 -0500 Stephen Gallagher sgallagh@redhat.com wrote:
New patch that also changes error codes.
Ack.
Revoking my ack. Further testing reveals that this broke something in pam_sss and auth is now failing.
Changing the error codes revealed an issue in the pam paths. Updated patch to properly map errors from SST_STATUS_ errors to PAM_ errors and properly check for PAM_ errors in the caller instead of NSS_STATUS_ errors.
Simo.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 02/11/2011 09:31 AM, Simo Sorce wrote:
On Fri, 11 Feb 2011 09:02:37 -0500 Stephen Gallagher sgallagh@redhat.com wrote:
New patch that also changes error codes.
Ack.
Revoking my ack. Further testing reveals that this broke something in pam_sss and auth is now failing.
Changing the error codes revealed an issue in the pam paths. Updated patch to properly map errors from SST_STATUS_ errors to PAM_ errors and properly check for PAM_ errors in the caller instead of NSS_STATUS_ errors.
Nack. This fixed an obvious bug, but it's not the one breaking auth.
- -- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 02/11/2011 09:52 AM, Stephen Gallagher wrote:
On 02/11/2011 09:31 AM, Simo Sorce wrote:
On Fri, 11 Feb 2011 09:02:37 -0500 Stephen Gallagher sgallagh@redhat.com wrote:
New patch that also changes error codes.
Ack.
Revoking my ack. Further testing reveals that this broke something in pam_sss and auth is now failing.
Changing the error codes revealed an issue in the pam paths. Updated patch to properly map errors from SST_STATUS_ errors to PAM_ errors and properly check for PAM_ errors in the caller instead of NSS_STATUS_ errors.
Nack. This fixed an obvious bug, but it's not the one breaking auth.
Turns out that this was in fact the right bug, but a missing "break" caused it to fall through to failure anyway.
Attached a new patch with a fix for this, as well as a clean up of our variable usage in that function (forcing too many different return types into the 'ret' variable is a recipe for mistakes)
- -- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
On Fri, Feb 11, 2011 at 10:54:09AM -0500, Stephen Gallagher wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 02/11/2011 09:52 AM, Stephen Gallagher wrote:
On 02/11/2011 09:31 AM, Simo Sorce wrote:
On Fri, 11 Feb 2011 09:02:37 -0500 Stephen Gallagher sgallagh@redhat.com wrote:
New patch that also changes error codes.
Ack.
Revoking my ack. Further testing reveals that this broke something in pam_sss and auth is now failing.
Changing the error codes revealed an issue in the pam paths. Updated patch to properly map errors from SST_STATUS_ errors to PAM_ errors and properly check for PAM_ errors in the caller instead of NSS_STATUS_ errors.
Nack. This fixed an obvious bug, but it's not the one breaking auth.
Turns out that this was in fact the right bug, but a missing "break" caused it to fall through to failure anyway.
Attached a new patch with a fix for this, as well as a clean up of our variable usage in that function (forcing too many different return types into the 'ret' variable is a recipe for mistakes)
passes my tests.
ACK
bye, Sumit
Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk1VW6EACgkQeiVVYja6o6PtUQCgk+OMHDU1oubNwLdA4WZDANUY PaAAn2xYb82Xxw649eTXw6Xcf+mxiOqt =UDFV -----END PGP SIGNATURE-----
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 02/11/2011 11:12 AM, Sumit Bose wrote:
On Fri, Feb 11, 2011 at 10:54:09AM -0500, Stephen Gallagher wrote: On 02/11/2011 09:52 AM, Stephen Gallagher wrote:
On 02/11/2011 09:31 AM, Simo Sorce wrote:
On Fri, 11 Feb 2011 09:02:37 -0500 Stephen Gallagher sgallagh@redhat.com wrote:
>> New patch that also changes error codes. > > > Ack. >
Revoking my ack. Further testing reveals that this broke something in pam_sss and auth is now failing.
Changing the error codes revealed an issue in the pam paths. Updated patch to properly map errors from SST_STATUS_ errors to PAM_ errors and properly check for PAM_ errors in the caller instead of NSS_STATUS_ errors.
Nack. This fixed an obvious bug, but it's not the one breaking auth.
Turns out that this was in fact the right bug, but a missing "break" caused it to fall through to failure anyway.
Attached a new patch with a fix for this, as well as a clean up of our variable usage in that function (forcing too many different return types into the 'ret' variable is a recipe for mistakes)
passes my tests.
ACK
Pushed to master.
- -- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
sssd-devel@lists.fedorahosted.org
-
Simo Sorce -
Stephen Gallagher -
Sumit Bose