Janfred / draft-rieckers-radext-rfc6614bis

Other
1 stars 1 forks source link

Peer validation #3

Open khuhtanen opened 1 year ago

khuhtanen commented 1 year ago

From draft:

Peer validation always includes a check on whether the locally configured expected expected DNS name or IP address of the server that is contacted matches its presendet certificate. 3{:jf} DNS names and IP addresses can be contained in the Common Name (CN) or subjectAltName entries. For verification, only one of these entries is to be considered. The following precedence applies: for DNS name validation, subjectAltName:DNS has precedence over CN; for IP address validation, subjectAltName:iPAddr has precedence over CN. Implementors of this specification are advised to read {{?RFC6125}}, Section 6, for more details on DNS name validation.

I think this should be defined more carefully. We do not want this to be interpreted so that server should always match client's IP address and DNS name to its certificate. Actually this kind of matching should not be in my opinion required since we may for example rely to the fact that the client/peer/server has a certificate from approved CA and we do not really care about the servers DNS name or its IP address -- only that it has a certificate certified by approved CA.

khuhtanen commented 1 year ago

I think these are also related:

Support for certificate-based mutual authentication is REQUIRED.1{:jf} Negotiation of mutual authentication is REQUIRED.2{:jf}

I think the first comment (1) might be about server accepting clients without checking client certificate. In Radiator we support this functionality and it could be useful for example in the trusted networks, where you can for example accept certain client IP network block to connect just by checking or not checking the server certificate.

restena-sw commented 1 year ago

Well (2) is only about the technicality of the TLS handshake: during the handshake, there is always a server and a client certificate. Situations like typically found in web contexts (server certificate, but no client certificate, and thus no control of client identity) are to be avoided. I think this makes sense and should stay - in a RADIUS environment, clients should not be allowed to remain anonymous.

(1) is only about support for TLS with server/client certificates, not about actually checking the certificates. I believe this also should stay; we want PKIX-style client+server auth to be a supported mode of operation.

The only real question is whether the checks are allowed to be turned off. That's a policy decision by the deployer much more than a technical specification. I understand that the phrase "Peer validation always includes ..." reads like a MUST; which it arguably shouldn't.

Maybe it is better to speak of a list of things a peer CAN look at to verify the other end's identity; and to add that when communicating outside trusted networks one SHOULD verify the identity (or can we even say MUST?).

Strawman text:

Peers MUST validate the identity of the respective other end of the connection, unless both ends are inside the same trusted network.

alandekok commented 1 year ago

In order to support multiple clients behind a NAT, it should be possible to do peer validation only on the client certificate. Or other TLS parameters, such as TLS-PSK, etc.

It is useful to validate the source IP (or IP range) if you know that there's no NAT. Doing so can help to discover misconfigurations or unauthorized network changes.

I find it less useful for a server to check hostnames in client certs. DNS offers little security, and we shouldn't rely on it for security checks. Checking hostnames is more useful for client to server connections, because of RFC 7585.

I would also suggest that the peer always has to be validated, even if both ends are in the same trusted network. There may be a network change or misconfiguration which "opens up" the network. We don't want to have a domino effect of one security issue allowing an attacker to exploit another one.

Stefan's text looks mostly pretty good.

khuhtanen commented 1 year ago

I also think Stefan's suggestion is already pretty good, but I would like it to have a more clear definition of the OpenRoaming use case:

The paragraph:

TLS Servers validate the incoming certificate against a local database of acceptable clients. The database may enumerate acceptable clients either by IP address or by a name component in the certificate. for clients configured by name, the configured name is matched against the presented names from the subjectAltName:DNS extension; if no such exist, against the presented CN component of the certificate Subject for clients configured by their source IP addresses, the configured IP address is matched against the presented addresses in the subjectAltName:iPAddr extension; if no such exist, against the presented CN component of the certificate Subject it is possible for the TLS server to not require additional name checks for incoming TLS clients. In this case, the certificate is accepted immediately after the RFC5280 trust chain checks.

And especially:

it is possible for the TLS server to not require additional name checks for incoming TLS clients. In this case, the certificate is accepted immediately after the RFC5280 trust chain checks.

I could interpret the above so that I do not have to implement a case where the verification would happen solely based on the client certificate being signed by an accepted certificate authority. This is the case with for example OpenRoaming Settlement-Free where organisations are free to connect with each other with only checking that the client and server certificates are signed by OpenRoaming root CAs. The TLS server does not really have all the connecting IP addresses or DNS names of the clients in their database then.

Should we have a third bulleting point for this? Or change the TLS Servers paragraph? I would say that a TLS Servers must be able to validate the connection only by certificate details without additional matching?

alandekok commented 1 year ago

The spec should require that verification happens solely based on a correct client certificate being presented.

