Commit graph

890 commits

Author SHA1 Message Date
Marcel Jordense
cb1d06b442 Freeing the writer security attributes should be done by the gc
Signed-off-by: Marcel Jordense <marcel.jordense@adlinktech.com>
2020-04-07 11:39:57 +02:00
Marcel Jordense
b6640d86b0 Correct length of encrypted submessage body
Signed-off-by: Marcel Jordense <marcel.jordense@adlinktech.com>
2020-04-07 11:39:57 +02:00
Marcel Jordense
f792b3ceed Store security info of the proxy endpoints in the common part
Signed-off-by: Marcel Jordense <marcel.jordense@adlinktech.com>
2020-04-07 11:39:57 +02:00
Erik Boasson
99df0956e7 Crypto endpoint relation compare routines cleanup
* Trying not to assume an int is at least 32 bits.

* Technically speaking, comparing "unrelated" addresses is undefined
  behaviour which can be avoided by a cast to uintptr_t.

* The early out if either local_crypto == 0 does work in context,
  provided the nodes in tree never have local_crypto == 0.  That implies
  crypto_insert_endpoint_relation must never have a 0 in there, which I
  think the callers do respect.  Still I think it is better to not hide
  these assumptions in the compare function and address the problem in
  the lookup function instead.

These changes likely make the code fractionally slower, but I do think
they improve clarity.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-04-01 09:39:11 +02:00
eboasson
00710a4e5d
Merge pull request #458 from eboasson/sec-plus-master
Merge master into security
2020-03-30 12:44:06 +02:00
Erik Boasson
5b1f288d6c Merge remote-tracking branch 'upstream/master' into sec-plus-master
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-30 12:09:03 +02:00
Erik Boasson
4f3cbf7a1c Clean up representation of discovery messages
* Remove the "plist" and "rawcdr" abuse of the "serdata_default" sample
  representation.

* Introduce a new "plist" topic type and a new "pserop" topic type.  The
  former represents parameter lists as used in discovery, the second
  arbitrary samples using the serialiser in ddsi_plist.c.

* Introduce sertopics for each of the built-in "topics" used by the DDSI
  discovery protocol using the two new topic types, and reference these
  in the readers/writers used in discovery.

* Construct and deconstruct the discovery message by using the
  conversion routines for these sample types, rather than fiddling with,
  e.g., the baroque interface for adding parameter lists to messages.

* As a consequence, it introduces standardized logging of received and
  transmitted discovery data and eliminates the annoying "(null)/(null)"
  and "(blob)" descriptions in the trace.

* Limits the dumping of octet sequences in discovery data to the first
  100 bytes to make the embedded certificates and permissions
  documents (somewhat) manageable.

* Eliminates the (many) null pointer checks on reader/writer topics.

* Fixes the printing of nested sequences in discovery data (not used
  before) and the formatting of GUIDs.

Various interfaces remain unchanged and so while this removes cruft from
the core code, it moves some of it into the conversion routines for the
new topic types.

It also now allocates some memory when processing incoming discovery
data, whereas before it had no need to do so.  Allowing for aliasing of
data in the new sertopics and adding a way to initialize these specific
types on the stack (both minor changes) suffices for eliminating those
allocations.

Signed-off-by: Erik Boasson <eb@ilities.com>

Check actual topic type before "downcasting"

Signed-off-by: Erik Boasson <eb@ilities.com>

Free the memory we own and is actually allocated

Signed-off-by: Erik Boasson <eb@ilities.com>

Ignore logging newlines if nothing is buffered

Signed-off-by: Erik Boasson <eb@ilities.com>

Suffix data with "(trunc)" one byte earlier

