If the RTPS_HE submessage is present, can we impose a restriction that requires it to be the first submessage of the RTPS packet? Placing it anywhere else renders some of its fields less useful (or useless). It also makes handling things like checksums trickier, as it requires you to:
1. Iterate over all submessages to determine if the RTPS_HE is present,
2. Verify the checksum (if present),
3. Go back to start, iterate over all submessages again and now process their contents, since we have now validated the checksum.

Other fields like the length of message also becomes less useful if it's the last submessage.

The specification should specify how string parameters should be escaped when used within a query string, for example a query_expression of "name = 'bar'", but what if bar is a string with a single quote, newline, tab, etc? The specification should specify the escaping rules for that.

Annex B declares MinimalStructMemberSeq as:

// Member of an aggregate type
@extensibility(APPENDABLE) @nested
struct MinimalStructMember {
    CommonStructMember common;
    MinimalMemberDetail detail;
};
// Ordered by common.member_id
typedef sequence<MinimalStructMember> MinimalStructMemberSeq;

The comment is wrong: It contradicts section 7.3.4.5 where it says it should be ordered by member index. It is also inconsistent with struct CompleteStructMemberSeq before.

Instead, the comment before MinimalStructMemberSeq it should say:

// Ordered by the member_index

The DDS spec is not clear about what should happen in the following scenario:
A reader is RELIABLE and KEEP_ALL, with max_instances set to n. A matching Writer is found, but this Writer doesn't have a similar instance limit. (Or alternatively, there are multiple Writers that together violate the instance limit of the Reader.)

Now in this case, the Reader accepts the first n instances, but when instance n+1 is received it rejects the incoming sample based on exceeding its configured max_instances. But what should happen next? If I permanently drop that sample I violate the Reliability and/or KEEP_ALL policy. If I try to retransmit it, I would very likely run into the same instance limit over and over again, since instances will only be purged when unregistered by the Writer and consumed on the Reader side.

So in the given scenario, should we expect the Writer to keep re-transmitting instance n+1 (subsequently blocking out any other samples following the rejected sample potentially belonging to existing instances due to the order preservation requirements of the Reliability policy?),. or should we expect it to just give up when a sample was rejected by instance limit? In the latter case, would this mean that the Reader would just acknowledge a Rejected sample just to avoid the Writer from retransmitting it? And should the Reader just expect a rejected instance to be lost forever?

And what happens if another instance gets purged, making room for the instance we just rejected before? For VOLATILE, BEST_EFFORT data it might be acceptable that you wait until the next update for this instance arrives, but what if the data is TRANSIENT_LOCAL or above? How does the Reader even ask for retransmission of the rejected sample, given that it might have already acknowledged it before?

I short, I think the spec should make more clear what the consequences are of an instance limit violation on the Reader side.

The description of the history depth QoS setting indicates it is optional and only used when KEEP_LAST is the history kind, and that it is a long (or sometimes referred to as an integer/int), and that the default value is 1, and that it needs to be consistent with related RESOURCE_LIMITS QoS settings, but it does not specify a bounds nor does it explicitly state that the bounds are actually any valid (signed) long value. So a few boundary cases are not well defined, for example:

• is KEEP_LAST with depth 0 valid? If so, is it a special behavior?
• what about negative values for depth?

RTI's Connext Pro documentation limits it to values greater than zero and less than 100,000,000:

> "[range] [1,100 million], <= DDS_ResourceLimitsQosPolicy::max_samples_per_instance"
– https://community.rti.com/static/documentation/connext-dds/6.0.1/doc/api/connext_dds/api_c/structDDS__HistoryQosPolicy.html#aef0fb3fd3579866be17d1a936f5e3729

Other implementations like Cyclone DDS and Fast-DDS will accept values of zero, though it's not clear what they're behavior is in that case.

Those are useful to imply what the behavior is, but I think the standard should probably take stance on the valid values for depth, and I think values >= 1 makes sense, though the "100 million" upper bound set by RTI's API seems kind of arbitrary, though I don't have a better alternative in mind.

The bullet point for " This profile adds two things " goes on to list three things. The third is about History QoS, which makes it appear to the reader like this was added on later and not initially part of the profile. That itself is not a problem, but for clarity this section should provide a rationale as to why the History QoS Policy is involved in the definition of Ownership Profile.

