| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
of the pre-allocated size
Some SMB servers such as HDS HNAS (Hitachi NAS) return error
NT Status: STATUS_INVALID_SECURITY_DESCR (0xc0000079)
during set cifs acl operation.
This happens due to mismatch in the size of actual security descriptor
being set versus the size of the security descriptor stated in the request.
Instead of sending allocated buffer size of a security descriptor,
send the actual size of the security descriptor during set cifs acl
operation.
Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@gmail.com>
|
|
A Coverity scan turned up this warning:
1. cifs-utils-6.2/setcifsacl.c:578:result_independent_of_operands – "(x & 0xff0000000000ULL) >> 48" is 0 regardless of the values of its operands. This occurs as the operand of assignment.
...which is entirely true. That shift should be 40 bits, not 48. Also
fix a similar bug in getcifsacl.c.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
Allow setcifsacl to function even in the case where the plugin can't
be initialized. ID mapping of course won't work, but we can still allow
it to accept "raw" SID strings.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
setcifsacl_facenum_ret: is called only if attrlen is equal to -1.
Signed-off-by: Peng Haitao <penght@cn.fujitsu.com>
|
|
Previously the code assumed that the ACE that was copied was of a
fixed size. Save off the return value from copy_ace and ensure that
we apply it correctly to the size and offset.
Reported-by: Jian Li <jiali@redhat.com>
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
Jian pointed out that this loop can cycle infinitely when the string
contains a ','.
Also, fix typo in manpage that shows a trailing ',' in one example.
Reported-by: Jian Li <jiali@redhat.com>
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
Add str_to_sid() functionality to the plugin API and have setcifsacl
use it.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
setcifsacl doesn't use syslog, so no need to open a channel to it.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
setcifsacl.c:833: warning: format '%ld' expects type 'long int', but
argument 3 has type 'ssize_t'
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
Since they are not necessarily aligned the same and potentially store
their fields with different endianness. Copy from the wbcDomainSid
to the cifs_sid as appropriate.
Also rename the same function in cifs.idmap.c for consistency.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
The size must also be kept in little-endian.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
Change verify_ace_mask to just attempt to convert the argument to an
unsigned long first. If that fails, then try to treat it as a symbolic
mask string.
Also, clean up ace_mask_value. There's no need to walk the string
twice. Walk it once and turn the single-char mask checks into a switch
statement instead of if/else clauses.
Finally, fix the endianness of the resulting value. It must be in LE.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
Pavel Raiskup found the following defect in setcifsacl with Coverity:
"segfault may occur also in cifs-utils-4.8.1/setcifsacl.c|644| because
of casesptr dereferencing. When you look e.g. at the line 605, in this
time any part of 'caseptr' may be yet uninitialized and program is going
through 'goto' to freeing -> and there you are freeing the 'caseptr[i]'
address."
The analysis there seems a little off, but is basically correct. The
freeing loop counts down from the current value of i to free the
secondary allocations here.
There is one situation though where this could go badly. If the strtok
parsing near the beginning of the loop fails, then we could end up
trying to free an uninitialized pointer.
Fix this by changing the cacesptr allocation to use calloc(), and stop
trying to be clever with the freeing loop. Just have it walk the
entire array and attempt to free each slot.
Reported-by: Pavel Raiskup <praiskup@redhat.com>
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
Pavel Raiskup reported the following defects that he found with Coverity:
"If the variable 'facesptr' on line cifs-utils-4.8.1/setcifsacl.c|365|
has not enough memory to be allocated, program 'setcifsacl' will fail
with segfault on line 365 (dereferencing facesptr)."
"you may return freed pointer here. There is some kind of return code
('rc') which should be transferred to >NULL< when is rc nonzero (and
returned)"
There are also a couple of other bugs here:
malloc doesn't necessarily set errno to anything when an allocation
fails, so having the error handling rely on that is wrong.
Fix all of these bugs by reorganzing this function to fix up the error
handling.
Reported-by: Pavel Raiskup <praiskup@redhat.com>
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
...instead of open-coding it thrice.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
The current method of trying to convert a name to a password struct and
then back to a SID is just weird. It also doesn't seem to work correctly.
Instead, look for a '\\' in the string. If there isn't one then try to
convert it directly to a SID.
If there is a '\\' or the direct-to-SID conversion didn't work, then
use wbcLookupName to do the conversion directly to a SID instead.
Also, fix the error handling. These routines return a wbcErr, so we
should use their macros to check whether it worked or not.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
One of the reasons to use "goto" in an error condition is to eliminate
unnecessary indentation. Fix that here by revering some error checks
end getting rid of some unneeded "else" cases.
After using strstr() to find "ACL:", there's no need to then use
strchr() to find ':'. We know where it is -- it's 3 bytes past the
current position.
Finally, there's no need to copy these strings into new buffers,
just set the pointers in the array to their original values.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
No need to walk the string twice or to hand-roll our own version of
strchr(). Also, move the check for no argument out into main().
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
Don't use htole32 when you really want le32toh. Also, when copying or
comparing ACEs, it's incorrect to convert the endianness of these
fields. Let's just keep things simple and declare that multibyte fields
in all of these structs are always kept in little-endian.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
Winbind keeps SID fields in host-endian format, but setcifsacl doesn't
currently account for that. Make sure that when we get a valid SID
from wbc that we convert the subauth fields to little-endian, which
the server will expect. The other fields are single bytes and don't
need conversion.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
'?' has a special meaning in getopt(). It means that the option
character was not recognized. You can override that behavior by making
':' the first character of the optstring, but that wasn't done here. I'm
not sure what the effect of having '?' in the actual optstring is in
this case, but it's probably best not to put it in there.
Remove '?' from the optstring and replace it 'h'. Also add '-h' as a
valid option to the manpage.
'-v' doesn't require an argument, so fix the optstring to reflect that.
Finally declare a new variable to hold optarg. Currently we only call
getopt() once, which is a little odd. Eventually we may want to make it
call it more than once, in which case we'll need some way to store the
optarg on each pass.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
...instead of relying on magic values of an int.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
This saves a tiny bit of memory, and doesn't make the program assume
that the binary is named something in particular.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
pntsd is never NULL here, and get rid of extra "else" that adds some
unneeded indentation.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
copy_sec_desc() copies the owner and group SIDs from one security
descriptor to another. Unfortunately, it doesn't take into account the
fact that these are variable length and routinely overruns the SID
structure when doing this copy and scribbles over the destination ACL.
This wasn't noticed before the change in the maximum number of subauths
because the code either overwrote the damage afterward, or the overrun
part was the same between source and destination anyway. Now that the
max number of subauths is 15, it's more noticable.
Fix it to only copy the number of subauths that claimed in the buffer
instead.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
The max number of subauthorities on windows and in winbind is generally
15, not 5. If winbind sends more than 5, then this code may end up
overrunning the buffer. Also, define some preprocessor constants and
use those instead of hardcoding '5' and '6' all over the place.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
When I moved the manpages for this to section 1, I missed some references
to them. Also, get rid of the unneeded clean-local-aclprogs makefile target.
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
...and eliminate this build warning:
setcifsacl.c: In function ‘build_cmdline_aces’:
setcifsacl.c:582:9: warning: variable ‘rc’ set but not used [-Wunused-but-set-variable]
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
Signed-off-by: Jeff Layton <jlayton@samba.org>
|
|
Parse the blob that contains a security descriptor obtained by
calling getxattr API using attribute system.cifs_acl .
Start parsing and printing security descriptor including
the a DACL within the security descriptor, printing each ACE of
the DACL by printing SID, type, flags, and mask.
Winbind apis are used to translate raw SID to a name.
Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@gmail.com>
|
