Ambiguous key-ids in keychain
rgwilton opened this issue · 9 comments
The keychain model has been updated with the key-id being a union:
grouping keychain-key-config {
description "This grouping defines key-chain key parameters.";
leaf key-id {
type union {
type oc-yang:hex-string {
length "1..64";
}
type uint64;
}
description
"Identifier for the key within the keychain.";
}
The problem with this is that for some values (e.g., a hex value that happens to contain no letters), and for some encodings then it will be ambiguous as to whether the key-id is meant to represent a hex string or an integer. E.g., with both the XML encoding and IETF YANG JSON encoding, these will both be encoded as strings. I.e., all keys will look to the server like they are hex strings.
Any implementations that treat MAC Sec keys (hex strings) differently from other integer based ids for keychains will likely have problems mapping this.
Options to resolve include:
- only use uint64 as a unique identifier for items in the list
- Change the hex-string type to something that can be differentiated from uint64, such as delimited with ":" as implemented by the IETF hex string type.
From community input, it seems there is preference to include the hex-string with a format change.
I move that we change the format of a hex-string to be a string delimited with : in between each byte, in the same style as the IETF https://datatracker.ietf.org/doc/rfc6991/ hex string type.
@m26singhvi Can you explain the motivation for #737 ? Creating a union of uint64 and hex-string raises this issue of ambiguity between the types. It is also undesirable to use a union type for this reference when attempting to map the key to some underlying implementation. I am wondering what the impact is if we were to change back to only a uint64?
@dplore - As captured in #733, this was done to primarily support Macsec model. Changing it back to uint64 would mean that Macsec model will need it's own keychain model, which is how it was to begin with. It was changed to leverage the global keychain model.
Can you provide some insight on why the key needs to be a mac address? I suspect it's for some reason that is obvious for mac-sec, but I am not familiar with mac-sec key requirements. Is a mac-address needed as part of identifying a mac-sec key? If true, would it make sense to make mac-address part of the key-chain configuration?
FOr example today we have:
module: openconfig-keychain
+--rw keychains
+--rw keychain* [name]
+--rw name -> ../config/name
+--rw config
| +--rw name? string
| +--rw tolerance? union
+--ro state
| +--ro name? string
| +--ro tolerance? union
+--rw keys
+--rw key* [key-id]
+--rw key-id -> ../config/key-id
+--rw config
| +--rw key-id? union <-- hex-key or uint64
| +--rw secret-key? string
| +--rw crypto-algorithm? identityref
+--ro state
| +--ro key-id? union
| +--ro secret-key? string
| +--ro crypto-algorithm? identityref
+--rw send-lifetime
| +--rw config
| | +--rw start-time? oc-types:timeticks64
| | +--rw end-time? oc-types:timeticks64
| | +--rw send-and-receive? boolean
| +--ro state
| +--ro start-time? oc-types:timeticks64
| +--ro end-time? oc-types:timeticks64
| +--ro send-and-receive? boolean
+--rw receive-lifetime
+--rw config
| +--rw start-time? oc-types:timeticks64
| +--rw end-time? oc-types:timeticks64
+--ro state
+--ro start-time? oc-types:timeticks64
+--ro end-time? oc-types:timeticks64
Making mac-sec part of the key configuration might look like the below. The purpose of reorganizing this removes the union as the key which simplifies the model and makes it possible to map OpenConfig to multiple underlying system implementations.
module: openconfig-keychain
+--rw keychains
+--rw keychain* [name]
+--rw name -> ../config/name
+--rw config
| +--rw name? string
| +--rw tolerance? union
+--ro state
| +--ro name? string
| +--ro tolerance? union
+--rw keys
+--rw key* [key-id]
+--rw key-id -> ../config/key-id
+--rw config
| +--rw key-id? unit64 <-- changed to uint64 only, no longer a union type
| +--rw secret-key? string
| +--rw crypto-algorithm? identityref
+--ro state
| +--ro key-id? union
| +--ro secret-key? string
| +--ro crypto-algorithm? identityref
+--rw macsec-parameters <-- new container for macsec specific parameters
| +--rw config
| | +--rw mac-address? oc-types:mac-address
| +--ro state
| | +--ro mac-address? oc-types:mac-address
+--rw send-lifetime
| +--rw config
| | +--rw start-time? oc-types:timeticks64
| | +--rw end-time? oc-types:timeticks64
| | +--rw send-and-receive? boolean
| +--ro state
| +--ro start-time? oc-types:timeticks64
| +--ro end-time? oc-types:timeticks64
| +--ro send-and-receive? boolean
+--rw receive-lifetime
+--rw config
| +--rw start-time? oc-types:timeticks64
| +--rw end-time? oc-types:timeticks64
+--ro state
+--ro start-time? oc-types:timeticks64
+--ro end-time? oc-types:timeticks64
Question - Can you provide some insight on why the key needs to be a mac address?
Answer - Sorry but I don't think I mentioned that it needs to be a mac address.
Context :
As per 802.1X-2010, for Macsec Key Agreement ( MKA ) protocol needs a key and key identifier to work among other things. key is usually referred as CAK and key identifier is CKN which is a hex string.
Since keychain model also had a key identifier, I had proposed it to be a union of hex string and U64 to make it work for Macsec.
It appears hex-string type is used in 5 places.
- 2 in macsec
- 1 in keychain
- 1 optional-capabilities in ospf
- 1 evpn esi type
I'd appreciate any other thoughts on changing the hex-string type to be a ":" delimited hexstring, in the same style as the IETF https://datatracker.ietf.org/doc/rfc6991/ hex string type.
Also any comments on whether this should be a breaking or non-breaking change. I am leaning towards making this a breaking change because of the limited scope of the use of the hex-string type and the next OC release will be 3.0 as it will include #992 which was a breaking change.
Sounds like this is the only feasible solution to prevent any such future usages. Please note that the latest macsec model doesn't have any hex-string usage, so the scope is even lesser.