Commit graph

550 commits

Author SHA1 Message Date
Erik Boasson
c8d8d2f8e6 Stop threads doing handshake processing earlier
In particular before the state they depend on gets torn down.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-30 10:32:38 +02:00
Marcel Jordense
fa0c6777d4 Remove setting volatile secure writer to incorrect state
Signed-off-by: Marcel Jordense <marcel.jordense@adlinktech.com>
2020-03-27 16:31:08 +01:00
Marcel Jordense
9175f44273 Send crypto tokens after handshake is completely finished
Signed-off-by: Marcel Jordense <marcel.jordense@adlinktech.com>
2020-03-27 16:31:08 +01:00
Marcel Jordense
a77fe10a04 Add index on receiver specific key to improve verification of origin authentication signing
Signed-off-by: Marcel Jordense <marcel.jordense@adlinktech.com>
2020-03-27 16:31:08 +01:00
Erik Boasson
d82b7fdd73 Return PRECONDITION_NOT_MET if security not supported
As opposed to NOT_ALLOWED_BY_SECURITY.  There is a meaningful
difference between something being disallowed and something being
impossible.

Co-Authored-By: Kyle Fazzari <github@status.e4ward.com>
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-26 08:46:26 +01:00
Erik Boasson
ecbd585f12 Generate header with compile-time features
Currently:

* DDS_HAS_SECURITY for DDS Security support
* DDS_HAS_LIFESPAN for lifespan QoS support
* DDS_HAS_DEADLINE_MISSED for "deadline missed" event support

These are defined to 1 if support for the feature is included in the
build and left undefined if it isn't.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-26 08:46:26 +01:00
Erik Boasson
d4e9300dad Do not silently ignore security QoS settings
When built without support for DDS Security, any attempt to create a
participant QoS settings in the security name space (those prefixed by
"dds.sec.") must fail.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-26 08:46:26 +01:00
Erik Boasson
4fe9cf290d Add DDS_HAS_PROPERTY_LIST_QOS feature test macro
If set, dds_q{set,get}_{prop,bprop} are available.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-26 08:46:26 +01:00
Erik Boasson
67c49235db Merge remote-tracking branch 'upstream/master' into master-to-security 2020-03-19 08:18:48 +01:00
Erik Boasson
e1201e678d Minor cleanup of UDP, TCP support code
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-18 17:31:20 +01:00
Erik Boasson
4df38f5bf9 Move all socket creation stuff to transport code
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-18 17:31:20 +01:00
Erik Boasson
59459b9b8b Change PrismTech references to Adlink
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-18 17:31:20 +01:00
Erik Boasson
89001a0f6a Remove unused PrismTech/Adlink-specials
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-18 17:31:20 +01:00
Erik Boasson
77c3545f5e Move all time support to ddsrt
* Move wctime, mtime, etime types to ddsrt

* Add ddsrt_time_wallclock

* Change ddsrt_time_monontic, elapsed to use mtime, etime types

* Remove now, now_mt, now_et

* Rename X_to_sec_usec to ddsrt_X_to_sec_usec

* add_duration_to_X to ddsrt_X_add_duration (to be in line with the
  existing ddsrt_time_add_duration)

* elimination of ddsrt/timeconv.h, it added more in the way of
  complications than it did in making things more elegant

* rename of q_time.[ch] to ddsi_time.[ch]: that now only deals with DDSI
  timestamps and durations on the wire

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-18 17:31:20 +01:00
Erik Boasson
1611adc20a Replace T_SECOND etc. by DDS_ equivalents
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-18 17:31:20 +01:00
Erik Boasson
763ed67958 Replace T_NEVER by DDS_NEVER, DDS_INFINITY
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-18 17:31:20 +01:00
Erik Boasson
0354b42cdc Check for permissions handle by testing for != 0
The test gates access-control plugin invocation and with the inverted
condition all remote readers/writers requiring access control are
blocked eiter because of the permissions handle, or because a NIL handle
is passed to the access control plugin.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-16 09:24:20 +01:00
Erik Boasson
4a6b134126 Fix passing of "relay_only" in check/register reader
* access-control check_remote_datareader has "relay_only" as an out
  parameter, so should pass in an address instead of "false";