i.e. if client and peer MUST do TLS layer authentication (certs, PSK, naked keys). But after that the client and server MAY do additional checks (subjectAltName is allowed, various OIDs exist or do not, PSK identity matches what's expected)

What should be forbidden is where a client does not authenticate a server, or where a server does not authenticate a client.

For me, the paragraph quoted above could be re-written. Instead of stating details first and TLS requirements last, it could do it the other way around:

Clients and servers MUST mutually authenticate each other at the TLS layer. This authentication can be done via certificates, PSKs, RFC 7250 public keys, etc. Clients and servers SHOULD perform additional authorization checks of the supplied credentials. For example, (OIDs exist or not, subjectAltName matches), or PSK identity is known, etc.

A particular set of TLS credentials MUST NOT be used on more than one client. For example, it is forbidden to use a global PSK identity and PSK across multiple clients, and it is forbidden to use one client certificate on multiple clients. For reasons of fail-over and load balancing, the opposite is not true. A particular set of TLS credentials can be used in multiple servers.

A server can also perform additional authorization checks based on non-TLS information. For example, verifying that the client IP address (source IP of the TLS connection) falls within a particular network range, etc.

The format here is to explain the high-level concept first: always authenticate, and then give additional details as to how that's done. I also thought it would be worth adding a note re-using credentials, because people will do that if it's not explicitly forbidden.

Janfred commented 1 year ago

The text for the mutual authentication requirement is good.

But I would object to the credential-sharing paragraph. I agree that it is a bad design if you use the same credentials for multiple devices. But I can image quite a number of use cases where i.e. sharing of TLS-PSK credentials is actually useful. It is the same way the RADIUS shared secret is used today: If I have a number of fat APs without a controller, but with some sort of management, I don't want to have a separate shared secret for every AP (this makes the configuration on the AP as well as on my RADIUS server very difficult). I want to authenticate the APs using a generic shared secret and a network rage.

As far as I can see we don't need a type of unique client identification, so I think it is acceptable to share secrets among devices in the same administrative domain and policy on the RADIUS server.

alandekok commented 1 year ago

The credential sharing is something which will be brought up in a review by the Security Area directorate. The standard should document best practices. If you want to ignore those recommendations, you're free to do so.

But the standards should explain why it's a terrible idea to re-use credentials.

For PSK, each machine should have it's own PSK identity, as that's the purpose of PSK identities. It might be OK to use the same PSK for multiple machines, and you could then claim that the credentials are different, so long as the PSK identities are different.

And if the provisioning is automated, who cares if the PSKs are different? The only cost of managing the PSKs is putting them into a database which matches PSK identity -> PSK.

khuhtanen commented 1 year ago

The spec should require that verification happens solely based on a correct client certificate being presented.

i.e. if client and peer MUST do TLS layer authentication (certs, PSK, naked keys). But after that the client and server MAY do additional checks (subjectAltName is allowed, various OIDs exist or do not, PSK identity matches what's expected)

What should be forbidden is where a client does not authenticate a server, or where a server does not authenticate a client.

I agree with the above.

For me, the paragraph quoted above could be re-written. Instead of stating details first and TLS requirements last, it could do it the other way around:

Clients and servers MUST mutually authenticate each other at the TLS layer. This authentication can be done via certificates, PSKs, RFC 7250 public keys, etc. Clients and servers SHOULD perform additional authorization checks of the supplied credentials. For example, (OIDs exist or not, subjectAltName matches), or PSK identity is known, etc.

The above is ok as well.

A particular set of TLS credentials MUST NOT be used on more than one client. For example, it is forbidden to use a global PSK identity and PSK across multiple clients, and it is forbidden to use one client certificate on multiple clients. For reasons of fail-over and load balancing, the opposite is not true. A particular set of TLS credentials can be used in multiple servers.

For the above paragraph I am not clear about this sentence: "For reasons of fail-over and load balancing, the opposite is not true." I think the 'not' should be removed there, if the idea is to allow using e.g. the same certificate for fail-over and load-balancing purposes.

Maybe this could also be rephrased to cover Janfred's use case although I agree that multiple PSKs stored in the database would solve the issue. I am also thinking if SHOULD NOT instead of MUST NOT would be sufficient and acceptable, but I have not reached a conclusion about it.

A server can also perform additional authorization checks based on non-TLS information. For example, verifying that the client IP address (source IP of the TLS connection) falls within a particular network range, etc.

I agree with this.

The format here is to explain the high-level concept first: always authenticate, and then give additional details as to how that's done. I also thought it would be worth adding a note re-using credentials, because people will do that if it's not explicitly forbidden.

I don't think we want in the actual protocol to implement the checks needed for verifying that same credentials are used only once so I would argue that these kind of requirements would be commented in a separate operational section perhaps with SHOULD/SHOULD NOT level requirement?

alandekok commented 1 year ago

On Nov 29, 2022, at 2:37 AM, Karri Huhtanen @.***> wrote:

For the above paragraph I am not clear about this sentence: "For reasons of fail-over and load balancing, the opposite is not true." I think the 'not' should be removed there, if the idea is to allow using e.g. the same certificate for fail-over and load-balancing purposes.

Yes, it's a little unclear. The goal is to say that individual servers can re-use the same certificate. Because servers in a load-balance pool are all logically the "same" server, and are presenting the same identity.

In contrast, every NAS is different. There is no reason to re-use the same identity on multiple NASes.

Maybe this could also be rephrased to cover Janfred's use case although I agree that multiple PSKs stored in the database would solve the issue. I am also thinking if SHOULD NOT instead of MUST NOT would be sufficient and acceptable, but I have not reached a conclusion about it.

My preference is to make the standards as secure as possible. That way people can still ignore the standard, but they've been warned about the consequences.

The format here is to explain the high-level concept first: always authenticate, and then give additional details as to how that's done. I also thought it would be worth adding a note re-using credentials, because people will do that if it's not explicitly forbidden.

I don't think we want in the actual protocol to implement the checks needed for verifying that same credentials are used only once so I would argue that these kind of requirements would be commented in a separate operational section perhaps with SHOULD/SHOULD NOT level requirement?

Sure. It's difficult for the server to know if the credentials have been reused.

restena-sw commented 1 year ago

FWIW, the text suggestion from Alan sounds good to me.

One could move text about credential sharing to Security Considerations maybe?

Janfred commented 1 year ago

With #6 some of this is fixed, but some issues are still open.