Suppose the DATA_AVAILABLE StatusChangedFlag (see Figure 2.16) is set on a DataReader and then a listener (with DATA_AVAILABLE_STATUS in its mask) is attached to the DataReader by the user calling set_listener(). The specification does not specify if the listener should be invoked. If the decision is that the listener is not invoked, then guidance should be given to users that they may need to invoke the listener manually.

The PIM contains definitions for most of the "reference semantic" classes. But it does not directly define the "value semantic" classes such as: Time_t, Duration_t, SequenceNumber_t, InstanceHandle_t, StatusKind, StatusMask, etc.

Some types like ReturnCode_t are defined. But this is done in an odd place: (section 2.2.1.1 'Format and Conventions').

The PIM would be more clear if these types were all defined clearly in a centralized place. For example the Infrastructure module.

This issue should be treated as a "Blocker" so it is resolved ahead of other issues. That way the other issues can refer to the new definitions and organization.

The specification does not fully define the meaning of a Duration_t when the second part is negative.

struct Duration_t {
    long sec;
    unsigned long nanosec;
};

What is he interpretation of a value when sec <0?
Does the nanosec part get interpreted to have the same sign?
If so, then the number between 0 < x < -1 cannot be represented

So the most logical way to interpret a negative value for the sec field appears to be:

value = sec + nanosec 

where sec can be positive or negative but nanosec is always positive.
So the value of negative 1ms would be represented as:

sec = -1
nanosec = 999,000,000

The specification should define the proper interpretation.

The `getTypeDependencies` operation (7.6.3.3.4.1) returns `TypeIdentifierWithSize`. The returned collection must use the SCCIdentifier for Strongly Connected Components. This begs the question, what is the size of the TypeObject corresponding to an SCCIdentifier?

Step (d) of the algorithm in 7.3.4.9.2 may imply that it is the serialized size of a sequence of TypeObjects of the components but there is nothing explicit.

Pg 48 7.2.2.4.6 first sentence

For each Aggergated Type "T" the type system defines a related [...]

-> Aggregated

Pg 48 7.2.2.4.6 first sentence

[...] obtainied from "T" by removing the key designation

-> obtained

Pg 50 7.2.2.4.7 first sentence

[...] obtainied from "T" as follows:

-> obtained

Pg 54 Figure 19 label at aggregation arrow to AnnotationParameter

+paramter_seq

-> +parameter_seq

Pg 133 Table 38 4th row column meaning

Conditionally swaps the bytes [...] matches the native Endianess

-> Endianness

Pg 146 continuation of 7.4.3.5.3 4th line from top

// FMMEBER can be NOPT_FMEMBER [...]

-> FMEMBER

Pg 215 Table 60 4th column, header

Endianess

-> Endianness

Pg 248 continuation of Annex A 2nd para from top

<xs:simpleType name="autoIdKind">
  <xs:restriction base="xs:string">
    <xs:enumeration value="hash"/>
      <xs:enumeration value="sequencial"/>

-> <xs:enumeration value="sequential"/>

Pg 263 continuation of Annex B bottom of page

// Flags that apply to type declarationa and DO affect assignability

-> declarations

Pg 266 continuation of Annex B 3rd para from top

// Used for Types that have cyclic depencencies with other types

-> dependencies

Pg 268 continuation of Annex B approx 10 lines down

// ============ Plain collectios - use TypeIdentifierKind =========

-> collections

The machine readable file machine readable file dds-xml_application_example.xml use some names that are not appropriate to the their context. For example it has a <data_reader> that has been named "MySquareWriter" this is likely a copy-paste error and should have been named "MySquareReader"

The spec states that @key members are always "must understand" does this mean that when represented in a TypeObject the member should also have the "must understand" flag set or is it enough with it being marked with the "key" flag as this would already imply its must understand nature?

Either way it should not impact assignability but it would impact the TypeId computation.

The type assignability rules for Enumerated Types require knowledge of whether the literal names should be considered for assignability.

Right now this can only be configured when the type is declared using the @ignore_literal_names annotation. However in some cases it may be necessary to change this behavior at run-time for a particular DataReader.

To support this use-case it would be helpful to add a ignore_enum_literal_names field to the TypeConsistencyEnforcement QosPolicy