* value of "relay_only" returned by check_remote_datareader must be
  passed to crypto register_matched_remote_datareader

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-16 09:24:20 +01:00
Marcel Jordense
f11dd50810 Set volatile secure reader initially out-of-sync
Signed-off-by: Marcel Jordense <marcel.jordense@adlinktech.com>
2020-03-13 19:24:03 +01:00
Erik Boasson
d1ed8df9f3 Create a separate socket for transmitting data
This is a workaround for interoperability issues, ultimately driven by a
Windows quirk that makes multicast delivery within a machine utterly
unreliable if the transmitting socket is bound to 0.0.0.0 (despite all
sockets having multicast interfaces set correctly) when there are also
sockets transmitting to the same multicast group that have been bound to
non-0.0.0.0.  (Note: there may be other factors at play, but this is
what it looks like after experimentation.)

At least Fast-RTPS in some versions binds the socket it uses for
transmitting multicasts to non-0.0.0.0, so interoperability with
Fast-RTPS on Windows requires us to bind the socket we use for
transmitting multicasts (which was the same as the one we use for
receiving unicast data) also to non-0.0.0.0 or our multicasts get
dropped often.

This would work fine if other implementations honoured the set of
advertised addresses.  However, at least Fast-RTPS and Connext (in some
versions) fail to do this and happily substitute 127.0.0.1 for the
advertised IP address.  If we bind to, e.g., 192.168.1.1, then suddenly
those packets won't arrive anymore, breaking interoperability.

