Subject: [secdir] Secdir early review of draft-ietf-scim-device-model-05
From: Mike Ounsworth via Datatracker <[email-masked]>
To: <[email-masked]>
Cc: [email-masked], [email-masked]
Date: Tue, 02 Jul 2024 09:42:56 -0700
List-Id: Security Area Directorate <secdir.ietf.org>

Reviewer: Mike Ounsworth
Review result: Not Ready

I have performed an early review of this document as requested

by the SCIM WG chairs. These comments were written primarily for the benefit of
the

document authors, chairs, and security area directors.

The summary of the review is "Not ready".

Review summary:

1. This document does not describe the intended purpose of this data -- ie why
is this data being stored centrally in a SCIM database? Will devices be pulling
their config from the sCIM server? Is the SCIM data merely a copy of device
config for audit purposes so that network admins know what is on their network?
Or something in between; such as a VPN gateway using SCIM data to make access
control decisions? Without this context knowledge, it is quite difficult to
evaluate the security implications of this draft.

2. My comment on "Section 9 Security Considerations" really also sumarizes my
view on the document as a whole:

This is besically saying that yes, there are huge security and privacy
implications to this draft, but we leave them all at the discretion of
implementors. I think we can do better. Maybe this security considerations
section can't go as far as normative MUSTs, but it should definitely have
sub-sections on the various types of security and privacy issues that need to
be considered, with SHOULD or RECOMMENDED security and privacy controls that
are expected to be placed around this SCIM data. IE I think I am asking you, as
the authors, to lay out a recommended access control framework that is
appropriate for the sensitivity of the data described in this schema. Thinking
off the top of my head here, maybe each item in the JSON Schemas in section 8
should be tagged with "sensitivity" which can be either empty, "read-sensitive"
(for values like BLE pairing keys which should require elevated privileges to
read (or write?)), "write-sensitive" (for values like BLE Pairing Method which
are ok to read, but should require elevated privileges to change), or both.

Since I am not at all expert on SCIM, I recommend that a different secdir
reviewer be assigned for the final review, but I'm happy to continue as
reviewer until the next document milestone.

General comment:

In the actual JSON Schema in section 8, you list the type of each object:
String, Boolean, Complex, etc.

Question 1: I'm not familiar with the JSON datatype "complex". I am familiar
with "object" and "array". Is "complex" meant to be one of those? Reference:
https://www.w3schools.com/js/js_json_datatypes.asp

Section 6.3

I think this section in particular has serious issues. It's not clear to my
what security goal this section is trying to accomplish, so I'm judging it
based on what I think it's trying to accomplish, but that may not be right.
Starting this section with a statement of the design / security goal could help.

The certificate naming information seems under-specified, which could lead to
device impersonation attacks.

What are the types here. I assume String? This should be stated clearly so that
people don't try to place here a DER-encoded DistinguishedName, or
RelativeDistinguishedName (which is the actual defition of CN in RFC 5280).

Lower down in the section, it says:

"Note that attributes clientToken and certificateInfo are used for the
authentication of the application."

That is a pretty important note, and means that this entire section is a
security mechanism.

rootCN

Why only the CN and not the entire DN? Or better yet, the public key and forget
about the friendly-name entirely?

If this is intended for security, then I don't think that rootCN is a good
choice; as an attacker, once I know the CN of the trusted root CA, I can
trivially create a new root CA with the same CN using openssl. The important
part of a Trust Anchor is the cryptographic part -- the public key -- not the
name. A better choice would be the hash of root cert (ie cert thumbprint),
public key, hash of public key, etc. You could, for example, consider building
a JSON Crust Anchor structure inspired by rfc5914.

The only way I could see this making sense is if you assuming that the rootCN
will be used as an index into a trust store held by the application to find the
actual root cert. If so, that needs to be stated clearly in normative language
because that is a very important part of the trust and security model. You'll
also need to address how the application should behave if the trust store has
two roots with the same CN (a problem which is avoided if you use cert
thumprint or public key -- by definition you can't have a collision because two
CAs with the same cert / public key are by definition the same CA).

I think the goals of this need to be a lot more clearly documented. I would be
happy to help you iron this out.

subjectName

I find this section slightly confusing. RFC 5280 (X.509) defines "Subject" as
the entire DN of the certificate, which is often informally refered to as a
"subject name". However here you seem to mean only the CN part.

subjectAlternativeName

I don't think you can flatten SAN to a simple text value without introducing
vulnerabilities. You can get away with that in TLS because a TLS libary is only
checking dNSName and iPAddress SAN types, whereas an email client will only
check rfc82Name SANs. Here, I think you need to allow the onboarding app to
specify both the expected SAN text value and which SAN type it should be found
in.

