Clarify identity generated from CIDR-based policies and add security identity internal docs

[christarazi]

This is meant to be a starting point and not exhaustive from the outset.

Suggested-by: Joe Stringer joe@cilium.io
Suggested-by: Quentin Monnet quentin@isovalent.com
Signed-off-by: Chris Tarazi chris@isovalent.com

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[joestringer]

Thanks for documenting this. I think we can maybe wordsmith this a bit more to simplify the explanations, particularly around the size of the values and a few of the smaller assertions that are not consistent across the whole document. Couple of threads below for further discussion.

[joestringer]

I find this explanation a little bit confusing, because it attempts to describe what the ranges may denote, but on the way towards explaining all of the identity space, small assertions are made which are later contradicted.

For instance, the text The minimum security identity is ``256`` suggests that the allocation range for cluster-wide identities starts from 256. But the text itself says that the minimum security identity is 256, and then the very next sentence describes that there are security identities below this minimum.

The "there are two caveats" part also contradicts the uint16 storage assertion - if some of the system supports 32-bit identities then we can't outright declare that the security identity is a uint16, because some security identities are not uint16s.

I think that it's fairly safe to say that any arbitrary security identity will fall under the range 1 - 2^32-1. There are limitations to this, for instance in general any identity that may be shared across multiple nodes (either inside the cluster or between Cilium-managed clusters) must fit inside a VXLAN or Geneve tunnel endpoint header's virtual network field, which is 24 bits for both of these protocols. So in practice, when handing traffic between nodes, the value must fall within 0 - 2^24-1. In practice, we reserve the first 256 values and hence there may be additional special semantics for these identities. Then values above 2^24 cannot be shared directly between nodes via tunnel headers, so we avoid using these for consistent global identities; this is why the "local identities" set the lowest bit of the fourth byte.

[joestringer]

A few more minor nits 👍

[joestringer]

Do we need this to be a dedicated call-out box? I think that it's fine to just drop the note / indent and let the following paragraphs flow on from this, there's no reason to highlight the below information vs. the other information in this doc.

[joestringer]

^^ If we end up with documentation where half of the writing is in note:: sections and half isn't, then the way that the documentation UI presents the information to the user will only emphasize the information in the notes and discourage reading the non-note content. In the end this means that rather than structuring the information, we place unintended emphasis on some information.

I'll generally encourage attempting to write all docs without any sort of notes or callouts first, and only then when you have a full document, decide whether some of the information should be emphasized with note or warning sections.

[christarazi]

Hm good point, I didn't think of it like that. Good to know for the future 👍

[qmonnet]

Still some nits on formatting and phrasing.

I think the current version suffers a little from the successive changes we brought to the document. Re-reading the whole document, I would likely find it clearer if we didn't present clustermesh/CIDR identity scheme as “caveats”, but clearly introduced three contexts from start (cluster-local/clustermesh/CIDR).

In addition to the inline observations below, I tried to reword the beginning of the document. Please feel free to reuse (or drop) any portion of the suggestion below.

Security identities are generated from labels. They are stored as ``uint32``,
which means the maximum limit for a security identity is ``2^32 - 1``. The
minimum security identity is ``1``.

.. note::

   Identitiy 0 is not a valid value. If it shows up in Hubble output, this
   means the identity was not found. In the eBPF datapath, it has a special
   role where it denotes "any identity", i.e. as a wildcard allow in policy
   map.

Security identities span over several ranges, depending on the context:

1) Cluster-local
2) ClusterMesh
3) Identities generated from CIDR-based policies

Cluster-local identities (1) range from ``1`` to ``2^16 - 1``. The lowest
values, from from ``1`` to ``255``, correspond to the reserved identity range.
See the `internal code documentation
<https://pkg.go.dev/github.com/cilium/cilium/pkg/identity#NumericIdentity>`__
for details.

For ClusterMesh (2), 8 bits are used as the ``cluster-id`` which identifies the
cluster in the ClusterMesh, into the 3rd octet as shown by ``0x00FF0000``. This
does not apply to CIDR identities however, see (3).

CIDR identities (3) are local to each node. CIDR identities begin from ``1``
and end at ``16777215``, however since they're shifted by ``24``, this makes
their effective range ``1 | (1 << 24)`` to ``16777215 | (1 << 24)`` or from
``16777217`` to ``33554431``.

When CIDR policies are applied...

[christarazi]

Thanks @qmonnet, much cleaner now! I took basically all your suggestions, with very minor wording changes.

[joestringer]

👍 Doc looks to be in good shape now, I think it helped a lot to do another overall proof read of the whole doc and shuffle things around to communicate the same information but in a more structured, consistent way.

[joestringer]

I'm a little bit confused what this last sentence means. The previous paragraph already defined that the range starts at 2^24 (and the next paragraph also reiterates the allocation range). What does it mean for the identities to be stored separately inside the agent?

[christarazi]

👍 I've removed this sentence, as it's not adding value. Probably a remnant after all the changes. I also merged it with the previous paragraph.

[joestringer]

super minor nit, Another more generic way to phrase the last sentence is to define that ClusterMesh identities must set the uppermost byte to 0. Then it's clear that they are distinct from the local identities below, and if we ever decide to allocate meaning to the other uppermost bits, no meaning would change.

[christarazi]

Addressed.

[qmonnet]

All good for me this time, thanks a lot!