On Sun, 8 Dec 2024 08:13:38 +0900
Takashi Yano wrote:
> On Tue, 19 Nov 2024 21:54:44 +0100
> Corinna Vinschen wrote:
> > On Nov 19 17:58, Takashi Yano via Cygwin wrote:
> > > On Mon, 18 Nov 2024 17:26:12 +0100
> > > Corinna Vinschen wrote:
> > > > We can safely assume that the current user is already authorized on the
> > > > SMB server. So... shouldn't AuthzInitializeResourceManager be
> > > > sufficient and the code from class authz_ctx already does what we want?
> > > > We may just have to use in in place of calling NtCheckAccess(),
> > > > maybe with a tweak or two...
> > >
> > > I already tried AuthzInitializeResourceManager(), but the result
> > > was the same with current implementation...
> >
> > So you tried to call authz_get_user_attribute()?
>
> Yes. But resulted in the same.
>
> > > BTW, I come up with another implementation. This make the things
> > > much simpler. What do you think of the patch attached?
> >
> > > [...]
> > > int
> > > check_file_access (path_conv &pc, int flags, bool effective)
> > > {
> > > @@ -711,10 +618,14 @@ check_file_access (path_conv &pc, int flags, bool
> > > effective)
> > > desired |= FILE_EXECUTE;
> > > if (!get_file_sd (pc.handle (), pc, sd, false))
> > > {
> > > - /* Tweak Samba security descriptor as necessary. */
> > > - if (pc.fs_is_samba ())
> > > - convert_samba_sd (sd);
> > > - ret = check_access (sd, file_mapping, desired, flags, effective);
> > > + HANDLE h = CreateFileW (pc.get_nt_native_path ()->Buffer, desired,
> > > + 0, NULL, OPEN_EXISTING,
> > > + FILE_FLAG_BACKUP_SEMANTICS, NULL);
> > > + if (h != INVALID_HANDLE_VALUE)
> > > + {
> > > + CloseHandle (h);
> > > + ret = 0;
> > > + }
> > > }
> > > debug_printf ("flags %y, ret %d", flags, ret);
> > > return ret;
> >
> > No, we can't do that, it's too simple.
> >
> > Just kidding.
> >
> > This is so simple, I'm puzzled we never tried that before. Or, if we
> > did, it's a loooong time ago...
> >
> > If we really do this, we don't even need to call get_file_sd(). And it
> > should use NtOpenFile and reopen semantics i.e. pc.init_reopen_attr().
> > Also, the sharing flags should allow all access. And the `effective'
> > argument needs to be taken into account.
>
> I have a question. What pc.init_reopen_attr() is for? I tested with
> pc.get_object_attr() instead, it works. What handle should I pass
> to pc.init_reopen_attr()?
>
> Anyway, I revised the patch as attached. What do you think?
diff --git a/winsup/cygwin/sec/base.cc b/winsup/cygwin/sec/base.cc
index fcc5e1ff7..68e7fd9e1 100644
--- a/winsup/cygwin/sec/base.cc
+++ b/winsup/cygwin/sec/base.cc
@@ -643,6 +643,8 @@ check_file_access (path_conv &pc, int flags, bool effective)
NtClose (h);
ret = 0;
}
+ else
+ __seterrno_from_nt_status (status);
if (!effective && cygheap->user.issetuid ())
{
/* Recover impersonation token */
--
Takashi Yano <takashi.y...@nifty.ne.jp>
--
Problem reports: https://cygwin.com/problems.html
FAQ: https://cygwin.com/faq/
Documentation: https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple
