Use of CryptoTransformKeyRevisionIntHolder alias in operation parameters

The DDS Security Specification 1.2 defines the CryptoTransformKeyRevisionIntHolder type as:

typedef int32 CryptoTransformKeyRevisionIntHolder;

The SPIs should use this type consistently where appropriate.
For example, activate_key_revision SPI uses CryptoTransformKeyRevisionIntHolder to type its key_revision parameter. However, the key_revision parameter of create_local_datawriter_crypto_tokens, create_local_datareader_crypto_tokens and create_local_datareader_crypto_tokens is an int32.

This should be changed so all these operations use CryptoTransformKeyRevisionIntHolder

Clarify section “9.8.10.2 Key Exchange with remote DataReader

The description in page 182 should clarify that that the participant can call create_local_datawriter_crypto_tokens multiple times (to get a crypto token sequence for different revisions).

The description of is_write_protected starts with "Indicates if read access". This should be "Indicates if write access."

The normative IDL doesn't have SecureSubmessageCategory_t, but it does have SecureSumessageCategory_t which appears to be a typo.

DDS Security 1.2 added two elements to the Governance XSD: enable_key_revision and rtps_psk_protection_kind.
These elements were added without specifying a value for minOccurs which according the XSD rules it would default to 1. The result of this is that existing governance documents would not be valid according to the XSD. This was not the intent

Instead the elements should have been added specifying minOccurs="0".

This change impacts the machine-readable file omg_shared_ca_governance.xsd

There is an inherent DoS network amplification attack that exploits peer-to-peer discovery. See https://issues.omg.org/browse/DDSIRTP26-6

This issue should be addressed by DDS-Security. Likely using some pre-shared key mechanics to protect all messages not otherwise protected. For example, the authentication handshakes.

Provide a layer of PSK protection

Peovide the means to use a pre-shared secret to protect any RTPS message (e.g. bootstrap messges) that is not otherwise protected by the keys that the DomainParticipants exchange.

Also define the "pre-shared" key mechanism as a separate "builtin" plugin

In section 10 and its subsections there are two properties that are named incorreclty in that a "." has been used instead of a "_". Specifically there are places in the section that use the names
"rtps_psk.secret_passphrase" and "rtps_psk.symmetric_cipher_algorithm"

These should be replaced with:
"rtps_psk_secret_passphrase" and
"rtps_psk_symmetric_cipher_algorithm"

Which are the names that appear in the "Configuration Properties" tables in the "Property Name" column.

Section 8.3.2.11.11 has no description of the permissions_token parameter

(This may be a moot point depending on the resolution of DDSSEC12-13.)

The row of Table 52 that describes validate_local_identity states that the QoS it receives must contain the properties defined in section 9.3.1. These properties are the ones starting with dds.sec.auth.

The problem is that for begin_handshake_* (either variant) to work, validate_local_identity must also receive the property dds.sec.access.permissions. This should be noted in the requirements for validate_local_identity: it must store the QoS, or at least this property, in a location that can be looked up by IdentityHandle so that handshake tokens can be created. Alternatively, the begin_handshake_* operations could be extended to receive this info separately.

Another connection between the two plugins is made by the PermissionsCredentialToken. If the intent is to use this data when generating handshake handles, that should be noted in tables 49-50 (c.perm) instead of referencing QoS policies there.

Section 8.8.9 specifies that when the crypto tokens are sent on the network,
BuiltinParticipantVolatileMessageSecureWriter is used so they are not sent in the clear. This detail is also described in 8.5.1.8.1 and other parts of 8.5.1.8.

Section 8.5.1.8.1 (2nd pgh) contains
...intended for transmission in "clear text" to the remote...
which contradicts 8.8.9.

To resolve this issue, remove a few unnecessary parts of 8.5.1.8:

• 8.5.1.8.1 2nd sentence of 2nd pgh "The returned CryptoToken sequence is intended..."
• 8.5.1.8.2 latter half of 2nd pgh starting "The CryptoToken sequence may contain..."
• 8.5.1.8.4 (similar content to 8.5.1.8.2)
• 8.5.1.8.6 (similar content to 8.5.1.8.2)

RTPS 2.4 added support for GROUP ordered access and coherent access. Part of this work added fields to the heartbeat submessage, one of which is a secureWriterSet.

This field shall contain the 32-bit xxHash32 of all of the writer EntityIds that are sent via secure discovery.

The use of this field should be mentioned somewhere in the security spec as the RTPS spec refers to the security specification to define the use of this field.

A new inline QoS was also added: PID_SECURE_WRITER_GROUP_INFO. This inline QoS should also be added to the security spec.

See the revised RTPS text here: DDSIRTP23-117

Table 63's entry for validate_remote_permissions describes a required interaction with the authentication plugin in order to obtain an AuthenticatedPeerCredentialToken. This appears to be an artifact of a previous spec revision, since the current API for validate_remote_permissions already provides the AuthenticatedPeerCredentialToken.

So that validate_remote_permissions makes more sense, remove the auth_plugin and remote_identity_handle parameters (Table 32, section 8.4.2.9.2, Table 63, IDL).

Section 8.7 specifies the optional Data Tagging model, but the details of how this works really reside in the Access Control plugin. Sections 1.2 and 2.3 need to be updated to remove references to a Data Tagging "Plugin." Also update section 8.1.1. For clarity, update section 8.7.2 to add a reference to Access Control.

In the IDL get_topic_sec_attributes's 3rd parameter has type TopicSecurityAttributes. In table 32 it appears as EndpointSecurityAttributes which seems to be an error.

Also Figure 10 calls this method get_topic_security_attributes instead of using "sec".

The name of field ac_endpoint_properties in ParticipantSecurityAttributes should be ac_participant_properties as specified in Table 27.

When the middleware calls Auth plugin operations begin_handshake_request and begin_handshake_reply, it must provide serialized_local_participant_data as an OctetSeq according to serialization rules described in 8.3.2.11.4-5. However, these sections do not specify that padding bytes in the serialized data should be initialized to 0. This is desirable so that the OctetSeq can be hashed with consistent results.

Alternatively, revisit the choice to use OctetSeq here in the plugin API. It seems like it would be fine to pass the structure ParticipantBuiltInTopicDataSecure directly to the plugin, as is already done with Access Control.

The class_id attribute in various Tokens includes the Plugin Name and a version number. The intention was that the version number would track the specification version so that it could be used to understand the format of the Token.

However this is not done consistently. Currently the following class_id are used:

As it can be seen some class_ids are missing the version number. The ones that have it use "1.0" instead of "1.1" which is the version of the spec. Finally when there are multiple tokens with the same class ID a suffix preceded by a "+" is uses to differentiate them, but this is not done in all cases.

The goal of this issue is to correct this irregularities. So DDS Security version 1.2 should modify the Token as follows:

Note that this change is not intended to break backwards interoperability. So the proposal could/should be adjusted to ensure that. Specifically the change in the name for the PermissionsCredentialToken should not impact interoperability as this token is not exchanged between participants. The same is true for the addition of the "+AuthPeer" to the AuthenticatedPeerCredentialToken.

The Builtin Authentication Protocol described in 9.3.4.2 'Protocol description' as well as in Table 52 'Actions undertaken by the operations of the builtin Authentication plugin' should be more explicit about how each of the protocol messages is validated.

Specifically it should prescribe that:

• Participant_A shall check the fields inside HandshakeReplyMessageToken and ensure that {Challenge1, Hash(C1), DH1}

  match what Participant A sent in the HandshakeRequestMessageToken.

• Participant_B shall check the fields inside HandshakeFinalMessageToken and ensure that {Hash(C1), Hash(C2), DH1, DH2, Challenge1, Challenge2}

  match what Participant B sent in the HandshakeRequestMessageToken.

This should be made clear both in 9.3.4.2 'Protocol description' and in Table 52 'Actions undertaken by the operations of the builtin Authentication plugin'.

The AES-CCM cypher suite used by the built-in authentication plugins is well suited for higher end processors that have hardware support for AES (e.g. the Intel AES-NI instructions and the the AES instructions in ARMv8).

ARMv7 and other low-end processors don't have hardware support. In these AES can be quite slow.

To better support lower end processors it would be good to add support for a cypher suite that has reasonable performance without hardware support. The industry seems to be converging around the ChaCha20. See:

https://tools.ietf.org/html/rfc8439
https://tools.ietf.org/html/rfc7905

Gerardo

There is only one <permissions> element in a permissions document, but it can contain multiple <grant> elements. Section 9.4.1.3.2.1 states "Each subject name can only appear in a single <permissions> Section"... which should be "single <grant> Section" and "A permissions Section with a subject name that doesn't match" should be "A grant Section with a subject name that doesn't match."

The "Domain Governance Document" and "DomainParticipant permissions document" chapters contain references to "IETF RFC 5761". This doesn't seem correct.

It seems that it should be "IETF RFC 2633". At least then the mentioned RFC sections fit.

Currently the INFO_SOURCE can only be protected by RTPS protection. This makes anyone on the Domain able to modify it. Moreover it must be left unprotected when communicating with unsecured participants.

It would be better if it was included into the DATA / DATA_FRAG submessge as an inlineQos

The IDL used by the specification should be updated to the latest IDL 4.2 syntax.

This specially impacts annotations. For example @Extensibility(MUTABLE_EXTENSIBILITY) has been replace with @mutable.

Section 9.6 'Builtin Logging Plugin' should specify the Qos used to publish and subscribe to the BuiltinLoggingTopic, similar to how it is done for other builtin topics introduced by DDS-Security.

See for example how it is done for the DCPSParticipantMessageSecure builtin Topic (section 7.4.2) where it refers to the Qos of other builtin Topics and also how it is done for the TopicBuiltinParticipantVolatileMessageSecureWriter and BuiltinParticipantVolatileMessageSecureReader (section 7.4.4.2 ) where it defines it explicitly.

8.5.1.9.4 (4th pgh) describes that encode_rtps_message may determine that no encoding is necessary. This is currently an awkward fit with the common API design of returning a boolean and "setting" or "not setting" an exception struct. What does "not setting" the exception struct mean in practice? How does an implementation portably handle this?

It would be better to have an enum or integer return type so that the plugin can more directly influence the implementation's control flow, with distinct return values for OK_ENCODED, OK_NOT_ENCODED, ERROR, possibly others.

A similar issue occurs with decode_rtps_message (8.5.1.9.5). Because key exchange happens asynchronously with respect to encoded message transmission, the plugin may not be able to decode a given message. Simply "returning an exception" in this case is not ideal since the calling code should probably log this exception and it may raise alarms.

Extending this idea of enum (or integer constants) return types instead of boolean to other CryptoTransform operations may also be useful. For example, the other encode operations could return a OK_NOT_ENCODED instead of copying data.

The Permission File is an XML file. The signature is also encoded as text. For large systems with a lot of Topics the size can grow to be quite big. This is amplified by the fact that it is possible and convenient to combine the permissions of multiple participants into a single file. This size can exceed the 64KB max size of IP.

An additional problem is that the permissions file is sent during the authentication handshake. The authentication handshake uses a special "best efforts stateless" that does not support fragmentation of large packets. This is done on purpose to make the channel to be robust to sequence number attacks but it results on the inability to send these large permission files.

This could be addressed by separating the permissions from the authentication handshake and there is already an issue filed for this, see DDSSEC12-13.

However there is a simple solution that can make the current approach more scalable. The proposed approach is to send the Permissions document compressed rather than in their current text form.

Users are already hitting this limit so this issue requests that capability to send the permissions compressed is added with high priority, even if later a more general solution is developed as requested in DDSSEC12-13.

The RTPS specification allows implementations to send implementation-specific RTPS sub-messages inside an RTPS message. These submessages are distinguished by having submessageId's within a specific range.

Prior to DDS security skipping implementation-specific RTPS sub-messages was very efficient requiring only the receiver to jump their length indicated in. the header.
However with DDS security the submessages can be encrypted inside a SRTPS_PREFIX/SUFFIX envelope this requires the receiver to always decrypt them even if it will end up skipping them.
An extreme, but likely common case is when the SRTPS_PREFIX/SUFFIX contains exclusively implementation-specific RTPS sub-messages which makes the effort to decrypt the RTPS message fruitless.

A way to resolve this would be to designate a flag in the SRTPA_PREFIX submessage to indicate that the content contains only vendor-specific submessages. A receiver can use this flag to skip the whole RTPS message avoiding the effort to decrypt i n the event that the sender vendorId does not match the receiver's.

The proposal therefore is to designate the leftmost flag of the SRTPS_PREFIX submessage to indicate it contains only "vendor specific content".

Handles that are allocated by plugins need to have a "return" operation so that the memory used by the plugin can be freed when the handle is no longer needed by the middleware.

boolean return_permissions_handle(in PermissionsHandle handle,
  inout SecurityException ex);

check_remote_datawriter_register_instance defined in 8.4.2.9.15 has these parameters: permissions_handle, reader, publication_handle, key, exception.

In the IDL file and Table 32 in 8.4.2.9 there's also an instance_handle parameter, which should be removed.

The operations get_topic_sec_attributes, get_datareader_sec_attributes and get_datawriter_sec_attributes are missing from Table 63 - Actions undertaken by the operations of the bulitin AccessControl plugin.

It would be helpful to implementors to include some known implementation best-practices in the CryptoTransform

The specification provides mechanisms to extend the set of cryptographic algorithms in future revisions.
The specification also provides mechanisms to extend the set of cryptographic algorithms by a single vendor without requiring a revision of the standard. Those extended algorithms will be understood by the vendor and ignored by others.
What is not clear is if two vendors can agree to use another cryptographic algorithm and interoperate. This use-case may be relevant to an end-user that is using multiple vendors and wants to have those vendors work together to use a new crypto algorithm

This issue requests some mechanism to support this last use case.

The RTPS spec allows a single RTPS message to contain multiple INFO_SRC submessages, effectively changing the Participant GUID of the sending Participant.

This seems problematic for DDS Security since the cryptograophic material is tied to that Participant GUID (e.g. the RTPS protection).
Also INFO_SRC does not work well with the new RTPS_HEADER_EXTENSION.

How commonly is this capability used? It is worth finding a way to make DDS-Security work with it or would it be better to disallow it.

Should it be disallowed just for DDS-Security or for DDS in general?

When "performing authentication only," the encode_serialized_payload operation wraps the SerializedPayload inside CryptoHeader and CryptoFooter. This resulting byte stream takes the place of the original SerializedPayload submessage element in a Data(Frag) submessage.

On the receiving side, this modified submessage element is passed to decode_serialized_payload. The problem comes in parsing this CryptoHeader-SeralizedPayload-CryptoFooter group.

The CryptoHeader is of fixed size and only contains octet-width data (therefore has no padding), so parsing it and determining where SerializedPaylod starts is trivial.

Then the implementation needs to determine where SerializedPayload ends in order to determine which bytes to authenticate. There is no in-stream indication of where the SerializePayload ends.

