Re: changes to struct rpc_gss_sec

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Nov 27, 2023 at 11:42:08AM -1000, Olga Kornievskaia wrote:
> Hi Chuck,
> 
> Given that rpc_gss_secreate() was written by you I hope you dont mind
> questions. I believe gssd can't use the new api because it is
> insufficient. Specifically, authgss_create_default() takes in a
> structure which is populated with the correct (kerberos) credential we
> need to be using for the gss context establishment.
> rpc_gss_seccreate() sets the sec.cred = GSS_C_NO_CREDENTIAL. If you
> believe I'm incorrect in my assessment that rpc_gss_secreate please
> let me know.

Caller can pass its preferred credential in via the *req parameter
(parameter 6). Then rpc_gss_seccreate(3t) does this:

846         if (req) {
847                 sec.req_flags = req->req_flags;
848                 gd->time_req = req->time_req;
849                 sec.cred = req->my_cred;
850                 gd->icb = req->input_channel_bindings;
851         }

Wouldn't that work?


> As far as I can see, current libtirpc needs to be modified no matter
> what.

I'm not convinced of that yet.


> Perhaps there needs to be some config magic to demand the use of
> higher version of libtirpc for the new code but it would be just a
> different way an upstream nfs-utils won't build without an appropriate
> libtirpc version.

Agreed, 4b272471937d ("gssd: handle KRB5_AP_ERR_BAD_INTEGRITY for
machine credentials") should have added some config magic.

If the libtirpc version is too low, then the new GSS status checking
logic you added can't be used, and it should fall back to the old
logic via #ifdef.


> I would imagine distros would build matching
> libtirpc and nfs-utils that would either both not have the fix or have
> the fix.

I don't think distros work that way unless they are forced to.
There doesn't seem to be a reason why the nfs-utils change has
to break things -- we can do this without the ABI disruption.

The change to struct rpc_gss_sec can't remain. IMO both

  f5b6e6fdb1e6 ("gss-api: expose gss major/minor error in authgss_refresh()")

and

  4b272471937d ("gssd: handle KRB5_AP_ERR_BAD_INTEGRITY for machine credentials")

need to be reverted first.

Then a patch that replaces the authgss_create_default(3) call with
rpc_gss_seccreate(3t) can be applied, as long as it contains new
configure.ac logic to fall back to using authgss_create_default(3)
if rpc_gss_seccreate(3t) is not available in libtirpc.

That should enable nfs-utils to be built no matter what version of
libtirpc is available in the build environment.


> On Wed, Nov 22, 2023 at 4:31 AM Chuck Lever III <chuck.lever@xxxxxxxxxx> wrote:
> >
> > Possibly because authgss_create_default() was the API
> > available to gssd back in the day. rpc_gss_seccreate(3t)
> > is newer. That would be my guess.
> >
> >
> > > On Nov 22, 2023, at 1:07 AM, Olga Kornievskaia <aglo@xxxxxxxxx> wrote:
> > >
> > > Hi Chuck,
> > >
> > > A quick reply as I'm on vacation but I can take a look when I get
> > > back. I'm just thinking there must be a reason why gssd is using the
> > > authgss api and not calling the rpc_gss one.
> > >
> > > On Tue, Nov 21, 2023 at 6:59 AM Chuck Lever III <chuck.lever@xxxxxxxxxx> wrote:
> > >>
> > >> Hey Olga-
> > >>
> > >> I see that f5b6e6fdb1e6 ("gss-api: expose gss major/minor error in
> > >> authgss_refresh()") added a couple of fields in structure rpc_gss_sec.
> > >> Later, there are some nfs-utils changes that start using those fields.
> > >>
> > >> That breaks building the latest upstream nfs-utils on Fedora 38, whose
> > >> current libtirpc doesn't have those new fields.
> > >>
> > >> IMO struct rpc_gss_sec is part of the libtirpc API/ABI, thus we really
> > >> shouldn't change it.
> > >>
> > >> Instead, if gssd needs GSS status codes, can't it call
> > >> rpc_gss_seccreate(3), which explicitly takes a struct
> > >> rpc_gss_options_ret_t * argument?
> > >>
> > >> ie, just replace the authgss_create_default() call with a call to
> > >> rpc_gss_seccreate(3) ....
> > >>
> > >>
> > >> --
> > >> Chuck Lever
> > >>
> > >>
> > >>
> >
> > --
> > Chuck Lever
> >
> >

-- 
Chuck Lever




[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux