[secdir] Re: Secdir early review of draft-ietf-scim-device-model-05

[Eliot Lear <[e-mail]>]

Hi Mike,

This is part two in answer to your earlier review.  I want to again thank you for your effort.  All edits can now be found in:

https://github.com/iot-onboarding/scim-devices/pull/37

Can everyone take a good stare and see if we’re in the right ball park?

Please now see details below.

> On 2 Jul 2024, at 18:42, Mike Ounsworth via Datatracker <[e-mail]> wrote:
> 
> 2. My comment on "Section 9 Security Considerations" really also sumarizes my
> view on the document as a whole:

I have completely rewritten the security considerations section to discuss the type of operations.  It doesn’t go so far as to discuss each object, and so for the moment we have what amounts to coarse access control along the following philosophy:

1.  If the SCIM server grants access, it is granting the device access to the infrastructure.
2.  If the client provides credential information, it is granting access to the device, to one extent or another (depending on device type)
3.  Clients should only be able to manipulate objects they create.
4.  Even if a client deletes an object, the server may retain access to the device.  DELETE is merely an advisory signal in this case.


> 
> Section 6.3
> 
> I think this section in particular has serious issues.

And so it did.  It’s getting an upgrade.  The intent is as follows:

Either the SCIM client is handed a client token or the client provides a root cert and a name, we think a DNS name by which it will authenticate.  We’ll post an update about that.  And yeah, we’ll do away with CNs everywhere.  And we’ll put some nasty words around tokens as well.
 

> 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?

Done.

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

There is a generic design goal which is provide various types of device access.  We’ll make that clearer.


> 
> 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?

Fixed.

> 
> 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 …"

This is about the network rather than the device.  The wording was poor and is corrected.

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

See the next update in Security Considerations.

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

It says base64.

Eliot