Spyderisk / domain-network

Network domain model
Apache License 2.0
1 stars 0 forks source link

Add Authorization Code Flow support to OIDC/OAuth model #85

Closed mike1813 closed 10 months ago

mike1813 commented 11 months ago

At present, the domain model supports a model whereby a service can delegate access control to another service. This is specified in a system model by asserting that the second service 'controls' the first. The second service may be specified to be in the AuthService asset class, meaning it is a specialised implementation designed for this purpose, and assumed to be less prone to software vulnerabilities than a general purpose ApplicationProcess.

The domain model assumes that when a Client tries to access the first service, it is redirected to the second for authentication, so the two services have a common client. If the Client is authenticated successfully, the second service responds by redirecting the Client back to the first service, using a URL that contains an access token. The domain model also assumes this token grants the client access to the first service, but at present it does not include authorization to use any 'back-end' resources - see #86.

The communication pattern corresponds to the OIDC/OAuth Implicit Flow model, although the domain model doesn't assume the implementation complies with the standards. Hence some aspects specified in the standard (e.g., the mandatory use of TLS) are not assumed - the user must assert this by selecting relevant controls.

The current implementation only supports the Implicit flow, in which the first service verifies that the token is signed by the second service but the two services do not communicate except via the common client. The access tokens therefore flow via the client.

Support for the Authorization Code flow should be added.

mike1813 commented 10 months ago

Detailed analysis now conducted of all authentication models, including:

The analysis classified threats to client-service relationships as 'internal' (caused by client-service relationships) or 'external' (with other causes).

In general:

Note that the interdependency threats in cases 4, 5 and 6 relate to Channel Confidentiality, Accessibility and Service Authenticity and not only to Client Authenticity.

To make this work, it is helpful to

Strictly speaking, these mean the revised domain model is no longer backward compatible with older system models. However, existing system models are unlikely to suffer problems, since (a) normally when one needs a reverse proxy one would make the asset a ServiceProxy whose behaviour has not changed, (b) the new relationship type doesn't correspond to any previous type, so adding that doesn't invalidate old models, and (c) it is rare for default levels to be overridden for the 'Comms Not Snoopable' attribute (Internet is Very Low, everything else Safe until affected by a threat).

It is then necessary to

Finally, other changes are likely to be needed for compatibility

mike1813 commented 10 months ago

First set of fixes covered:

Then merged fixes for issue #103, because data flow must be fixed before refactoring threats from Client Channels on Data Flows.

What still needs to be done:

mike1813 commented 10 months ago

Notes on first test runs:

mike1813 commented 10 months ago

First issue: remote access construction sequences.

The fix made to address #102 produces side effects when remote access is nested, e.g., someone uses their local machine to run SSH to access SSHD on a second (remote) host where they run another SSH to access another SSHD on a third host where they finally run a target process (a command line tool like a text editor).

This means the first SSH has remote (indirect) access to the second SSHD, This interferes with subsequent patterns refactored to address #103. This is because a ServiceProxy asset doesn't only represent HTTP(S) proxies like nginx. It can also represent an SSH Bastion service, configured to run SSH sessions for remote users.

Need to revert the original fix for #102, and find another way to address that issue.

mike1813 commented 10 months ago

Twelve iterations later, found a solution that doesn't create a problem elsewhere. It turns out that the key issues are as follows:

  1. Asserted model structures use false client-service relationships where the service is really a remotely accessed process. The real client-service relationship is with the remote access service, which then 'uses' the process signifying it is launched from the shell. If users add a false relationship, the real relationships must be constructed, along with the remote access service if also not asserted. However, if users assert the real relationships, the construction sequence must still work correctly and lead to the same outcome.
  2. The false relationships must be suppressed consistently and comprehensively before inferring data flow paths and channels. If this is not done, false paths will be found from remote access clients and remotely executed processes.
  3. Remote accessibility must be consistent and comprehensive - a remote access client running on its user's host must be related to every process from which remote access is possible. If processes are added by inference (e.g., inferred remote access services), there must still be a 'usesRemotely' relationship or the data flow inference won't be able to find the path used for remote access.

Getting all these right creates some sequencing dilemmas. They can't be done all at once because construction patterns must be executed in some order. The problem is that whichever is done first must work correctly despite the fact that the others still aren't addressed. The others are done later, and must respect any extra false relationships used to suppress asserted ones.

It turns out there is no sequence that works whether or not the user asserted real relationships or false ones, and in the latter case, whether or not they asserted the remote access service. The only way to cover everything is to deal with whichever the user added to the asserted model using one sequence, accepting that this won't work if they used false relationships. Then later one must add patterns that apply corrections in that case. So the three key points get addressed at different points in the sequence depending on which approach was used in the asserted model.

This is why things are so easily screwed up. The three requirements are met in different ways at different points in the sequence. If the patterns don't cover one of the three key requirements, it is easy to fix locally but not without disrupting later patterns.

It seems now to be working OK on 10 distinct test cases covering most (probably not all) possible permutations. The changes have been pushed to branch 85.

One thing that helped was renaming inferred relationships used to express credential sharing/forwarding and access via reverse proxies, which previously had very long names related to two distinct proxy models, one of which has now been replaced. This is discussed in #41.

mike1813 commented 10 months ago

Next step: improve the inference patterns to work out which clients use which back-end services indirectly, if not explicitly asserted.

The problem is that without this, either:

The proposed approach is to use the following principles:

  1. If a client could uses a service via a chain of reverse proxies or credential sharing intermediaries, 'via' relationships will be used to indicate that.
  2. If a client does use a service via a chain of reverse proxies or credential sharing intermediaries, the appropiate 'accessesService' relationship subtype is used, indicating the nature of the dependency. This is duplicated by the 'via' relationship, so it is possible to tell that access is indirect. From those two inputs, one can determine whether trust relationships or communication channels should be generated (step 5).
  3. These 'accessesService' subtypes can be added in several ways:
    • by a system-modeller user including the relationship in the asserted graph
    • by inferring the existence of a data flow between client and service via the chain (implies a 'uses' relationship)
    • by inferring the existence of an authentication exchange between client and service (implies a 'usesForAuth' relationship, but can only work if the intermediaries are all reverse proxies that forward credentials)
    • by inferring the existence of a remote access client using a remote access service (implies a 'uses# relationship, but can only work if the intermediaries are all reverse proxies)
  4. Where a client is related in these ways to a back-end service, a trust (but not communication) relationship is created, which is itself related to the client-service trust relationship (this time linked to communication channels) between adjacent client-service relationships in the reverse proxy/credential sharing/forwarding chain.
  5. The trust attributes for direct relationships between intermediaries depend on those for the end-to-end relationship in various ways depending on the attribute. The direct relationships can also be attacked in other ways, of course.
mike1813 commented 10 months ago

Changes now implemented.

Some refactoring was needed in the remote access inference sequence so it is easier to understand and check that all the relevant links are created. Previously the chain of remote access clients/services was being generated correctly, and data flow channels were being found for getting data to/from a remote interactive user, but some relationships needed by the authentication models were missed.

The presence of data flows allows indirect access from client to back-end service via credential sharing chains to be determined. The presence of a viaSharedCredentials relationship means this is not done until after communication channels have been added, so trust exists without direct communication.

Credential forwarding tests now behave as expected, apart from modelling error threat P.E.LnSucP.9, which detects remote access services that use and are controlled by a process. Previously this was disallowed because it implied the login service was acting as a proxy of some kind, which is clearly nonsense. Now it is acceptable because the relationships imply credential sharing but not acting as a proxy, although that only makes sense if the credentials is passed on by P to some other process.

For example, suppose a system model asserts that:

Having a remote terminal accessing P means RemoteTerminal is used to login to Host, and launch P from the shell. The model does not include a remote access service, so the remote access inference sequence fills the gap by adding a LoginService, where:

Now, the relationships between P and Q signify that P will be forwarding a credential to Q from its client. P has no client as such, because it was started from the shell (a RemoteTerminal-uses-LoginService session). Adding P-controls-LoginService indicates that P will get its credential via the shell, i.e., from RemoteTerminal. This is also added by the remote access inference sequence - a user could only do this for themselves if the LoginService had been included in their asserted model.

This corresponds to a procedure in which RemoteTerminal uses LoginService to carry out a remote command execution (of P), and passes a credential to P, either via the command line or (since that would make the credential visible to other users on Host) most likely by sending it to P via stdin after P has started. P then uses this credential to access Q, rather than using one of its own.

The point here is that one wouldn't do this to supply a credential for accessing P. That makes no sense because once you started a session with LoginService, no other credential is needed to start the process via a shell command. So without the Q-controls-P relationship, having P-controls-LoginService would be an error. In that situation, P would use its own credential to access Q.

Conclusion:

mike1813 commented 10 months ago

After 22 iterations, the regression tests pass for user interactivity via remote access clients (RAC) and remote access services (RAS), including access via reverse proxies and access using forwarded credentials.

The example of P and Q above turned out to be the critical aspect, but a bit more work was needed. The basic scenario is when a client RAC accesses process P via a RAS, and P in turn accesses Q. The user can specify this in two ways: by asserting simply that RAC-uses-P and P-uses-Q, or by including a RAS on the same host as P and specifying RAC-uses-RAS, RAS-uses-P and P-uses-Q. The inference sequence ensures the end result is RAC-uses-P, RAC-usesViaRAS-P, RAC-uses-RAS, RAS-uses-P and P-uses-Q. The usesViaRAS relationship between RAC and P indicates that their uses relationship should henceforth be ignored.

There are two credential-forwarding scenarios:

  1. Client RAC uses credentials shared by its client to access P. Since access is really to the RAS, this is expressed by adding RAS-controls-RAC. If the user didn't include the RAS, they will specify it as P-controls-RAC, so in that case the RAS-controls-RAC is inferred.
  2. P uses credentials shared by RAC to access Q. This is represented as P-controls-RAS (RAS forwards credentials from RAC to P), and Q-controls-P (P forwards credentials from RAS to Q), so the credentials used by P come from RAC. If the user didn't include the RAS, they will just assert that Q-controls-P, so the P-controls-RAS must be inserted.

So in the end, the opposed 'uses' and 'controls' relationships indicating credential forwarding can be established, and two 'controls' relationships are needed to set up forwarding from before the RAC through to Q: one from the RAS and one to the RAS. Each must be inferred if the corresponding side has an asserted 'controls'.

The same approach should work if two or three of the RAC, P and Q are (asserted to be) controlled by the same OIDC service. That has already been added to the remote access inference patterns in branch 85, but to fully test it requires some more test cases...

mike1813 commented 10 months ago

OIDC tests now run, yielding a new list of bugs to be fixed:

In addition, indirect client-service relationships (trust but no direct communication) are now determined from a basic relationship type ('uses', 'usesForAuth' or 'usesForAuthZ'), and a qualifier relationship indicating the indirectness ('viaSharedCredentials'). This means the base relationship type 'usesIndirectly' is never actually used, so it may make sense to also:

mike1813 commented 10 months ago

All issues that need to be addressed are now addressed on branch 85.