Romascanu, Dan (Dan) wrote:
I have reviewed as shepherding AD draft-zorn-radius-pkmv1-04.txt
Following on Dan's review, I've reviewed the document for agreement
with the RADIUS design guidelines document [1]
Both the PKM-SS-Cert and PKM-CA-Cert attributes provide 'ad-hoc'
extension of the RADIUS attribute size, much like the EAP-Message
attribute. It would have been preferable to follow the extended
attribute format [2]. This provides a standardized way of carrying data
larger than a 253 bytes.
However, that document has not yet been published. My question is
that if there are no current implementations of the PKM specification,
it may be preferable to wait until the extended attributes document is
finished, and then to use that format.
Section 3.3 defines the PKM-Config-Settings. This is a "complex
attribute" within the meaning of Section 2.1.3 of the design guidelines
document, which states:
As a
result, the introduction of new complex data types within RADIUS
attribute specifications SHOULD be avoided, except in the case of
complex attributes involving authentication or security
functionality.
I do not see any pressing need to make this attribute carry seven
independent fields. The RADIUS attribute space has sufficient room to
allocate 7 attributes. If the extended attribute format is used, then
nearly all space limitations are removed. I would recommend that these
independent values be split up into independent attributes that follow
the RADIUS data model.
In Section 3.4. PKM-Cryptosuite-List, can the attribute be longer
than 253 bytes? If so, do the same ad-hoc rules as above apply? The
IEEE specification seems to permit attributes up to 32768 octets in length.
It would also be good to reference a specific page or section in the
IEEE spec, rather than just referring to the document as a whole.
Section 3.5. PKM-SAID, defines an attribute containing 2 octets of
data. It would be preferable to use a 4-octet type, and to specify that
the upper 2 octets are zero. This would allow the attribute to fit
within the existing RADIUS data model, as discussed in Section 2.1.1 of
the design guidelines document:
It is worth noting that since RADIUS only supports unsigned integers
of 32 or 64 bits, attributes using signed integer data types or
unsigned integer types of other sizes will require code changes, and
SHOULD be avoided.
Section 3.6. PKM-SA-Descriptor defines another complex attribute as
discussed above. It would be good to define this as a 64-bit integer,
which would fit within the RADIUS data model.
Section 3.7. PKM-AUTH-Key defines a complex attribute for
authentication or security, which fits within the design guidelines.
However, the encryption of the attribute is specified by reference to
[IEEE.802.16-2004]. This encryption is unlike anything previously used
in RADIUS, but it appears to be mandated by the IEEE specificiation.
It would also be good to reference a specific section of
[IEEE.802.16-2004] for this definition, rather than referencing the
document as a whole.
I also have some comments on the Security Considerations section:
If the Access-Accept message is not subject to strong integrity
protection, an attacker may be able to modify the contents of the
PKM-Auth-Key Attribute. For example, the Key field could be replaced
with a key known to the attacker.
Would it be sufficient to require that the Access-Accept contains a
Message-Authenticator for integrity protection?
Although it is necessary for a plaintext copy of the Key field in the
PKM-AUTH-Key Attribute to be transmitted in the Access-Accept
message, this document does not define a method for doing so
securely.
How is inter-operability to be achieved if there is no specification
for how to carry a necessary field?
I suggest changing the suggestion to use MS-MPPE-Send-Key into a
mandatory requirement, and making it part of the specification.
Alan DeKok.
[1] http://tools.ietf.org/html/draft-ietf-radext-design-07
[2] http://tools.ietf.org/html/draft-ietf-radext-extended-attributes-08
_______________________________________________
Ietf mailing list
Ietf(_at_)ietf(_dot_)org
https://www.ietf.org/mailman/listinfo/ietf