Chain of trust issues with a single CA certificate

amrc-benmorrow commented 1 year ago

Currently the security keystores created by ros2 security use a single CA symlinked as both Identity CA and Permissions CA. This opens a security hole where a malicious node can sign its own permissions document:

The node creates a new permissions.xml and signs it with its own enclave certificate and private key.
The node publishes the signed document over DDS as usual.
Other nodes attempt to verify the signature; since the enclave certificate is signed by the Identity CA, and the Identity CA is the same as the Permissions CA, the signature is accepted.

I don't believe it is possible to work around this with certificate flags; the enclave certificate must have the digitalSignature flag in order to be able to participate in Secure DDS. The solution is to separate the two CA roles into different certificates.

amrc-benmorrow commented 1 year ago

These are the slides from the meeting. ROS2-security-wg-presentation.pptx

amrc-benmorrow commented 1 year ago

This is a shell script that demonstrates the problem. The script is set up to use Foxy because that is what we use internally but the problem still exists in Rolling.

Run the script in an empty directory. It will:

Create a new keystore.
Create /talker and /listener enclaves.
Create ACLs which allow /talker to publish but do not allow /listener to subscribe.
Run the nodes to show that the permissions are respected.
Acting solely as a malicious /listener node, modify and re-sign permissions.xml to allow subscribing.
Run the nodes again to show that now the listener node has permission to subscribe.

Shell script. (This is a .txt file because Github won't accept .sh.)

amrc-benmorrow commented 1 year ago

@ruffsl asked for a mention.

ruffsl commented 1 year ago

Hey @amrc-benmorrow , thanks for opening the ticket! So I looked into this a bit deeper, and what I suspect is that this issue has less to do with sros2 than it does with DDS vendor compliance.

Specification

I’ll start first by citing the current DDS Security Specification, where the following section excerpts here are of particular relevance:

9.4.1.1 Permissions CA Certificate

This is an X.509 certificate that contains the Public Key of the CA that will be used to sign the Domain Governance and Domain Permissions document. The certificate can be self-signed or signed by some other CA. Regardless of this the Public Key in the Certificate shall be trusted to sign the aforementioned Governance and Permissions documents (see 9.4.1.2 and 9.4.1.3).
9.3.1.1 Identity CA Certificate

The certificate shall be the X.509 v3 Certificate [39] of the issuer of the Identity Certificates in section 9.3.1.3. The certificate can be self-signed if it is a root CA or signed by some other CA public key if it is a subordinate CA. Regardless of this the Public Key in the Certificate shall be accepted as the one for the Identity CA trusted to sign DomainParticipant Identity Certificates, see 9.3.1.3.

From section 9.4.1.1, we can gather that the chosen Permissions CA is to be used to sign Domain Governance and Domain Permissions documents, and is not delegatable. Consequently, it appears the specification leaves no room for the signing of such documents to be delegated beyond that certificate authority, regardless of any chains of trust in public certificates.

Similarly, from section 9.3.1.1, we can also gather that the chosen Identity CA is to be used to issue Identity Certificates, and is not delegatable. Consequently, it again appears the specification leaves no room for the issuing of such certificates to be delegated beyond that certificate authority, regardless of any chains of trust in public certificates.

Under the current specification, we can conclude that combining the roles of such Permissions and Identity into a single CA is admissible for at least two reasons. First, that the role of either CA is never defined as being mutually exclusive. Although admittedly the omission of a "shall not" clause remains a rather weak argument in the contexts of complex protocol specifications, it can also be justified by the explicit disregard of how either CA certificate is itself signed.

As a counter example, let's assume the following axioms about the specification are true:

the issuer of both Permissions and Identity CA certificate is not disregarded
the delegation of both Permissions and Identity CA signature authority is admissible
only the Permissions CA can be used to validate Governance and Permissions documents
only the Identity CA can be used to issue Identity Certificates

Consequently, validating delegated signatures would inherently necessitate validating a chain of trust for issued certificates. However, given the specification does not restrict the issuing identities of either the Permissions or Identity CA, we can assume the chain of trust for such issuing identities to be arbitrary. Without loss of generality, we can assume such issuing identities to be a single self-signed root level CA, as would be common among corporate infrastructures.

However, the mere possibility of this permissible chain of trust subsequently contradicts our last two axioms, given that such a trusted root level issuer CA would allow valid chain of trust to be built when validating either Identity Certificates issued by the Permissions CA, or when validating delegated Identity CA signatures embedded in Governance and Permissions documents.

Thus it stands to reason that 1) the delegation of the Permissions and Identity CA role signature authority remains inadmissible, while 2) the validity of Permissions and Identity CA signatures is evaluated regardless of the issuer of such CA certificates.