Dear OMG Team,

we have found a security vulnerability in the DDS Security Specification [1] that we would like to disclose to your security contact directly.

Would you please send us the email address of the person in charge of handling security vulnerability for the mentioned specification?

Thank you and best regards from Germany,

FZI Security Research Team

[1] https://www.omg.org/spec/DDS-SECURITY

@value(8x08) GAP instead of 0x08

higuest_sent_seq_num is written four times instead of highest_sent_seq_num

higuest_sent_seq_num is written five times instead of highest_sent_seq_num

higuest_sent_seq_num is written five times instead of highest_sent_seq_num

this.higuest_sent_seq_num instead of this.highest_sent_seq_num

the figure erroneously uses the label "new DDS DataReader / delete Reader" instead of "new DDS DataReader / new Reader"

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).

2.2.3 (p. 96) The DataReader requests that liveliness of
the writers is maintained by the requested
means and loss of liveliness is detected with
delay not to exceed the lease_duration.
The DataWriter commits to signalling its
liveliness using the stated means at
intervals not to exceed the lease_duration.

Since both the DataReader and DataWriter have a lease duration, the reference to lease_duration is ambiguous.

2.2.3.11 (p. 107) Changes in LIVELINESS must be detected by the Service with a time-granularity greater or equal to the lease_duration. This
ensures that the value of the LivelinessChangedStatus is updated at least once during each lease_duration and the related
Listeners and WaitSets are notified within a lease_duration from the time the LIVELINESS changed.

Same problem, the reference to lease_duration could refer to the writer or the reader.

Other sections not mentioning the lease_duration may have similar problems.

The language around on_liveliness_changed implies that changes (listener, wait set) must be communicated at least once for every lease_duration of the DataReader.

The spec should clarify three things:
1. How often does the DataReader evaluate the liveliness of a DataWriter? Is this at a period of at most the DataReader's lease_duration or a period of at most the DataWriter's lease_duration?
2. When the DataReader evaluates the liveliness of a DataWriter, which lease_duration should it use, the lease_duration of the DataReader or the lease_duration of the DataWriter?
3. Are the rules for evaluating liveliness for the purpose of ownership the same as for listeners and waitsets? This question is particularly vexing because it can lead to inconsistency where ownership should be eventually consistent.

To illustrate the problem, consider the following a system using exclusive ownership.

• Writer 1 has a lease_duration of 1 second but only writes/asserts every 4 seconds. Ownership strength of 1.
• Writer 2 has a lease_duration of 1 second but only writes/asserts every 4 seconds. Ownership strength of 2.
• Reader 1 has a lease_duration of 2 seconds.
• Reader 2 has a lease_duration of 4 seconds.

Let events happen according to this sequence:

If we assume that the readers evaluate liveliness at a period of their own lease_duration using their own lease_duration, then Reader 2 will always see Writer 2 as the owner while Reader 1 will toggle ownership between the writers. Thus, there is no eventual consistency of exclusive ownership.

If we assume that the readers evaluate liveliness at a period of their own lease_duration using the lease_duration of the writer, then both readers will consistently toggle and never see a live writer.

If we assume that the readers evaluate liveliness independently for each writer at the lease_duration of the writer, then the readers will see ownership toggling but periods where writers are alive.

Ideally, liveliness would be objective and not subjective. That is, liveliness should be based on the behavior of the writer interpretted by its lease_duration. Different readers may observe different things leading to temporary inconsistency, but eventual consistency would be guaranteed.

If the spec choose a subjective approach, then language should be added that warns the reader of the problem and the consequences like ownership not working as expected.

When an annotation application definition has a simpgle paramater the application can omit the paramater name.

When an annotation application specifies multiple parameters it is possible to commit the parameter name for the parameter named "value" ,

This means that an annotation that was specified with a single parameter (not called value) which is then extended will need to be modified if a second paramater is added.

Instead it would be more flexible to say that the first paramater can be unnamed regardless of the name.

There are three problems.

1. The beginning of 7.3.1.2.1.11 says “Table 18. In some cases, this is not desirable. This default behavior may be modified using the @ignore_literal_names annotation.” It is unclear what exactly may not be desirable. Perhaps this was referring to the fact that enumerator names are used in the assignability check which indeed may be undesirable.