The sample printing code changed over time and now stops as soon as it
can once it has filled up the buffer.  As the return value is simply the
number of bytes written, if that number is equal to buffer size less
one (because of the terminating nul) it may or may not have been
truncated, but the likelihood is that it has been.  So add the "(trunc)"
suffix once that point has been reached.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-30 11:13:55 +02:00
eboasson
b18fd395d3
Do not rewrite secure messages in retransmit queue (#456)
* Do not rewrite secure messages in retransmit queue

Messages to be retransmitted spend some time on a transmit queue, and
are subject to the rewriting of the destination information to reduce
the number of outgoing copies.  Self-evidently, altering the message
header does not sit well with encryption and/or authentication of
messages.

The way the rewriting works is that the offset of the "reader entity id"
in the DATA submessage is saved on message construction (the GUID prefix
is at a fixed location), so that it can be read and possibly zero'd out
later.  The crypto transformations move the message around and it so
happens that it can end up pointing to the key id in the encoded
message.  Zeroing that one out leads to uninterpretable messages.

This commit adds a message/event kind to distinguish between retransmit
that may and retransmit that may not be merged (and thus rewritten) and
gets used when the crypto plugin is invoked to transform a message.

Signed-off-by: Erik Boasson <eb@ilities.com>

* Update comment on changing REXMIT to REXMIT_NOMERGE

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-30 10:32:58 +02:00
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
6413d71599 Workaround for false positive from clang-tidy (#452)
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-27 15:28:00 +01:00
Erik Boasson
1c31fba043 Fix race in deleting lingering writers
Deleting a writer with unacknowledged data present in its WHC causes it
to linger for a configurable duration.  Once it is lingering, there are
two routes to actually deleting the writer: because the samples get
acknowledged, or because the linger duration elapses.

When these two happen roughly concurrently, there was a possibility of
both succeeding in looking up the writer by its GUID, in which case one
of them then asserts on removing it from the entity index (if assertions
are enabled, if not, things are worse).

This fixes that by ensuring only one of the two actually does something,
as was always the intent.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-27 15:28:00 +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
5d53e74029 Fix read/take/return_loan edge cases
* read/take failed to restore the null pointer in the first entry of the
  sample pointer array it gets passed, in the case no "loan" had been
  allocated yet and it returned an empty set.  The consequence is that
  on a subsequence read it will reuse the address without marking at as
  in use, so that a *second* read using with a null pointer in that
  first entry will overwrite the first result.  (Introduced by
  d16264fd82.)

* return_loan failed to free all memory if its argument wasn't actually
  a loan.  There are many good arguments why the read/take/return_loan
  interface is messed up, but in the context of the existing interface
  this is a perfectly reasonable case: there is at most one "loan" for
  each reader, but one can keep calling read/take and return_loan as if
  there's an infinite number of "loans".  It's just that the first gets
  cached and the others don't.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-25 14:53:30 +01:00
Erik Boasson
d089ce946c Dedup function to create unique topic names in tests
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-25 14:53:30 +01:00
Dennis Potman
d03587fcea Add identity bob to default test permissions xml
Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
2020-03-25 10:46:17 +01:00
Dennis Potman
bf23dee70a Fix in access control identity subject compare
Fixed a bug in the subject compare function for identity subjects, that
could cause using the incorrect permission grant in case multiple grants
are provided in the permissions configuration of the access control plugin.

Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
2020-03-25 10:46:17 +01:00
Erik Boasson
cec8aea6c9 Compound literals go out of scope, too
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-23 14:15:38 +01:00
Erik Boasson
63f67ae965 Fix IPv6 multicast breakage
The moving around and cleaning up of network code broke the IPv6
multicast support by memcpy'ing a sockaddr_in6 instead of an in6_addr in
a multicast join record.

Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-20 14:25:32 +01:00
Dennis Potman
0768ad59ed
Remove duplicated code in authentication plugin (#442)
* Remove duplicated code in authentication plugin

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

* Fix build warnings

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

* Fix memory leak and call create_validate_asymmetrical_signature directly from create_validate_signature_impl

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

* Fix refcount issue (assert in openssl) for identity cert in hs remote info

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

* Refactoring of validate_handshake_token function

Co-authored-by: Erik Boasson <eb@ilities.com>
Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
2020-03-20 13:44:27 +01:00
Thijs Sassen
79c086868f Fixed compile errors for targets that do not support IPV6
Signed-off-by: Thijs Sassen <thijs.sassen@adlinktech.com>
2020-03-19 18:47:14 +01:00
eboasson
ab5f51eada
Merge pull request #424 from eboasson/master-to-security
Update security branch to current master
2020-03-19 17:50:01 +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
f139dbcd5e MS C++ is troubled by C99 compound literals
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-18 17:31:20 +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
0b9ab17018 Do not set DCPSParticipant listener too early
The participant listener creates a pong writer, setting a publication
matched listener on it.  That listener can be invoked immediately and as
it queries the subscriptions reader, it must not be enabled before the
latter reader has been created.

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
39c7997c67 Remove unused dds_sleepuntil
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-18 17:31:20 +01:00
Dennis Potman
3ea2cea318
Code formatting fixes and clean-up authentication plugin (#439)
* Fix code formatting, fix for memory leak in validate_handshake_reply_token and
make error handling and return values more consistent with the other two
plugins.

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

* Processed review comments: fixed memory leaks and more consistent error handling and function returns

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

* Fix trusted ca dir max exceeded

Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
2020-03-18 10:01: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
6507859f36 Correct handling of identity certificates with EC key
Signed-off-by: Marcel Jordense <marcel.jordense@adlinktech.com>
2020-03-13 19:24:03 +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
Thijs Sassen
269f18e98a Updated version for ros2 package
Signed-off-by: Thijs Sassen <thijs.sassen@adlinktech.com>
2020-03-12 09:37:02 +01:00
Sid Faber
9fe51ef3fb Install security plugins
Security plugins are built but not installed.  Add target to CMakeLists.txt
for three security plugins.

Signed-off-by: Sid Faber <sid.faber@canonical.com>
2020-03-10 15:19:50 +01:00
Dan Rose
2c16dfa23e Don't link winsock1
wsock32.lib is only needed for the legacy version of Winsock and is not needed with Winsock2 (the current version).
This appears to be a root cause of the multicast issue on Win10 and may allow us to reverse #404

Signed-off-by: Dan Rose <dan@digilabs.io>
2020-03-10 10:07:58 +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
9e673769ce Add "deaf/mute" to pubsub
Signed-off-by: Erik Boasson <eb@ilities.com>
2020-03-05 16:10:46 +01:00