Skip to content

Commit

Permalink
Participants can sign their own permissions file
Browse files Browse the repository at this point in the history
Problem
-------

Assume a usage of DDS Security where the same CA is used for both permissions
and identity.  The certificates issued to particpants allow them to
sign documents.  Assume the participant generates a permissions file
and then signs it.  Chain verification causes verification attempts to
succeed since the signing certificate, i.e., the participant's
certificate, can be chained back to the permission CA's
certificate (which is also the identity CA).

This problem was identified in
ros2/sros2#282.

Solution
--------

Implement the suggestion in ros2/sros2#282.
Specifically, use PKCS7_NOINTERN to not accept any signatures in the
signed document.  This, in turn, requires the use of the `certs` parameter to
`PKCS7_verify`.  PKCS7_NOVERIFY is used since the permissions CA
certificate will not be chain verified.
  • Loading branch information
jrw972 committed Jan 18, 2023
1 parent 94f42bb commit 70410e8
Showing 1 changed file with 41 additions and 7 deletions.
48 changes: 41 additions & 7 deletions dds/DCPS/security/SSL/SignedDocument.cpp
Expand Up @@ -12,6 +12,7 @@
#include <dds/DCPS/Definitions.h>

#include <openssl/pem.h>
#include <openssl/x509.h>

#include <cstring>
#include <sstream>
Expand Down Expand Up @@ -91,6 +92,38 @@ bool SignedDocument::load(const std::string& uri, DDS::Security::SecurityExcepti
return true;
}

class StackOfX509 {
public:
StackOfX509()
: certs_(sk_X509_new_null())
{}

~StackOfX509()
{
if (certs_) {
sk_X509_free(certs_);
}
}

STACK_OF(X509)* certs() const { return certs_; }
operator bool() const {return certs_;}

bool push(const Certificate& certificate)
{
if (sk_X509_push(certs_, certificate.x509()) != 1) {
OPENDDS_SSL_LOG_ERR("sk_X509_push failed");
return false;
}

return true;
}

private:
// No copy.
StackOfX509(const StackOfX509&);
STACK_OF(X509)* certs_;
};

class X509Store {
public:
X509Store()
Expand Down Expand Up @@ -197,13 +230,14 @@ class PKCS7Doc {

operator bool() const { return doc_; }

bool verify(STACK_OF(X509)* certs,
const X509Store& store,
bool verify(const StackOfX509* certs,
const X509Store* store,
const Bio& indata,
const Bio& outdata,
int flags)
{
if (PKCS7_verify(doc_, certs, store.store(), indata.bio(), outdata.bio(), flags) != 1) {
if (PKCS7_verify(doc_, certs ? certs->certs() : 0, store ? store->store() : 0,
indata.bio(), outdata.bio(), flags) != 1) {
OPENDDS_SSL_LOG_ERR("SMIME_read_PKCS7 failed");
return false;
}
Expand All @@ -222,12 +256,12 @@ bool SignedDocument::verify(const Certificate& ca)
content_.clear();
verified_ = false;

X509Store store;
if (!store) {
StackOfX509 certs;
if (!certs) {
return false;
}

if (!store.add_cert(ca)) {
if (!certs.push(ca)) {
return false;
}

Expand All @@ -252,7 +286,7 @@ bool SignedDocument::verify(const Certificate& ca)
return false;
}

if (!doc.verify(0, store, bcont, content, PKCS7_TEXT)) {
if (!doc.verify(&certs, 0, bcont, content, PKCS7_TEXT | PKCS7_NOVERIFY | PKCS7_NOINTERN)) {
return false;
}

Expand Down

0 comments on commit 70410e8

Please sign in to comment.