Re: Win32 bug in krb5_rc_io_destroy

2011-07-01 Thread Jeffrey Altman
This is just one symptom of a more fundamental bug.  The replay cache
code as implemented is full of race conditions because of the reliance
on the C Run time library file operations instead of native Win32
operations.  In addition to not permitting files to be opened with the
correct share modes, they cannot be opened with the correct access
control lists.  The RTL operations have the same names as those on Unix
but they do not have the same behaviors.  unlink() is a perfect example.

Small patches to the replay cache will not create a replay cache that is
safe to use on Windows.  Instead a new Windows specific implementation
is required.

Jeffrey Altman


On 7/1/2011 2:31 AM, checker wrote:
> 
> Hi, the unlink(d->fn) at the top of krb5_rc_io_destroy fails on Win32
> because the file was opened without FILE_SHARE_DELETE (since it's
> opened through _open there's actually no way to set that flag without
> marking it a temporary file), so I patched it to close the file
> first.  I assume this is a bug?  I also assume no one noticed this
> since most of the code calls krb5_rc_close rather than
> krb5_rc_destroy?  I found it while getting sim_client.c to work with
> my build.
> 
> Should I file a bug, or is this enough (the bug list says to post here
> first)?
> 
> Thanks,
> Chris
> 
> === modified file 'src/lib/krb5/rcache/rc_io.c'
> --- src/lib/krb5/rcache/rc_io.c   2011-04-09 08:50:00 +
> +++ src/lib/krb5/rcache/rc_io.c   2011-07-01 06:18:12 +
> @@ -481,6 +481,13 @@
>  krb5_error_code
>  krb5_rc_io_destroy(krb5_context context, krb5_rc_iostuff *d)
>  {
> +#if defined(_WIN32)
> +// the file isn't opened with FILE_SHARE_DELETE so we need to
> close it first
> +if(close(d->fd) == -1) {
> +return KRB5_RC_IO_UNKNOWN;
> +}
> +d->fd = -1;
> +#endif
>  if (unlink(d->fn) == -1)
>  switch(errno)
>  {
> 
> 
> 
> 
> Kerberos mailing list   Kerberos@mit.edu
> https://mailman.mit.edu/mailman/listinfo/kerberos



signature.asc
Description: OpenPGP digital signature

Kerberos mailing list   Kerberos@mit.edu
https://mailman.mit.edu/mailman/listinfo/kerberos


Win32 bug in krb5_rc_io_destroy

2011-07-01 Thread checker

Hi, the unlink(d->fn) at the top of krb5_rc_io_destroy fails on Win32
because the file was opened without FILE_SHARE_DELETE (since it's
opened through _open there's actually no way to set that flag without
marking it a temporary file), so I patched it to close the file
first.  I assume this is a bug?  I also assume no one noticed this
since most of the code calls krb5_rc_close rather than
krb5_rc_destroy?  I found it while getting sim_client.c to work with
my build.

Should I file a bug, or is this enough (the bug list says to post here
first)?

Thanks,
Chris

=== modified file 'src/lib/krb5/rcache/rc_io.c'
--- src/lib/krb5/rcache/rc_io.c 2011-04-09 08:50:00 +
+++ src/lib/krb5/rcache/rc_io.c 2011-07-01 06:18:12 +
@@ -481,6 +481,13 @@
 krb5_error_code
 krb5_rc_io_destroy(krb5_context context, krb5_rc_iostuff *d)
 {
+#if defined(_WIN32)
+// the file isn't opened with FILE_SHARE_DELETE so we need to
close it first
+if(close(d->fd) == -1) {
+return KRB5_RC_IO_UNKNOWN;
+}
+d->fd = -1;
+#endif
 if (unlink(d->fn) == -1)
 switch(errno)
 {




Kerberos mailing list   Kerberos@mit.edu
https://mailman.mit.edu/mailman/listinfo/kerberos