Consider, for example, a human user with the network login username "ounswm1"
-- they migh get a certificate with SAN "otherName: username: "ounswm1"", while
a machine with a hostname of "ounswm1" would get a cert with SAN "dNSName:
mounswm1". Despite having the same textual value, they ARE NOT referring to the
same entity. To avoid authentication vulnerabilities, the certificate parser
needs to know whether the value "ounswm1" is supposed to be found in a DNS name
SAN, email address SAN, otherName SAN, etc.

RFC 5280 allows 9 types of SANs:

   GeneralName ::= CHOICE {

        otherName                       [0]     OtherName,

        rfc822Name                      [1]     IA5String,

        dNSName                         [2]     IA5String,

        x400Address                     [3]     ORAddress,

        directoryName                   [4]     Name,

        ediPartyName                    [5]     EDIPartyName,

        uniformResourceIdentifier       [6]     IA5String,

        iPAddress                       [7]     OCTET STRING,

        registeredID                    [8]     OBJECT IDENTIFIER }

Are you allowing all of them? Are you allowing the onboarding app to specify
which which SAN type a given identifier should be found in?

7

Again, I think this section would benefit from a statement of the design goal.

Is the SCIM database intended to be the source of truth for configuration of
the device? IE is the BLE device expected to reach out to the SCIM database to
pull its own config? (I suspect not since some of this config data is required
in order to successfully establish a network connection). So what is the
purpose of holding this data in the SCIM database? Is it for relying parties to
make security decisions about the device (ex.: ALLOW / DENY network access?).
Is it merely for network admins to inventory what's on their network and do
debugging? I think that knowing the intended purpose of this data will affect
how I read between the lines for security implications. For example, if SCIM is
a read-only copy of device config, then there are not really any security
concerns, but on the other hand, if write access to SCIM allows me to set BLE
pairing keys, then that places a huge trust relationship on the SCIM
infrastructure.

7.1.1

Editorial nit:

"irk

  ... It is required when addressType is TRUE."

A CTRL+F shows that `addresstype` is not defined in this document. What is it?
Can you add a link or reference to its definition?

Another editorial nit:

"mobility

  A boolean attribute to enable mobility on BLE device. If set to True, the BLE
  device will automatically connect to the closest AP."

Technically here we're talking about attributes set in the SCIM database,
right? I'm guessing that most BLE devices don't know or care what same SCIM DB
thinks its local config is. I'm nitpicking about the implication that changing
this value from TRUE to FALSE in the SCIM DB will cause the device to change
its behaviour. I'm guessing that what you mean is "a boolean attribute that
indicates whether the BLE devices is configured to ..."

7.1.3 BLE Pairing Method Extensions

Here we have detailed information about device pairing config, potentially
including pairing keys (pins). My comment above about whether this is READ-ONLY
or READ-WRITE applies. If write-access to SCIM allows SCIM admins to set
devices to `pairingNull` or `pairingJustWorks`, that would have large security
implications that need to be documented. Even if it's READ-ONLY, knowing which
devices on your network are configured for `pairingNull` or `pairingJustWorks`
tells an attacker the easy targets, and learing pairing keys could also be bad.
One mantra of security / privacy is "If you don't need it, then don't collect
it". Given that this document doesn't state the purpose of collecting this
data, what is the purpose of storing every device's pairing keys in a central
server? It seems like a beautiful target for hackers; is there a legitimate
purpose to holding that data?Does SCIM have a concept of access controls? Can
this data be flagged as SENSITIVE and only viewable by users with elevated
priviledge?

7.2.1

bootstrapKey

Editorial nit:

"A string value representing Elliptic-Curve Diffie–Hellman (ECDH) public key."

How is this encoded? I assume Base64 or PEM? There are a few different flavours
of Base64 around, so I would expect this to say "... MUST be encoded in Base64
as per RFCxxxx", or "... SHOULD be encoded in Base64 as per RFCxxxx, but MAY be
encoded in other flavours of Base64", or something similar.

9 Security Considerations

This is besically saying that yes, there are huge security and privacy
implications to this draft, but we leave them all at the discretion of
implementors. I think we can do better. Maybe this security considerations
section can't go as far as normative MUSTs, but it should definitely have
sub-sections on the various types of security and privacy issues that need to
be considered, with SHOULD or RECOMMENDED security and privacy controls that
are expected to be placed around this SCIM data. IE I think I am asking you, as
the authors, to lay out a recommended access control framework that is
appropriate for the sensitivity of the data described in this schema. Thinking
off the top of my head here, maybe each item in the JSON Schemas in section 8
should be tagged with "sensitivity" which can be either empty, "read-sensitive"
(for values like BLE pairing keys which should require elevated privileges to
read (or write?)), "write-sensitive" (for values like BLE Pairing Method which
are ok to read, but should require elevated privileges to change), or both.