Compliance

At present, I believe the root cause of the discussed chain of trust exploit stems from a non-compliant implementation of permission document verification used by some DDS vendors. Specifically, an improper use of the OpenSSL PKCS7_verify function used to validate S/MIME signatures.

Consulting the OpenSSL documentation for PKCS7_verify:

OpenSSL v3.0 ships with Ubuntu 22.04 LTS:
- https://www.openssl.org/docs/man3.0/man3/PKCS7_verify.html

We can see this function takes 6 arguments, including 1) the PKCS#7 signedData structure to be verified, 2) the set of certificates in which to search for signer's certificates, 3) x509 store that may be NULL or point to the trusted certificate store to use for chain verification, 4) input buffer for signed data in question if content is detached, 5) output buffer of signed content read, and 6) optional set of flags used to modify verification behavior.

For reference, we can see the use of this function in both Fast-DDS and Cyclonedds:

Fast-DDS v2.6.2 ships with ROS 2 Humble:
- https://github.com/eProsima/Fast-DDS/blob/2.6.2/src/cpp/security/accesscontrol/Permissions.cpp#L518
Cyclonedds v0.9.1 ships with ROS 2 Humble:
- https://github.com/eclipse-cyclonedds/cyclonedds/blob/0.9.1/src/security/builtin_plugins/access_control/src/access_control_utils.c#L251

Note that for these DDS implementations: while the set of certificates in which to search for signer's certificates is set to null, only the store of trusted certificates to use for chain verification is provided, with just the PKCS7_TEXT flag being set. Thus, the resulting verification is prone to admissible delegation of role signature authority given the inclusion of the CA within the trusted certificate store allows for any certificate issued by that CA to pass the signature verification by proxy, due to the nature of issuing-certificate chain of trust. Furthermore, this chain of trust could then also be extended, given p7 may contain extra untrusted CA certificates that may be used for chain building.

Solution

To correct existing vendor implementations and remain in compliance with the DDS Security Specification, I would recommend the following:

Instead of including the Permission CA into the store of trusted certificates to use for chain verification, the Permission CA (and only the Permission CA) should be included in the set of certificates in which to search for signer's certificates. The store of trusted certificates to use for chain verification should then also be set to null.
With the store of trusted certificates to use for chain verification set to null, the PKCS7_NOVERIFY flag should then be enabled so any signer's certificates (i.e. only the Permission CA) is not chain verified.
Given that only a valid signer's certificate must be the Permission CA, the PKCS7_NOINTERN flag should then be enabled so any set of certificates in the message itself are not searched when locating the signer's certificates.

However, it is still unclear if this change would introduce other side effects. E.g. My guess is that verification of signatures using the chain of trust primitives also inherently checks the period of validity for each certificate used in such a search with regards to the current date and system time. With the use of PKCS7_NOVERIFY and thus no chain verification, I’m unsure if the period of certificates would still be validated. However, given the specification disregards the issuer for such Identity and Permissions CAs, and that they are allowed to be self signed, the period of validity for the public certificate of such CAs may be well inconsequential as far as the specification is concerned.

Testing

Upon testing the following DDS vendor implementations supported by ROS 2 using the provided minimal viable example, the table below depicts which implementations are susceptible to the discussed chain of trust exploit:

DDS Vendor	Vulnerable?
Fast-DDS v2.6.2	Yes
Cyclonedds v0.9.1	Yes
Connext dds v6.1.1	No

Future work

It may be worth auditing how such x509 trust stores are constructed, and where else they are used in existing DDS implementations. E.g. could the same trust store also be used for establishing a chain of trust in validating identity certificates, or could the Permission CA be inadvertently included in another x509 trust store, again defeating any separation of concerns between CA roles, even when separate CAs are intentionally deployed?

ros-discourse commented 1 year ago

This issue has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2023-01-19/29423/1

ros2 / sros2