CID 304509 - it does not affect behaviour because the called function
uses it as an out parameter and the result is never inspected.
Signed-off-by: Erik Boasson <eb@ilities.com>
Failure to generate a signature for in handshake tests attempted to free
the address of the pointer, instead of the pointed-to memory (CID
304462).
Signed-off-by: Erik Boasson <eb@ilities.com>
Triggered by CID 304462, 304471, 304517: dereference before null check.
Note that it is a second-order problem because it would require the
plug-in functions to be called with a null pointer for the plug-in
instance.
Signed-off-by: Erik Boasson <eb@ilities.com>
The changes in d92d491b83 to deal with
local readers and writers with the same topic and type name but
different underlying `struct ddsi_sertopic`s did not include the
provisioning of historical data from a (local) transient-local writer to
a (local) transient-local reader.
Signed-off-by: Erik Boasson <eb@ilities.com>
All incoming samples end up in ddsi_plist_fini, usually one with nothing
present, sometimes one containing status info or a keyhash. The
"present" flags allow this to be a very quick operation in these simple
cases, and this should be made use of.
Signed-off-by: Erik Boasson <eb@ilities.com>
The macOS 10.12 build was put in because of ROS2 "Dashing" specified
10.12 as the supported version, but Eloquent and later specify
10.14. The relevance of this is no longer there because of Foxy. The
build itself took an inordinate amount of time with lots of warnings
about the platform being deprecated.
Signed-off-by: Erik Boasson <eb@ilities.com>
Fix using the variable relay_only uninitialized in the function
connect_proxy_writer_with_reader when security is disabled in the
build configuration.
Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
This addresses a number of issues with building Cyclone DDS including
DDS Security while using OpenSSL 1.0.2. Compatibility with 1.0.2 is a
courtesy towards those who are unable to move to 1.1.x or later because
of other libraries.
* On Windows, one must include Winsock2.h prior to including the OpenSSL
header files, or it'll pull in incompatible definitions from Winsock.h
and that breaks some of the files.
* OpenSSL 1.0.2 requires initializing the library (or more particular,
loading all the required algorithms) but this is no longer needed in
OpenSSL 1.1.x. It ends up being needed in a few places and having tons
of essentially dead initialization code lying around is unpleasant.
Hence this has been consolidated in a single function and protected
with ddsrt_once().
* One ought to undo the above initialization on 1.0.2g and older, but it
is impossible to know whether that can safely be done from a library.
This is also the reason OpenSSL deprecated all the initialization and
cleanup interfaces. So if one insists on trying it with such an old
version, let there be some leaks.
* Thread state cleanup is sort-of required prior 1.1.0, but that suffers
from the same problems; we'd have to do per-thread cleanup code for
OpenSSL for any thread that could call into it (which is pretty much
any thread). So once again, people should just use 1.1.0 or newer.
* There are some interfaces added in 1.1.0 that we use, but a few small
workarounds those can be made to work on 1.0.2 as well. These also
were replicated in a number of places and consolidated by this commit.
Signed-off-by: Erik Boasson <eb@ilities.com>
The various network partition-related structs were left mostly
uninitialized by the various init functions (e.g., if_partition_mapping)
and these were moreover interpreted on parse errors. This initializes
them to null pointers and skips all configuration post-processing in
case of an error.
Signed-off-by: Erik Boasson <eb@ilities.com>
This changes the handling of the removal of the lease of a manual
liveliness (proxy) writer from the (proxy) participant, such that the
invariant maintained for the "min lease" objects in the (proxy)
participant changes from: a clone of some lease with the minimum
duration, to: a clone of the lease that is returned by the
ddsrt_fibheap_min operation on the lease heap.
This fixes a use-after-free of the entity pointed to by the cloned lease
object in a scenario where the shortest lease duration is used by
multiple writers and the removal of a lease from the heap shuffles the
remaining entries around. For example (before this change):
1. initial situation: three writers w1, w2 and w3 with equal lease
durations:
- pp.heap = w1.lease : w2.lease w3.lease
- pp.minl = clone of w1.lease
2. delete w2:
- assuming deleting w2.lease from the heap moves w3.lease to the
front (only guarantee is that there are no smaller keys in the heap
than that of the entry returned by minimum operation)
- min(pp.heap) = w1.lease != w2.lease
thus: pp.minl unchanged, pp.minl.entity = w1
- pp.heap = w3.lease : w1.lease
3. delete w1:
- min(pp.heap) = w3.lease != w1.lease,
thus: pp.minl unchanged, pp.minl.entity = w1
- pp.heap = w3
- free w1
- now pp.minl.entity has a dangling pointer, touched on deleting
the (proxy) particpant or on lease expiry.
With this chamge, pp.minl is updated in step 2 to be a clone of w3.lease
because the lease returned by min(pp.heap) changes. This ensures that
in step 3 there is no dangling pointer and no use-after-free.
Signed-off-by: Erik Boasson <eb@ilities.com>
* Increase the matching timeout to 5s (there are some hints the failures
on Travis are timing related)
* Replace the relative timeout in the waitset by a timestamp so that it
gives up after the specified timeout regardless of the number of
events that occur
Signed-off-by: Erik Boasson <eb@ilities.com>
* Compute the time at which the handshake must have completed from the
initial timeout specification, rather than using it as a timeout for
the individual steps of the handshake
* If the handshake fails because an expected message is not present,
print this, including whether the timeout occured because the message
queue was empty or because the expected message could not be found in
a non-empty queue.
* Replace the 0.5s per-step timeout to a single 2s timeout.
Signed-off-by: Erik Boasson <eb@ilities.com>
When defining a new topic, typically the serializer instructions that
are usually in constant memory and generated by the IDL compiler are
copied into memory managed by the Cyclone implementation. For this it
needs to compute the size of the serializer, which the IDL compiler
doesn't provide. It does this by effectively dry-running the
program. (Note that it doesn't validate the program.)
All but the JSR operations move the program counter forward, but the JSR
operation can cause it to go backward instead and allows implementing
recursive types (the IDL compiler doesn't support them, but one might
decide to work around that limitation). When dry-running the program,
following a backwards jump can cause a non-terminating loop.
The jump could potentially be to an unexplored address and so ignoring
all backwards jumps potentially means it skips part of the program. As
this is not a validator and the program can always be arranged so that a
following a backwards jump is not relevant to computing the size
correctly, this is reasonable approximation.
Signed-off-by: Erik Boasson <eb@ilities.com>
The dds_wait_for_acks function follows the DCPS specification and allows
waiting for all matching readers to have acknowledged all data written
prior to that point. This commit leaves the API unchanged but extends
the implementation to make it possible to wait until a specific reader
has acknowledged everything, as this is a useful device in testing with
deliberate one-way disconnections using dds_domain_set_deafmute.
Signed-off-by: Erik Boasson <eb@ilities.com>
In particular, this means instances published by a transient-local
writer will go back to ALIVE following a disconnect and reconnect.
Signed-off-by: Erik Boasson <eb@ilities.com>
This leaves the argument pointer in the destination unchanged, rather
than resetting it to an irrelevant value.
Signed-off-by: Erik Boasson <eb@ilities.com>
The memory allocation in deserializing property lists within the crypto
code should not trust the deserialized length and try to allocate that
much memory but should first verify that the length is consistent with
the number of bytes remaining in the input. (Noted by Coverity as use
of tainted data.)
Signed-off-by: Erik Boasson <eb@ilities.com>
Sending a heartbeat to all matched readers for the P2P builtin
participant volatile secure writer unlocks the writer before pushing
each individual message out, and so determining the time of the next
heartbeat event before writing and updating it afterwards means the
state may have changed. While this is appears benign, it is better to
do the update atomically.
Signed-off-by: Erik Boasson <eb@ilities.com>
The security plugins currently use the standardized representations of
octet sequences, unlike the DDSI stack's internal representation. A
shallow copy is therefore not simply a memcpy.
Signed-off-by: Erik Boasson <eb@ilities.com>
Coverity has difficulty observering that dds_entity_pin /
ddsrt_mutex_lock / dds_entity_unlock is correct. It is perhaps a bit
confusing, so change it.
Signed-off-by: Erik Boasson <eb@ilities.com>
A few failures to signal DATA_AVAILABLE (as well as some where it was
signalled unnecessarily) were discovered while refactoring the RHC
despite the tests all passing. Clearly the tests were inadequate.
The enormous amount of boilerplate in the tests prompted a small rewrite
to a programmable listener invocation tester that one simply feeds a
noise-like one-liner in a string. This trades the boilerplate for
somewhat inscrutable code.
Signed-off-by: Erik Boasson <eb@ilities.com>
One cannot create writers for built-in topics, therefore one generally
does not create samples for them. However, one can lookup an instance
handle from a sample with just a key value in it, and so the function is
needed.
Signed-off-by: Erik Boasson <eb@ilities.com>
The standard defines GUIDs as an array of 16 uint8_t and so they are
presented on the network and in built-in topic samples. Internally they
are arrays of 4 uint32_t, requiring byte-order conversion.
A keyhash is also an array of 16 uint8_t, and used almost exclusively on
the network. The exception is the generation of built-in topic samples,
which relies on the "from_keyhash" function. One would expect the
keyhash here to contain the GUID in the external representation, and
this commit adds the byte-order conversions to conform to the
expectation.
Signed-off-by: Erik Boasson <eb@ilities.com>
Disposing an instance would only add an invalid sample if the instance
is empty, but it should also do so when the latest sample is read.
Otherwise reading all NOT_READ samples gives nothing or nonsensical
output.
Signed-off-by: Erik Boasson <eb@ilities.com>
