| Commit message (Collapse) | Author | Age |
|
|
|
|
|
|
|
|
|
|
| |
Although the previous commit should make it very unlikely we screw up the
subscription sharding, be defensive about waiting for SERVACKs. ZSubscribeTo
does mess up, Z_SendFragmentedNotice will shard with a z_multiuid. In that
case, although the second packet will get a SERVACK, Z_ReadWait kindly drops it
on the floor. The ZIfNotice will then just hang.
Tested by bumping zwgc's BATCH_SIZE up to 200, reverting the previous commit,
and strace.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Header lengths are not constant-size because Zcode escapes bytes 0xFF and 0x00
into two bytes. If we end up filling up close to all the space we have and
Z_SendFragmentedNotice then computes a header length larger than ours by
enough, the message gets fragmented.
Getting it fragmented is especially unfortunate because only the first of a
fragmented notice ever has a SERVACK survive. (They all get SERVACKs, but
libzephyr kindly drops all but the first on the floor.)
This isn't a watertight fix; we may get really really unlucky and blow up 13
bytes in the authenticator and checksum. But that's not likely, and a proper
fix would involve either computing based on the maximum possible authenticator
size (wasteful and hard to bound tightly) or changing to protocol to use a less
inappropriate encoding.
|
|
|
|
| |
Minor memory leak, but we may as well fix it.
|
|
|
|
|
|
| |
With a custom send_routine that mirrors ZSrvSendList. This allows for an
asynchronous version that replaces send_routine with non-blocking versions (and
waits for ACKs out-of-band).
|
|
|
|
|
|
| |
It's only used by ZCancelSubscription, but the server rejects unauthenticated
CLIENT_CANCELSUB requests anyway. The unauthenticated codepath results in a
SERVNAK and doesn't drop subs.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
ACKs to fragmented notices keep the multiuid field, but multipart becomes "".
This is interpreted as 0/z_message_length. This means ACKs to non-initial
fragments look like an initial fragment from the multipart field, but not when
checking uid == multiuid. The result is that they get smashed when passing
through reassembly.
6e8ec12b0ba9d476e065957028e4cf9cf69d6ac2 addressed this. For SERVACKs and
SERVNAKs, it drops all but the initial ones (uid == multipart) on the floor. It
ignores the problem for HMACKs. Normally ZSendPacket blocks on the HMACK before
sending each successive fragment, so there's no opportunity for them to
collide. But if calling ZSrvSendNotice with a custom send_function that doesn't
block, the HMACKs can smash into each other depending on timing.
Instead, fix it by using z_uid instead of z_multiuid as the multiuid key. For
compatibility, keep the SERVACK dropping behavior. (I'd like to get all the
SERVACKs too, but potentially that'll confuse clients somewhat.)
|
|
|
|
| |
May as well put it in .rodata
|
|
|
|
|
| |
I would hope this codepath can never trigger, but good to clean up properly
here.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When Z_ReadWait receives a packet which doesn't start with a zephyr
version header, it considers the packet to be "obviously non-zephyr".
Such packets are discarded and, previously, caused Z_ReadWait to return
ZERR_NONE. Unfortunately, this can cause things to block for up to 60s
when a caller was expecting a non-blocking call to pick up a new packet
if there is one.
This changes Z_ReadWait to return ZERR_BADPKT in this situation,
eliminating the potential wait.
This fixes #100
|
|
|
|
|
|
|
|
| |
Provide a new zctl subcommand, flush_subs, to flush all subscriptions for
a specified recipient. This is implemented using a new library function,
ZFlushUserSubscriptions().
This is the client side of #103
|
|
|
|
|
|
|
|
| |
Provide a new library function, ZFlushUserLocations(), to flush locations
for a specified user. This can be called using zctl flush_locs, which
now takes an optional username parameter.
This is the client side of #102
|
|
|
|
| |
This fixes #94
|
|
|
|
|
|
|
|
| |
If we have no Kerberos credentials, we cannot create a checksum.
This can happen if, for example, we end up with an expired TGT.
In this case, instead of crashing, just leave the zero checksum.
This fixes #80
|
|
|
|
|
|
| |
This can't get subs in the athena realm.
This reverts commit b92153fac201a9a22779817be5f2375f7cf754fc.
|
|
|
|
| |
This fixes #94
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Eliminate compiler warnings due to various issues (listed below). This
allows Zephyr to build cleanly under GCC versions ranging from 4.1.0 to
4.7.2 with all of the options shown below:
-g -O2 -Wall -Werror
-Wno-deprecated-declarations
-Wmissing-declarations
-Wpointer-arith
-Wstrict-prototypes
-Wshadow
-Wextra
-Wno-missing-field-initializers
-Wno-unused-parameter
and, on recent versions, -Wunreachable-code
Test builds were done
- On Ubuntu 12.10 (Quantal Quetzal) using both MIT Kerberos 1.10.1 and
Heimdal 1.6, without krb4 and both with and without C-Ares and Hesiod
- On Fedora 14 using Heimdal 0.6, without C-Ares or Hesiod and both
with and without krb4 (KTH Kerberos 1.3rc2)
- On Fedora Core 3, Fedora Core 5, Fedora 7, and Fedora 10, using
Heimdal 0.6 and without C-Ares, Hesiod, or krb4
It also allows clean builds on Solaris 10 under the Sun Studio 12 (9/07)
C compiler with the following options:
-g -fd -v -errfmt -errhdr=%user -errtags=yes -errwarn=%all
-erroff=E_OLD_STYLE_FUNC_DECL,E_ENUM_TYPE_MISMATCH_ARG,E_ARG_INCOMPATIBLE_WITH_ARG
... and under Solaris 9 with the Sun Forte 7 (3/02) C compiler with the above
options and -erroff=E_FUNC_HAS_NO_RETURN_STMT. Solaris builds were done
with Heimdal 0.6 and without C-Ares, Hesiod, or krb4.
The following types of issues are addressed in this change:
- Parameters and local variables with the same names as library functions
- Parameters and local variables with the same names as globals
- Declarations for exported global variables missing from headers
- Prototypes for exported functions missing from headers
- Missing 'static' on functions that shouldn't be exported
- Old-style function declarations
- Duplicate declarations
- Type mismatches
- Unused variables and functions
- Uninitialized variables
- Forward references to enums
- Necessary header files not included
- Violations of the aliasing rules, where GCC was able to detect them
- Missing braces on if blocks that might be empty
- Attempts to do pointer arithmetic on pointers of type void *, which
is not permitted in standard C.
- An attempt to pass a function pointer via a void * parameter, which is
not permitted in standard C. Instead, we now pass a pointer to a
structure, which then contains the required function pointer.
- Unnecessary inclusion of <krb5_err.h>, which is already included by
<krb5.h> when the former exists, and might not be protected against
double inclusion, depending on which com_err was used.
- Missing include of <com_err.h>, which was masked by the fact that it is
included by headers generated by e2fsprogs compile_et
- Use of com_err() with a non-constant value in place of the format string,
which in every case was a fixed-size buffer in which a message was built
using sprintf(!). Both the calls to sprintf and the fixed-size buffers
have been removed, in favor of just letting com_err() do the formatting.
- Various cases where X library functions expecting a parameter of type
wchar_t * were instead passed a parameter of type XChar2b *. The two
types look similar, but are not the same and are _not_ interchangeable.
- An overly-simplistic configure test which failed to detect existence of
<term.h> on Solaris, due to not including <curses.h>.
- Using the wrong type for the flags output of krb5_auth_con_getflags()
when building against Heimdal. A configure test is added to detect
the correct type.
|
| |
|
| |
|
|
|
|
|
|
|
| |
The result of Z_krb5_verify_cksum is supposed to be nonzero on success and
0 on failure. But if krb5_crypto_init() failed, we were returning the
resulting error code, effectively accepting any checksum, when instead we
should reject the checksum since we cannot verify it.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
From -c shadow on 15-Nov-2011, discussing a problem where some notices
received from other realms were causing clients to crash:
So, the packet that crashed my client had extra garbage beyond what
should have been the end of the packet. So z_multinotice was 0/61,
but the packet was longer than 61. Which means the logic that should
have treated this as an unfragmented notice (because partof ==
z_message_len) did not trigger.
So a holelist gets created, with enough storage for partof, and then
Z_AddNoticeToEntry is called to copy z_message_len (> partof) bytes
into it.
So, I don't know why your client, or the server, or something, is sending
packets longer than the message length, but I don't think I actually want
to just discard those, because then "legitimate" messages would vanish.
Instead, if part + notice->z_message_len > partof, I just want to ignore
the extra.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Bug report from dkindred in libzephyr affecting amd64_fc5:
There's a bug in libzephyr (introduced in version
zephyr-064) that is causing tzc to fail on amd64_fc5:
In /afs/cs/misc/zephyr/src/zephyr-064/lib/ZRecvNot.c line
33, 'nextq' is tested without being initialized (see code
below).
I imagine the appropriate fix is to put that "if (!nextq)"
test just *after* the "nextq = Z_GetFirstComplete();" line
instead of just before.
- Darrell
|
|
|
|
|
|
|
| |
Z_GetFirstComplete() can return NULL; in that case, we don't want to
dereference the pointer it returns.
Extracted from Andrew zephyr/064; authorship uncertain.
|
|
|
|
|
|
|
| |
Have Z_FormatRawHeader call Z_ZcodeFormatRawHeader to reduce duplication
and error. Z_FormatRawHeader was previously adding headers 17 and 18
unconditionally, which was not proper for a server forwarding an unauth
message.
|
| |
|
|
|
|
|
| |
i.e. don't keep generated or foreign stuff in our source tree.
As a side effect, this lets us use a libtool, etc. from this century
|
| |
|
|
|
|
|
|
|
| |
The fact that the Heimdal and MIT APIs are subtly different strikes again.
I am honestly starting to wonder if they make it look this similar just
to frustrate people; I only don't believe it because neither team seems
like that sort of person. Fixes #74.
|
| |
|
| |
|
|
|
|
|
|
|
|
|
| |
Since these are constants used in the protocol be explicit about what values
the C compiler is assigning them, and that they can't be arbitrarily
rearranged.
Also, since we were promising strings for describing them in zephyr.h
actually define the array.
|
| |
|
| |
|
|
|
|
| |
notice->z_authent_authent_len will be zero.
|
|
|
|
|
|
| |
with SO_REUSADDR set on a given port, other people can also open listening
sockets with SO_REUSEADDR set, so turn SO_REUSADDR back off after we've
bound our port.
|
| |
|
|
|
|
| |
the pointer
|
|
|
|
| |
(thanks to wthrowe@mit.edu)
|
|
|
|
| |
wthrowe@mit.edu
|
|
|
|
| |
and it breaks the build when there isn't a utmp.h
|
|
|
|
| |
nuke-trailing-whitespace.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
I am becoming increasingly enamored with the fall-through on error
clean-up-everything-at-the-end style of C error handling and resource
management.
Also remove some misleading/useless/wrong comments.
(also fix a problem in the tickets expired case where it was using
the wrong (possibly undefined) authenticator lengh)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Per Nelson Elhage:
find_or_insert_uid sorts 'buffer' by the uid, which is a
remotely-provided field. However, in order to expire uids, it does:
while (num && (now - buffer[start % size].t) > CLOCK_SKEW)
start++, num--;
start %= size;
i.e. starts from the start of the queue and goes until it finds
something sufficiently new. Since the queue ordering is
attacker-controlled, we can send an arbitrarily-long sequence of
decreasing uids, consuming memory and forcing the client into an
ever-growing quadratic loop to insert them at the beginning.
--
Solve this by not keeping the buffer sorted; just tack the incoming
uids on the end. This way an attacker can make us keep five minutes
worth of UIDs, but only five minutes, and also anecdotally a client
under attack spends all of its CPU sort uids.
|
|
|
|
|
|
|
|
|
|
|
|
| |
krb5 actually checks in mk_req and fails if the ticks are expired,
rather than giving you an authenticator that would fail and, handing
you the session key that you'd already negotiated. This causes (meh)
sending auth to fail as opposed to just ending up unauthentic and
(poor) verifiable messages to look unauthentic or forged.
So get the session key from the ccache without checking the expiration
time, and have the cert routine skip making an authenticator if
krb5_mk_req_extended says the ticket is expired.
|
| |
|
|
|
|
|
|
|
| |
(The time used to be set before the bounds check, so one could
potentially get an accumulation of packets in the queue without
timestamps that could never be assembled into a full notice; thanks to
nelhage@mit.edu for noticing.)
|
|
|
|
| |
(thanks to nelhage@mit.edu for noticing this)
|
| |
|
| |
|
| |
|