One possibility would be to look at the end of the byte sequence that decode_serialized_payload received and "step backwards" by the length of the CryptoFooter, however the CryptoFooter is variable length (with receiver_specific_macs). Even if the implementation has external knowledge that receiver_specific_macs are not in use, the alignment requirement of the plugin_sec_tag.receiver_specific_macs.length effectively makes this a variable-length element (also see issue #58).

To resolve this, an additional element for "length" could be added before the SerializedPayload, just like the CryptoContent submessage element does. This would make parsing the encoded payload similar for the encrypt and auth-only cases.

Defer to next RTF

Defer to next RTF

When "performing authentication only," the encode_serialized_payload operation wraps the SerializedPayload inside CryptoHeader and CryptoFooter. This resulting byte stream takes the place of the original SerializedPayload submessage element in a Data(Frag) submessage.

On the receiving side, this modified submessage element is passed to decode_serialized_payload. The problem comes in parsing this CryptoHeader-SeralizedPayload-CryptoFooter group.

The CryptoHeader is of fixed size and only contains octet-width data (therefore has no padding), so parsing it and determining where SerializedPaylod starts is trivial.

Then the implementation needs to determine where SerializedPayload ends in order to determine which bytes to authenticate. There is no in-stream indication of where the SerializePayload ends.

One possibility would be to look at the end of the byte sequence that decode_serialized_payload received and "step backwards" by the length of the CryptoFooter, however the CryptoFooter is variable length (with receiver_specific_macs). Even if the implementation has external knowledge that receiver_specific_macs are not in use, the alignment requirement of the plugin_sec_tag.receiver_specific_macs.length effectively makes this a variable-length element (also see issue #58).

To resolve this, an additional element for "length" could be added before the SerializedPayload, just like the CryptoContent submessage element does. This would make parsing the encoded payload similar for the encrypt and auth-only cases.

The description of is_write_protected starts with "Indicates if read access". This should be "Indicates if write access."

Defer to next RTF

Defer to next RTF

domainId is not listed in section 8.4.2.9.12

Defer to next RTF

Defer to next RTF

domainId is not listed in section 8.4.2.9.12

In 9.5.3.3.4.2 remove this part:
"Note that the cipher operations have 16-byte block-size and add padding when needed. Therefore the secure data.length (“N”) will always be a multiple of 16."

AES-GCM doesn't add padding. The implication of this is that the CryptoContent Submessage Element may end at an arbitrary point in the stream (from the point of view of alignment). Bringing the stream "back into alignment" will depend on the usage context:

• When encoding payload, the CryptoFooter follows directly after CryptoContent. This means that CryptoFooter's first element (common_mac) may start unaligned. Then receiver_specific_macs.length appears in the stream as a 32-bit value so it will be preceded by 0-3 padding bytes depending on the length of CryptoContent.
• When encoding a submessage or full message, CryptoContent is followed by the start of another submessage which must be aligned to 4 per the RTPS spec.

Defer to next RTF

Defer to next RTF

In 9.5.3.3.4.2 remove this part:
"Note that the cipher operations have 16-byte block-size and add padding when needed. Therefore the secure data.length (“N”) will always be a multiple of 16."

AES-GCM doesn't add padding. The implication of this is that the CryptoContent Submessage Element may end at an arbitrary point in the stream (from the point of view of alignment). Bringing the stream "back into alignment" will depend on the usage context:

• When encoding payload, the CryptoFooter follows directly after CryptoContent. This means that CryptoFooter's first element (common_mac) may start unaligned. Then receiver_specific_macs.length appears in the stream as a 32-bit value so it will be preceded by 0-3 padding bytes depending on the length of CryptoContent.
• When encoding a submessage or full message, CryptoContent is followed by the start of another submessage which must be aligned to 4 per the RTPS spec.

9.5.3.3.6 "The message digest is computed on the crypto_header and the ciphertext."

This statement appears to contradict the descriptions of common_mac and receiver_specific_macs in 9.5.3.3.4.4 (payload), 9.5.3.3.4.5 (submessage), and 9.5.3.3.4.6 (message).

Defer to next RTF

Defer to next RTF

XML Schema doesn't use all-caps TRUE and FALSE.
https://www.w3.org/TR/xmlschema-2/#boolean

Defer to next RTF

Defer to next RTF

9.5.3.3.6 "The message digest is computed on the crypto_header and the ciphertext."

This statement appears to contradict the descriptions of common_mac and receiver_specific_macs in 9.5.3.3.4.4 (payload), 9.5.3.3.4.5 (submessage), and 9.5.3.3.4.6 (message).

XML Schema doesn't use all-caps TRUE and FALSE.
https://www.w3.org/TR/xmlschema-2/#boolean

In the Table 70 row for register_local_datareader, the ReaderKeyMaterial is created based on Data Protection Kind. This should be Metadata Protection Kind because the ReaderKeyMaterial signs/encrypts submessages and not data payloads.

Defer to next RTF

Defer to next RTF

The term CryptoKeyTransform is used 3 times (section name of 9.5.3.3, text of 9.5.3.3.1, caption of Table 72). These should be CryptoTransform.

Defer to next RTF

Defer to next RTF

participant_security_attributes is currently spelled participant_security_atributes on page 25 of the specification.

Defer to next RTF

Defer to next RTF

In the Table 70 row for register_local_datareader, the ReaderKeyMaterial is created based on Data Protection Kind. This should be Metadata Protection Kind because the ReaderKeyMaterial signs/encrypts submessages and not data payloads.

The term CryptoKeyTransform is used 3 times (section name of 9.5.3.3, text of 9.5.3.3.1, caption of Table 72). These should be CryptoTransform.

Section 9.3.1.3 (Identity Certificate) indicates it is possible to use certificate chains.

However the language of this section is confusing because an X.509 v3 Certificate does not contain a chain. It is a single certificate. What can contain a chain of certificates is a file. E.g. the standard "PEM" file format referenced in Table 44 can contain a certificate chain.

To clarify this the following modifications are suggested:
(1) In table 44, row for "identity_certificate" change from:
"URI to a X509 certificate signed by the IdentityCA in PEM format"...
To
"URI to a X509 certificate (or certificate chain) signed by the IdentityCA, either as the root CA or as an intermediate CA, in PEM format"

(2) In section 9.3.1.3 change from:
"An X.509 v3 Certificate [39] that chains up to the Identity CA (see 9.3.1.1)."
To:
"A X.509 v3 Certificate chain [39] that is signed by the Identity CA (see 9.3.1.1) either as the root or as an intermediate CA in the chain"

(3) In section 9.3
In bullet 3. Clarify it may not be a single X.509 rather it can be a chain.

(4) In table 49, field "c.id" indicate this is not a single certificate but a certificate chain. As that is what was configured in the PropertyQosPolicy.

(5) In table 50, field "c.id" indicate this is not a single certificate but a certificate chain. As that is what was configured in the PropertyQosPolicy.

(5) In table 52, field "validate_local_identity" indicate this is not a single certificate but a certificate chain.

(6) In Table 53 IdentityCertificate is undefined. Maybe it should be "identity_certificate" and refer to the property defined in Table 44.

Defer to next RTF

Defer to next RTF

Section 9.3.1.3 (Identity Certificate) indicates it is possible to use certificate chains.

However the language of this section is confusing because an X.509 v3 Certificate does not contain a chain. It is a single certificate. What can contain a chain of certificates is a file. E.g. the standard "PEM" file format referenced in Table 44 can contain a certificate chain.

To clarify this the following modifications are suggested:
(1) In table 44, row for "identity_certificate" change from:
"URI to a X509 certificate signed by the IdentityCA in PEM format"...
To
"URI to a X509 certificate (or certificate chain) signed by the IdentityCA, either as the root CA or as an intermediate CA, in PEM format"

(2) In section 9.3.1.3 change from:
"An X.509 v3 Certificate [39] that chains up to the Identity CA (see 9.3.1.1)."
To:
"A X.509 v3 Certificate chain [39] that is signed by the Identity CA (see 9.3.1.1) either as the root or as an intermediate CA in the chain"

(3) In section 9.3
In bullet 3. Clarify it may not be a single X.509 rather it can be a chain.

(4) In table 49, field "c.id" indicate this is not a single certificate but a certificate chain. As that is what was configured in the PropertyQosPolicy.

(5) In table 50, field "c.id" indicate this is not a single certificate but a certificate chain. As that is what was configured in the PropertyQosPolicy.

(5) In table 52, field "validate_local_identity" indicate this is not a single certificate but a certificate chain.

(6) In Table 53 IdentityCertificate is undefined. Maybe it should be "identity_certificate" and refer to the property defined in Table 44.

The behavior of the built-in CryptoKeyFactory operations is described in Table 70. Many entries of this table include direct references to details of the built-in Access Control plugin's configuration file (an example, "see 9.4.1.2.5.6").

It would be easier to follow and more modular if this was changed to instead reference the data structure that the CryptoKeyFactory can actually see, which in this case is ParticipantSecurityAttributes.

Defer to next RTF

Defer to next RTF

All the other flag names defined here take the form of "PLUGIN_PARTICIPANT_SECURITY_ATTRIBUTES_FLAG_IS_XXX_YYY". Should the BUILTIN part of this flag really be there? I believe this should instead read: PLUGIN_PARTICIPANT_SECURITY_ATTRIBUTES_FLAG_IS_DISCOVERY_ENCRYPTED.

Defer to next RTF

Defer to next RTF

The behavior of the built-in CryptoKeyFactory operations is described in Table 70. Many entries of this table include direct references to details of the built-in Access Control plugin's configuration file (an example, "see 9.4.1.2.5.6").

It would be easier to follow and more modular if this was changed to instead reference the data structure that the CryptoKeyFactory can actually see, which in this case is ParticipantSecurityAttributes.

Table 70 introduces the term "Participant2ParticipantKxKeyMaterial" which isn't used anywhere else in the spec.

It would be helpful to have a table that lists each "key material" object along with which operation creates it and how it's used.

Defer to next RTF

Defer to next RTF

All the other flag names defined here take the form of "PLUGIN_PARTICIPANT_SECURITY_ATTRIBUTES_FLAG_IS_XXX_YYY". Should the BUILTIN part of this flag really be there? I believe this should instead read: PLUGIN_PARTICIPANT_SECURITY_ATTRIBUTES_FLAG_IS_DISCOVERY_ENCRYPTED.

Table 70 introduces the term "Participant2ParticipantKxKeyMaterial" which isn't used anywhere else in the spec.

It would be helpful to have a table that lists each "key material" object along with which operation creates it and how it's used.

The last paragraph of 9.5.3.3.4.3 refers to the "CryptoHeader", this should be "CryptoFooter"

Defer to next RTF

Defer to next RTF

The last paragraph of 9.5.3.3.4.3 refers to the "CryptoHeader", this should be "CryptoFooter"

participant_security_attributes is currently spelled participant_security_atributes on page 25 of the specification.

Section 7.2.5 changes QoS in ways that break RTPS compatibility, however minor modifications can fix this.

Because the mapping for PropertyQosPolicy in Table 10 (is this an implied entry in Table 12 as well?) conflicts with RTPS's definition of PID 0x59, the Binary Property values may not be sent on the wire. Note that RTPS has no concept of appendable extensibility, and requires backwards compatibility for all 2.x versions.

We can represent this restriction on PropertyQosPolicy in IDL4:

@extensibility(FINAL)
struct PropertyQosPolicy {
  PropertySeq value;

  @non-serialized
  BinaryPropertySeq binary_value;
};

The practical effect of this change is that any BinaryProperty entry with propagate == TRUE is not actually propagated inside PropertyQosPolicy. However a search through the specification indicates that there is no requirement for this, at least for built-in plugins. Any other plugins are necessarily vendor specific so those are not necessarily restricted from using an appendable policy, as long as they are aware of the compatibility issues (for allow_unauthenticated_participants == TRUE).

Also, for consistency the Tag and DataTags structs could be made @extensibility(FINAL). This is not as important since only Security-aware implementations will know about DataTagQosPolicy.

Defer to next RTF

Defer to next RTF

Section 7.2.5 changes QoS in ways that break RTPS compatibility, however minor modifications can fix this.

Because the mapping for PropertyQosPolicy in Table 10 (is this an implied entry in Table 12 as well?) conflicts with RTPS's definition of PID 0x59, the Binary Property values may not be sent on the wire. Note that RTPS has no concept of appendable extensibility, and requires backwards compatibility for all 2.x versions.

We can represent this restriction on PropertyQosPolicy in IDL4:

@extensibility(FINAL)
struct PropertyQosPolicy {
  PropertySeq value;

  @non-serialized
  BinaryPropertySeq binary_value;
};

The practical effect of this change is that any BinaryProperty entry with propagate == TRUE is not actually propagated inside PropertyQosPolicy. However a search through the specification indicates that there is no requirement for this, at least for built-in plugins. Any other plugins are necessarily vendor specific so those are not necessarily restricted from using an appendable policy, as long as they are aware of the compatibility issues (for allow_unauthenticated_participants == TRUE).

Also, for consistency the Tag and DataTags structs could be made @extensibility(FINAL). This is not as important since only Security-aware implementations will know about DataTagQosPolicy.

There are a few cross-references that don't resolve, resulting in the text "see 0" or "in subclause 0" in the spec:

• Table 30 plugin_endpoint_attributes column 3 contains "found in 0"
• Table 52 get_authenticated_peer_credential_token contains "See section 0"
• 9.4.1.2.6.7 right before the bullet list
• Table 63 validate_remote_permissions
• 9.5.3.3.4.1 final paragraph has "subclause 0"

Defer to next RTF

Defer to next RTF

In the normative IDL, the struct ParticipantSecurityAttributes's field plugin_participant_attributes has type ParticipantSecurityAttributesMask but it should have type PluginParticipantSecurityAttributesMask

Defer to next RTF

Defer to next RTF

In the normative IDL, the struct ParticipantSecurityAttributes's field plugin_participant_attributes has type ParticipantSecurityAttributesMask but it should have type PluginParticipantSecurityAttributesMask

Section 7.4.4.2 "TopicSecurityAtributes" should be "Attributes"

Section 8.8.9.1, 8.8.9.2, and 8.8.9.3 "cripto" should be "crypto"

Section 8.8.3 "best-efforts" should be "best-effort"

Section 8.8.4 "simultanepusly" should be "simultaneously"

Table 63 caption: "bulitin" should be "built-in"

Section 9.5 "BEST_EFFORTS" should be "BEST_EFFORT"

Section 9.5.2.3 "indentifier" should be "identifier"

Section 9.5.3.1 "ciphetext' should be "ciphertext"

Normative IDL parameter 1 of create_local_data_crypto_tokens "cryto" should be "crypto"

Defer to next RTF

Defer to next RTF

Section 7.4.1.3 'Reserved RTPS parameter IDs' reserves the whole 0x1000 to 0x1FFF (plus 0x5000 to 0x5FFF for must-understand) . That is 2 x 4096 PIDs. In reality DDS security only uses 6 PIDs... So this is a bit too much of a land grab.

It would be better to be more conservative and reserve a smaller range which can then be expanded as needed.

RTPS version 2.4 states that the reserved range for DDS security is 0x1000 to 0x10FF and 0x5000 to 0x50FF.

Section 7.4.1.3 should be updated to reflect this reduced range.

Defer to next RTF

Defer to next RTF

In CryptoKeyFactory, all operations other than the unregister_* ones return handles. But the descriptions of register_matched_remote_participant and register_local_datawriter (8.5.1.7.2-3) end with "operation returns false". That should be "operation returns HandleNIL".

Defer to next RTF

Defer to next RTF

Section 7.4.1.3 'Reserved RTPS parameter IDs' reserves the whole 0x1000 to 0x1FFF (plus 0x5000 to 0x5FFF for must-understand) . That is 2 x 4096 PIDs. In reality DDS security only uses 6 PIDs... So this is a bit too much of a land grab.

It would be better to be more conservative and reserve a smaller range which can then be expanded as needed.

RTPS version 2.4 states that the reserved range for DDS security is 0x1000 to 0x10FF and 0x5000 to 0x50FF.

Section 7.4.1.3 should be updated to reflect this reduced range.

In Table 48 (9.3.2.4), "properties" should be "binary_properties" so that the property future_challenge is treated as binary, matching Table 49's challenge1.

Defer to next RTF

Defer to next RTF

In CryptoKeyFactory, all operations other than the unregister_* ones return handles. But the descriptions of register_matched_remote_participant and register_local_datawriter (8.5.1.7.2-3) end with "operation returns false". That should be "operation returns HandleNIL".

In Table 48 (9.3.2.4), "properties" should be "binary_properties" so that the property future_challenge is treated as binary, matching Table 49's challenge1.

Section 7.4.4.2 "TopicSecurityAtributes" should be "Attributes"

Section 8.8.9.1, 8.8.9.2, and 8.8.9.3 "cripto" should be "crypto"

Section 8.8.3 "best-efforts" should be "best-effort"

Section 8.8.4 "simultanepusly" should be "simultaneously"

Table 63 caption: "bulitin" should be "built-in"

Section 9.5 "BEST_EFFORTS" should be "BEST_EFFORT"

Section 9.5.2.3 "indentifier" should be "identifier"

Section 9.5.3.1 "ciphetext' should be "ciphertext"

Normative IDL parameter 1 of create_local_data_crypto_tokens "cryto" should be "crypto"

There are a few cross-references that don't resolve, resulting in the text "see 0" or "in subclause 0" in the spec:

• Table 30 plugin_endpoint_attributes column 3 contains "found in 0"
• Table 52 get_authenticated_peer_credential_token contains "See section 0"
• 9.4.1.2.6.7 right before the bullet list
• Table 63 validate_remote_permissions
• 9.5.3.3.4.1 final paragraph has "subclause 0"

DDS security does not provide any additional means to secure the DDS-RPC service calls. Absent this the protection would be based on the underlying DDS Topics.

DDS-RPC maps all operations of an interface to two unkeyed Topics. One for the request and one for the reply. The request Topic is used for all operations and likewise the reply Topic.

Thus relying on the underlying Topics would provide access control granularity at the level of the service and not allow to give narrower permissions. E.g. invoke certain operations and not others. This is not acceptable in some deployment situations.

One solution may be to make the Topics keyed by the operation. This is possible now that IDL42 allows the discriminator of a union to act as the key.

Alternatively DDS-Security could specify some other mechanism, for example designate a data-tag to indicate the operations that the request DataWriter may invoke. The receiver side would check that the operation invoked matches the DataWriter tag before processing the request.

Defer to next RTF

Defer to next RTF

In section 8.8.2.2, it's established that the BuiltinParticipantStatelessMessage built-in endpoints are used for exchanging the "authentication handshake" messages. This makes sense based on how those built-in endpoints are defined in 7.4.3 and how they're used in some parts of 8.3.2.9 and .11.

However there are parts of 8.3.2.11 and 8.8.2 that describe this handshake using the names BuiltinParticipantMessageWriter / Reader (note the absence of "Stateless"). The name BuiltinParticipantMessageWriter is defined by the RTPS spec as the endpoint used to implement the participant scoped liveliness (see RTPS 2.2 section 8.4.13.2).

Defer to next RTF

Defer to next RTF

DDS security does not provide any additional means to secure the DDS-RPC service calls. Absent this the protection would be based on the underlying DDS Topics.

DDS-RPC maps all operations of an interface to two unkeyed Topics. One for the request and one for the reply. The request Topic is used for all operations and likewise the reply Topic.

Thus relying on the underlying Topics would provide access control granularity at the level of the service and not allow to give narrower permissions. E.g. invoke certain operations and not others. This is not acceptable in some deployment situations.

One solution may be to make the Topics keyed by the operation. This is possible now that IDL42 allows the discriminator of a union to act as the key.

Alternatively DDS-Security could specify some other mechanism, for example designate a data-tag to indicate the operations that the request DataWriter may invoke. The receiver side would check that the operation invoked matches the DataWriter tag before processing the request.

If the crypto plugin transforms the payload but the layers containing the payload (submessage or message) are not transformed, the resulting Data/Frag submessage contains no indication that its payload has a nonstandard format.
Since this spec is essentially defining a new minor version of RTPS (along with RTPS RTF process concurrent to this), one of the reserved flags in the submessage header of Data/Frag can be defined as FLAG_S meaning that the format of the SerializedPayload submessage element is defined by the security spec.

Defer to next RTF

Defer to next RTF

In section 8.8.2.2, it's established that the BuiltinParticipantStatelessMessage built-in endpoints are used for exchanging the "authentication handshake" messages. This makes sense based on how those built-in endpoints are defined in 7.4.3 and how they're used in some parts of 8.3.2.9 and .11.

However there are parts of 8.3.2.11 and 8.8.2 that describe this handshake using the names BuiltinParticipantMessageWriter / Reader (note the absence of "Stateless"). The name BuiltinParticipantMessageWriter is defined by the RTPS spec as the endpoint used to implement the participant scoped liveliness (see RTPS 2.2 section 8.4.13.2).

If the crypto plugin transforms the payload but the layers containing the payload (submessage or message) are not transformed, the resulting Data/Frag submessage contains no indication that its payload has a nonstandard format.
Since this spec is essentially defining a new minor version of RTPS (along with RTPS RTF process concurrent to this), one of the reserved flags in the submessage header of Data/Frag can be defined as FLAG_S meaning that the format of the SerializedPayload submessage element is defined by the security spec.

9.4.1.2.6.4 describes Enable Read Access Control, however the bullet list within this section uses the xml tag <enable_write_access_control> (twice), it should be <enable_read_access_control>.

Defer to next RTF

Defer to next RTF

9.4.1.2.6.4 describes Enable Read Access Control, however the bullet list within this section uses the xml tag <enable_write_access_control> (twice), it should be <enable_read_access_control>.

The rows in Table 61 is_submessage_encrypted and is_payload_encrypted each have paragraphs starting "If is_*_encrypted is FALSE..." and including "GCM authenticated encryption", which should be "GMAC authentication transformation" like the corresponding rows of Table 59.

Defer to next RTF

Defer to next RTF

In Table 30, the entry for is_payload_protected says "If is_payload_protected is FALSE, then the CryptoKeyFactory, KeyExchange and CryptoTransform operations are called only if is_payload_protected is TRUE", but the last phrase should be "only if is_submessage_protected is TRUE".

Defer to next RTF

Defer to next RTF

The rows in Table 61 is_submessage_encrypted and is_payload_encrypted each have paragraphs starting "If is_*_encrypted is FALSE..." and including "GCM authenticated encryption", which should be "GMAC authentication transformation" like the corresponding rows of Table 59.

In Table 30, the entry for is_payload_protected says "If is_payload_protected is FALSE, then the CryptoKeyFactory, KeyExchange and CryptoTransform operations are called only if is_payload_protected is TRUE", but the last phrase should be "only if is_submessage_protected is TRUE".

2nd parameter to get_topic_sec_attributes has type "String", should be "string".

Defer to next RTF

Defer to next RTF

2nd parameter to get_topic_sec_attributes has type "String", should be "string".

The last paragraph of 7.4.2 indicates that TopicSecurityAttributes::is_liveliness_protected is used to determine if a liveliness message should be sent using the new DCPSParticipantMessageSecure builtin endpoint or the old DCPSParticipantMessage builtin endpoint.

However it is not clear which topic's TopicSecurityAttributes are to be used. The liveliness message essentially belongs to the participant and not any given topic (see RTPS v2.2 8.4.13.5). Should the security spec use ParticipantSecurityAttributes here instead?

Defer to next RTF

Defer to next RTF

Section 7.4.3.2:

The data type associated with these endpoints is ParticipantStatelessMessage defined
below (see also 7.2.5):
typedef ParticipantStatelessMessage ParticipantGenericMessage;

Two issues:
a. I don't see how this relates to 7.2.5, should that be 7.2.6?
b. The typedef is backwards. Typedef declares a new name (second) from an existing type (first).

Defer to next RTF

Defer to next RTF

The last paragraph of 7.4.2 indicates that TopicSecurityAttributes::is_liveliness_protected is used to determine if a liveliness message should be sent using the new DCPSParticipantMessageSecure builtin endpoint or the old DCPSParticipantMessage builtin endpoint.

However it is not clear which topic's TopicSecurityAttributes are to be used. The liveliness message essentially belongs to the participant and not any given topic (see RTPS v2.2 8.4.13.5). Should the security spec use ParticipantSecurityAttributes here instead?

Section 7.4.3.2:

The data type associated with these endpoints is ParticipantStatelessMessage defined
below (see also 7.2.5):
typedef ParticipantStatelessMessage ParticipantGenericMessage;

Two issues:
a. I don't see how this relates to 7.2.5, should that be 7.2.6?
b. The typedef is backwards. Typedef declares a new name (second) from an existing type (first).

SubscriptionBuiltinTopicDataSecure should inherit from SubscriptionBuiltinTopicData in the local module, not the one in the DDS:: module.

Defer to next RTF

Defer to next RTF

Operation: get_datarwriter_sec_attributes
should be
Operation: get_datawriter_sec_attributes

Defer to next RTF

Defer to next RTF

SubscriptionBuiltinTopicDataSecure should inherit from SubscriptionBuiltinTopicData in the local module, not the one in the DDS:: module.

Operation: get_datarwriter_sec_attributes
should be
Operation: get_datawriter_sec_attributes

The permissions document allow and deny rules contain a section for matching the data tags of the DataReader/DataWriter. The match is based on an exact match of the tag-name and tag-value.

For consistency with the way we match the permissions for Topic and Partitions we should also allow an expression for the tag-value portion. This is only for the permission rule itself. Not for the DataTag QosPolicy.

So to match the rule the DataTag name in the Qos must exactly match the name in the permissions document (no expressions here) and the DataTag value in the Qos should match (using fnmatch) the expression in the permissions document.

Defer to next RTF

Defer to next RTF

The permissions document allow and deny rules contain a section for matching the data tags of the DataReader/DataWriter. The match is based on an exact match of the tag-name and tag-value.

For consistency with the way we match the permissions for Topic and Partitions we should also allow an expression for the tag-value portion. This is only for the permission rule itself. Not for the DataTag QosPolicy.

So to match the rule the DataTag name in the Qos must exactly match the name in the permissions document (no expressions here) and the DataTag value in the Qos should match (using fnmatch) the expression in the permissions document.

Normative IDL has no parameter handshake_message_in (from Table 23). It would be possible to use handshake_message_out to serve the purposes of "in" since it's an inout parameter in IDL, but is that the intention here?

Defer to next RTF

Defer to next RTF

In the normative IDL interface CryptoTransform, the decode_datawriter_submessage operation uses the "in" parameter passing mode for SecurityException – should be "out" or "inout".

Defer to next RTF

Defer to next RTF

The normative IDL doesn't have SecureSubmessageCategory_t, but it does have SecureSumessageCategory_t which appears to be a typo.

Defer to next RTF

Defer to next RTF

Normative IDL has no parameter handshake_message_in (from Table 23). It would be possible to use handshake_message_out to serve the purposes of "in" since it's an inout parameter in IDL, but is that the intention here?

In the normative IDL interface CryptoTransform, the decode_datawriter_submessage operation uses the "in" parameter passing mode for SecurityException – should be "out" or "inout".

Table 32 (8.4.2.9) has a topic_name parameter for both get_datawriter_sec_attributes and get_datareader_sec_attributes, however this parameter is not present in the normative IDL.

Defer to next RTF

Defer to next RTF

In the normative IDL, the type of the 2nd parameter for set_permissions_credential_and_token is listed as PermissionsCredential but should PermissionsCredentialToken.

Defer to next RTF

Defer to next RTF

DDS::Security::LongLongSeq is not used so it should be removed from the normative IDL.

Defer to next RTF

Defer to next RTF

Table 32 (8.4.2.9) has a topic_name parameter for both get_datawriter_sec_attributes and get_datareader_sec_attributes, however this parameter is not present in the normative IDL.

In the normative IDL, the type of the 2nd parameter for set_permissions_credential_and_token is listed as PermissionsCredential but should PermissionsCredentialToken.

DDS::Security::LongLongSeq is not used so it should be removed from the normative IDL.

In 7.2.3.1, DataHolder is a struct annotated with @extensibility(APPENDABLE) and no optional fields.
However in ptc/17-09-26 (normative IDL), DataHolder has no @extensibility annotation and two optional fields.

The specification contains full copies of the machine readable XSD files:

• DDS Security Access Control Permissions XSD
• DDS Security Access Control Governance XSD

This causes problems as updates may not always be done consistently.

Therefore it would be better if the specification removed this duplication and just referenced the external machine-readable files.

Defer to next RTF

Defer to next RTF

The specification depends on many other standards that appear referenced thought the specification and in Annex A (References) but do not appear in Section 3.

Section 3 should be expanded to include the subset of those references that are used for normative behavior. [46], [47], and [52] are clear examples of references that should be moved to Section 3.

Defer to next RTF

Defer to next RTF

The UML diagrams in the specification use UML stereotypes, such as <<discovery>>. However these are not documented anywhere in the specification nor do they appear in the XMI.

Also the diagrams are non-standard UML in that they show superclasses in the top right corner. See for example Figure 10.

These issues should be corrected.

Defer to next RTF

Defer to next RTF

The specification contains full copies of the machine readable XSD files:

• DDS Security Access Control Permissions XSD
• DDS Security Access Control Governance XSD

This causes problems as updates may not always be done consistently.

Therefore it would be better if the specification removed this duplication and just referenced the external machine-readable files.

The specification depends on many other standards that appear referenced thought the specification and in Annex A (References) but do not appear in Section 3.

Section 3 should be expanded to include the subset of those references that are used for normative behavior. [46], [47], and [52] are clear examples of references that should be moved to Section 3.

The authentication handshake requires participants to exchange their permission files. This is done as clear text.

Visibility into the permissions file leaks information as to what the system Topic names are as well as what the application publishes and subscribes. This can be sensitive for some systems.

Aside form this sending this document is potentially expensive as the knowledge could be available on a separate channel.

The RTF should look share this information only with authenticated participants and possibly also avoid sending it if it is already known to the peer participant.

Related this is the fact that permissions are "monolithic" of you need to add a permission to a Participant you need to create and sign a new document. Would be nice to have some way to grant/propagate incremental permissions. E.g. something that could be bundled into the propagation of the Endpoint discovery data itself.

Defer to next RTF

Defer to next RTF

The authentication handshake requires participants to exchange their permission files. This is done as clear text.

Visibility into the permissions file leaks information as to what the system Topic names are as well as what the application publishes and subscribes. This can be sensitive for some systems.

Aside form this sending this document is potentially expensive as the knowledge could be available on a separate channel.

The RTF should look share this information only with authenticated participants and possibly also avoid sending it if it is already known to the peer participant.

Related this is the fact that permissions are "monolithic" of you need to add a permission to a Participant you need to create and sign a new document. Would be nice to have some way to grant/propagate incremental permissions. E.g. something that could be bundled into the propagation of the Endpoint discovery data itself.

The Security plugin model supports being able to control access to each DDS Topic instance (instead of just controlling access to the DDS Topic). However the builtin plugin permission files do not provide a way to configure these permissions.

This is important for many DDS-Security use-cases where each instance represents a different object and needs to separately indicate who has access to it. For example imaging a "CarPosition" topic that is used to publish the position of cars. Someone may be given permission to suscribe or publish its own car position, but not other cars. Likewise a manufactured or insurance company may be given permissions to the cars that they manage but not others.

Defer to next RTF

Defer to next RTF

The Security plugin model supports being able to control access to each DDS Topic instance (instead of just controlling access to the DDS Topic). However the builtin plugin permission files do not provide a way to configure these permissions.

This is important for many DDS-Security use-cases where each instance represents a different object and needs to separately indicate who has access to it. For example imaging a "CarPosition" topic that is used to publish the position of cars. Someone may be given permission to suscribe or publish its own car position, but not other cars. Likewise a manufactured or insurance company may be given permissions to the cars that they manage but not others.

At end of 2nd to last para document has (matching the errata):

"sending the ParticipantSecurityInfo (the default value of zero has is_valid=0) or sending it with is_valid."

It should probably be

"sending the ParticipantSecurityInfo (the default value of zero has is_valid=0) or sending it with is_valid set to zero.”

Defer to next RTF

Defer to next RTF

In 7.2.3.1, DataHolder is a struct annotated with @extensibility(APPENDABLE) and no optional fields.
However in ptc/17-09-26 (normative IDL), DataHolder has no @extensibility annotation and two optional fields.

Defer to next RTF

Defer to next RTF

At end of 2nd to last para document has (matching the errata):

"sending the ParticipantSecurityInfo (the default value of zero has is_valid=0) or sending it with is_valid."

It should probably be

"sending the ParticipantSecurityInfo (the default value of zero has is_valid=0) or sending it with is_valid set to zero.”