The only work around is to use a separate socket for sending.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-09 20:49:47 +01:00
Erik Boasson
e58f4dc344 Fix macro for checking serdata has get_keyhash
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-05 16:25:36 +01:00
Erik Boasson
ea91e17a62 Rename nn_keyhash to ddsi_keyhash
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-05 16:25:36 +01:00
Erik Boasson
0845337f47 Test interface dropping incoming/outgoing packets
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-05 16:10:46 +01:00
Dennis Potman
0a9d391c64 Fixed logging in find_own_ip
Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
2020-03-05 15:25:16 +01:00
Marcel Jordense
8ca1780538 Correct problem with cleanup of security handshake
Signed-off-by: Marcel Jordense <marcel.jordense@adlinktech.com>
2020-03-05 15:25:16 +01:00
Dennis Potman
1cf472ea06 Refactored tests for security core: introduced generic wrappers for security plugins and
updated plugin loading tests to use these instead of specific wrappers per test. Added
test for securing communication ad handshake fail (using different identity CAs)

Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
2020-03-05 15:25:16 +01:00
Dennis Potman
323de40cba Moved add_definitions in cmake files to /src/ so that all components and tests use the same definitions when compiling sources
Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
2020-03-05 15:25:16 +01:00
Dennis Potman
b3c5a8d4fc Refactored expand_vars so that a custom lookup function can be used, which makes it usable for replacing tags by values other than environment variables
Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
2020-03-05 15:25:16 +01:00
eboasson
e412f6fab2
Fix issue in dds_create_topic_arbitrary (#422)
* Fix issue in dds_create_topic_arbitrary

Changed the behaviour of dds_create_topic_arbitrary with respect to the
sertopic parameter: the existing function dds_create_topic_arbitrary is
marked deprecated and replaced by dds_create_topic_generic, which returns
the sertopic that is actually used in as an out parameter. This can be eiter
the provided sertopic (if this sertopic was not yet known in the domain) or an
existing sertopic if the sertopic was registered earlier.

Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>

* Fix memory leaks in case topic creation fails.

Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
2020-03-05 15:21:02 +01:00
Marcel Jordense
1dad535bc0 Correct misplaced lock
Signed-off-by: Marcel Jordense <marcel.jordense@adlinktech.com>
2020-03-03 13:32:38 +01:00
Marcel Jordense
4960fbf94c Add security authentication handshake and encoding and decoding
Signed-off-by: Marcel Jordense <marcel.jordense@adlinktech.com>
2020-03-03 13:32:38 +01:00
Dan Rose
ca4b5a368f Fix undefined behavior when hash function given null pointer
[test_subscriber-12] /opt/ros/master/src/eclipse-cyclonedds/cyclonedds/src/ddsrt/src/mh3.c:28:53: runtime error: applying zero offset to null pointer
[test_subscriber-12] SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /opt/ros/master/src/eclipse-cyclonedds/cyclonedds/src/ddsrt/src/mh3.c:28:53 in
Signed-off-by: Dan Rose <dan@digilabs.io>
2020-02-29 08:48:46 +01:00
Dan Rose
d72ebb0ed3
Don't pass null to memcmp (#413)
* Don't pass null to memcmp

```
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /opt/ros/master/src/ros2/rmw_cyclonedds/rmw_cyclonedds_cpp/include/rmw_cyclonedds_cpp/serdes.hpp:135:3 in
/opt/ros/master/src/eclipse-cyclonedds/cyclonedds/src/core/ddsi/src/ddsi_sertopic_default.c:41:15: runtime error: null pointer passed as argument 1, which is declared to never be null
/usr/include/string.h:64:33: note: nonnull attribute specified here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /opt/ros/master/src/eclipse-cyclonedds/cyclonedds/src/core/ddsi/src/ddsi_sertopic_default.c:41:15 in
/opt/ros/master/src/eclipse-cyclonedds/cyclonedds/src/core/ddsi/src/ddsi_sertopic_default.c:41:31: runtime error: null pointer passed as argument 2, which is declared to never be null
/usr/include/string.h:64:33: note: nonnull attribute specified here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /opt/ros/master/src/eclipse-cyclonedds/cyclonedds/src/core/ddsi/src/ddsi_sertopic_default.c:41:31 in
/opt/ros/master/src/eclipse-cyclonedds/cyclonedds/src/core/ddsi/src/ddsi_sertopic_default.c:45:15: runtime error: null pointer passed as argument 1, which is declared to never be null
/usr/include/string.h:64:33: note: nonnull attribute specified here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /opt/ros/master/src/eclipse-cyclonedds/cyclonedds/src/core/ddsi/src/ddsi_sertopic_default.c:45:15 in
/opt/ros/master/src/eclipse-cyclonedds/cyclonedds/src/core/ddsi/src/ddsi_sertopic_default.c:45:30: runtime error: null pointer passed as argument 2, which is declared to never be null
/usr/include/string.h:64:33: note: nonnull attribute specified here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /opt/ros/master/src/eclipse-cyclonedds/cyclonedds/src/core/ddsi/src/ddsi_sertopic_default.c:45:30 in
```
Signed-off-by: Dan Rose <dan@digilabs.io>

* clearer non-null check

Signed-off-by: Dan Rose <dan@digilabs.io>
2020-02-29 08:32:02 +01:00
Erik Boasson
1ee2dfe08f Avoid race causing thread-state pointer aliasing
The thread_states array resets the "state" to ZERO on thread termination
to indicate that the slot was unused, but it leaves the thread id
unchanged because some platforms don't have a defined value that will
never be used as a thread id.  A consequence is that a newly created
thread may result in multiple slots containing their own thread id, but
generally there will only be one that is not in state ZERO.

However, the code for create_thread used to set the state to ALIVE prior
to creating the thread, and so if the events get scheduled like:

1. thread A: X.state = ALIVE
2. create new thread B, storing tid in X.tid
3. thread A: Y.state = ALIVE
4. new thread B: lookup self (and cache pointer)
5. create new thread C, storing tid in Y.tid
6. new thread C: lookup self (and cache pointer)

Then B will observe two slots in the ALIVE state, with X.tid certain to
match and Y.tid undefined (and hence possibly matching).  It may
therefore pick Y.  C will (in this schedule) of course always choose Y.
They cache the pointer and never look at X and Y again, except for
updating their virtual clocks.

These virtual clocks are updated non-atomically (by design it is private
to the thread) and so if both B & C use Y they can end up racing each
other in updating the virtual clock and cause the nesting level of the
"awake" state controlling garbage collection to get stuck (or wrap
around, or do other horrible things).  The consequence can be anything,
from a somewhat benign variant where GC effectively stops and some
operations (deleting readers and writers and shutting down) block
forever, to use-after-free and the undefined behaviour that implies.

This commit avoids looking up the slot in the newly created threads,
instead passing the correct address in the argument.  It also adds an
intermediate state INIT that serves to reserve the slot until the new
thread is actually running.  It does make the look-up safe (if one were
to do it), and as it is essentially free and gives more insight in the
state of the system when viewed from a debugger, it appears a useful
addition.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-02-28 08:13:31 +01:00
Erik Boasson
6dc28db197 Fix warning by cleaning up dds_set_qos_locked_raw
gcc 5.4 correctly warned that a null pointer was being passed into the
entity-specific "set_qos" function when changing a topic QoS, where that
parameter was tagged as "non-null".  As it was never dereferenced in
this case the resulting behaviour was still correct.

It turns out that the entire function was overly complicated and that
simply passing the entity pointer round allows eliminating a few
arguments as well.

(Oddly none of the more modern toolchains used pick this up.)

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-02-22 14:30:46 +01:00
Dennis Potman
8bd6f34f67 Renamed unsupp config sections to internal
Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
2020-02-20 16:47:06 +01:00
Dennis Potman
754eb4090e Fixed issue that Cyclone does not receive multicast data on Windows when
the destination cache of the network stack is in a certain state. The issue
is resolved by binding unicast sockets (incoming unicast and all outgoing
traffic) to the address of the interface instead of inaddr_any (0.0.0.0).
Set the new configuration option internal/BindUnicastToInterfaceAddr to
false to get the old behavior.

Co-authored-by: Erik Boasson <eb@ilities.com>

Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
2020-02-20 16:47:06 +01:00
ChenYing Kuo
af3604dea7
Fix some typos. (#399)
* Fix some typos.

Signed-off-by: ChenYing Kuo <evshary@gmail.com>

* Also update q_config.c, cyclonedds.rnc, cyclonedds.xsd for correct
build.

Signed-off-by: ChenYing Kuo <evshary@gmail.com>

* Remove cdds.md.

Signed-off-by: ChenYing Kuo <evshary@gmail.com>
2020-02-19 12:33:39 +01:00
Erik Boasson
ef047d6bd5 Check all dds_write calls in liveliness tests
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-02-18 13:55:44 +01:00
Erik Boasson
54fad0d601 Merge remote-tracking branch 'upstream/master' into security 2020-02-13 13:13:54 +01:00
Erik Boasson
4ed0128578 Rework security code in proxy participant creation
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-02-13 13:12:25 +01:00
Erik Boasson
62a6004e8a Remove "CM" topic related flags reintroduced by merge
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-02-13 12:00:13 +01:00
Erik Boasson
9b43303d82 Remove unused "kernel sequence numbers" notion
The flag originates in OpenSplice but is meaningless in Cyclone.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-02-13 12:00:07 +01:00
Erik Boasson
ad58db0721 Merge branch 'master' into security
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-02-12 17:30:38 +01:00
Erik Boasson
701c6f5a5c Retain less data in keep-last WHC in absence of ACKs
A keep-last volatile WHC retained data already overwritten by the writer
in the absence of ACKs, introduced by 231cb8c9.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-02-12 17:19:48 +01:00
Erik Boasson
6ed190ce2a Fix leak in converting hostname to IP address
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-02-12 17:19:48 +01:00
Erik Boasson
01f9c0599c Set transport in locator of TCP server sockets
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-02-12 17:19:48 +01:00
Erik Boasson
ab7c95e02f Clean up sertopic_default definition
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-02-11 23:26:01 +01:00
Erik Boasson
ad19f571ae Rename nn_plist, xqos to ddsi_plist, xqos
This already was leaking out in the interface, so this name change was
needed too.  The relationship between plist and xqos being so intimate,
doing the one but not the other made no sense.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-02-11 23:26:01 +01:00
Erik Boasson
551dae69a4 Rename q_globals to ddsi_domaingv
The name (not its definition) now leaks out in ddsi_sertopic, and the
messy old names really shouldn't pollute the interface any more than
necessary.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-02-11 23:26:01 +01:00