2. There is no way for `@ignore_literal_names` to be encoded in TypeObjects which means they are not used during discovery and cannot be used in assignability checks.

3. The text should clarify if this is an annotation that is only applied locally and/or applied to remotes.

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

Would be useful to have support for @optional for union members. As IDL exceptions can be used through DDS-RPC also add exception members to this table, very likely to the ones that also are supported by structures

In the spec this section reads:

7.2.1.1 Type Evolution Example

Assume a DDS-based distributed application has been developed that uses the Topic “Vehicle
Location” of type VehicleLocationType. The type VehiclePositionType itself was defined
using the following IDL:

(quote)
// Initial Version
struct VehicleLocationType {
(quote)

VehiclePositionType in the second sentence should be replaced with VehicleLocationType

The specification is not consistent regarding which elements can be annotated. Specifically regarding typedef declartions.

Section 7.2.2.6 "Annotations" says:
"An annotation describes a piece of metadata attached to a type or an element/member/literal of an aggregated/collection/enumerated type. Annotations can also be attached to the related_type of an alias type."

However
section 7.3.1.2.1 (Built-in Annotations) says:

In IDL, an annotation may be applied to any construct or sub-construct
(see Sub Clause 7.4.15.2, [IDL]). This specification restricts the applicability of annotations to constructed types, bitmask constants, enumerated type literals, and members of aggregated types.

I believe the intent all along was to allow annotations, such as @range and @unit in typedef. Also would be nice to align with whet IDL4 says.

At a minimum in XTYPES we should remove the specification of where annotations apply from section 7.3.1.2.1 and instead reference 7.2.2.6.

The specification does not state clearly if a writer and and reader that have types with incompatible values of @unit or @range should match?

Also it is not clear as if both a complete TypeObject and a minimal TypeObject should be sent or just one and how it is decided.

It would seem like.a good idea to allow application (DataReaders) to opt in or out of matching writers that have types with incompatible @range and/or @unit. If so mechanisms should be provided to configure this and make sure the complete TypeIdentifier is propagated in those cases as the corresponding TypeObject is the only one that has the information of @range and @unit.

Pg 43 7.2.2.4.4.4.3 Member Index 2nd sentence

[...] of the member within the Aggegated type.

Pg 47 7.2.2.4.5 Inheritance of Aggregated Types 1st sentence

The Type System supports single inheritance of Aggegated Types.

Pg 49 continuation of 7.2.2.4.6 comment before union Command_TypeErased

// The related KeyErased type definition only applies to aggegated types.

Pg 51 continuation of 7.2.2.4.7 comment before union Command__KeyHolder

// The related KeyHolder type definition only applies to aggegated types.

On this last occurrence, is the double underscore in union Command__KeyHolder intentional?

The Resolution Summary of DDSXTY13-23 states:

Structure types can inherit from other structure types as long as:

• [...]
• The derived type does not define any key fields.
  Is this really what we want to do. Will it break some definition where the "base type" is just being reused as a "header"?

I propose to change the first sentence to

• The derived type does not define any key fields if any of its ancestor types is non nested.

This restores viability for the mentioned use case where the base type(s) are just being used as header(s).
It hinges on the assumption that if all base types are nested then the question of assignability does not arise at the topic level.

Some JSON schemas contain lack certain restrictions that are enforced in strict validations.

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.

This is related to https://issues.omg.org/issues/DDSXTY14-13, and it might actually be covered by it, but I can't read into the comments at the moment to see if this specific issue was mentioned. As mentioned in 7.2.2.2.1, strings should be either UTF-8 or UTF-16. @try_construct(TRIM) can cause strings to be truncated. If they are truncated in the middle of a UTF-8 multi-byte code point or UTF-16 surrogate pair, then it would leave invalid data at the end of the string. Ideally an implementation could read trim the strings intelligently, but the spec should say what the behavior is.

As defined create_sample can only return the sample object. This doesn't allow the API to indicate that there was an error unlike other calls in DDS. Other operations either can't normally fail, return an interface that can be null, or return a ReturnCode_t. At least in the C++ Language Mapping, create_sample can return a struct or a union by value, so it can't be null in those cases (section 5.11). I imagine this might be different in other mappings though.

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'.