-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for RADIUS TLS-PSK #108
Add support for RADIUS TLS-PSK #108
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution, @ghalse .
Minor comments I've pointed out in the code - otherwise looks well designed.
Shouldn't be too much to address my concerns.
Cheers,
Vlad
@@ -152,6 +152,8 @@ NRO_PROV_SOCIAL_MEDIA_CONTACT = [ | |||
|
|||
# Helpdesk, used in base.html: | |||
NRO_DOMAIN_HELPDESK_DICT = {"name": _ld({'en': "Domain Helpdesk"}), 'email':'helpdesk@example.com', 'phone': '12324567890', 'uri': 'helpdesk.example.com'} | |||
# ream used to generate a TLS-PSK identity for service providers | |||
NRO_TLSPSK_REALM = "example.com" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How come the default values for NRO_TLSPSK_REALM
is different in settings.py
than it is here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Razorfang , this is a change @ghalse did on my suggestion.
The value in settings.py is an intentionally invalid value that is used only when local_settings.py does not provide a value for NRO_TLSPSK_REALM
and is there to prevent the code from breaking.
This is a value to be customised by local deployments. Existing deployments that do not update local_settings.py will get the clearly invalid value.
As the two values server different purposes, it makes sense to me to have them different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, as @vladimir-mencl-eresearch suggests, they're intentionally different. The local_settings.py.dist file contains example values and uses the same example domain as the rest of the file (and fortunately that's already a documentation domain). The settings.py file uses an intentionally invalid value and uses a domain reserved for that purpose. Both are consistent with RFC2606.
With the advent of Blast-RADIUS there have been strong moves towards using RADIUS over TLS, and increased vendor support. The traditional mutual certificate auth method is hard to configure, but RADIUS TLS-PSK is a promising alternative. It is already supported by both FreeRADIUS and radsecproxy.
To allow institutions to configure RADSEC TLS-PKS, DjNRO needs to be extended to accommodate a new RADIUS type and to configure a TLS PSK identity and a TLS PSK key. This PR provides for that.
There are some assumptions in the implementation:
TLS PSK will use RADIUS-TLS
In order to avoid extending the
proto
field from 12 characters, I've opted to use an identifier ofradius-psk
inRADPROTOS
. This is technically ambiguous because there should be options forradius-tls-psk
andradius-dtls-psk
in RADPROTOS. However, DTLS doesn't seem widely implemented and is commented out in DjNRO's stock config. I think it's safe to assume this will be RADIUS-TLS rather than DTLS. Of failing that, that NROs will support one or the other.PSK identity for the local server
The local FLR server's PSK identity is not user-configurable. Instead, it is generated as a Network Access Identifier from the institution's UUID and a pre-defined realm configured in local_settings.py (new
NRO_TLSPSK_REALM
setting).This creates an opaque token in the user portion, as contemplated by 4.2 of draft-ietf-radext-tls-psk. An example identifier is:
The identity is exposed in the user interface, and in the servdata YAML. It can also be constructed from the database if needs be.
PSK for a remote server
The remote IdP's PSK identity can be configured and is simply validated as a Network Access Identifier.
Shared pre-shared key
In theory, the IdP and SP connections can be configured completely independently. This means that a pair of servers can have a different RADIUS secret and/or a different PSK key for each direction.
However, DjNRO already assumes that both directions will share a common RADIUS secret. This also seems to an assumption made by many server admins. To avoid having to add two separate fields for PSK keys, this implementation extends that assumption by assuming that both directions will share a common TLS pre-shared key.
This isn't strictly in accordance with draft-ietf-radext-tls-psk, but it's a reasonable compromise between an ideal implementation and practical constraints. It will probably also reduce confusion.
Our own implementation of this extends things by having the initial PSK key pre-generate as a strong random string, rather than relying on the admin to supply one.