Skip to content
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

2.10.1 Verify that intra-service secrets do not rely on unchanging credentials such as passwords, API keys #1032

Closed
Sjord opened this issue Jul 8, 2021 · 60 comments
Assignees
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet V2 _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@Sjord
Copy link
Contributor

Sjord commented Jul 8, 2021

2.10.1:

Verify that intra-service secrets do not rely on unchanging credentials such as passwords, API keys or shared accounts with privileged access.

What does this mean? How should you authenticate to other services? My application currently authenticates to the database server using username and password, and uses API keys to access several API's. Is that in violation of this requirement?

What is the proper way to do manage intra-service secrets? Can we rephrase this to a positive requirement instead of a negative requirement?

What does it mean for a secret to rely on credentials? When are credentials unchanging?

What was the original purpose of this requirement?

See also #763.

@jmanico
Copy link
Member

jmanico commented Jul 8, 2021 via email

@elarlang elarlang added the 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet label Jul 20, 2021
@Sjord
Copy link
Contributor Author

Sjord commented Jul 30, 2021

I attempted to retrieve the history of this requirement, but it doesn't provide much more information:

  • 0e0ca92 Verify that intra-service secrets do not rely on unchanging passwords, such as API keys or shared accounts with privileged access.
  • c0f9c29 Integration secrets SHOULD NOT rely on unchanging memorized secrets, such as API keys or shared privileged accounts. If memorized secrets are required, the credential should not be a default account, and stored with sufficient protection to prevent offline recovery attacks, including local system access.
  • 81a5b36 Integration memorized secrets SHOULD NOT rely on unchanging memorized secrets, such as API keys or shared privileged accounts. If memorized secrets are required, the credential should not be a default account, and stored with sufficient protection to prevent offline recovery attacks, including local system access.

@Sjord
Copy link
Contributor Author

Sjord commented Aug 13, 2022

I propose to delete this requirement.

@wet-certitude
Copy link

@jmanico I would consider the private key of an mTLS connection "unchanging" as well. I see that the private key, unlike a password/token is not transmitted - at least that is one interpretation, but one that after much thought, I could not figure out based the current stipulation alone.

Since it's been over a year since this was reported and no one was able to figure out what exactly this means, no one is going to miss this stipulation if we simply remove it?

@settantasette
Copy link

I do not think deleting this requirement is the right way to go, rather I would re-phrase it to bring in some clarity. I understand @jmanico's point, but at the moment the way control point is presented is very confusing. Verify that intra-service secrets do not rely on unchanging credentials such as passwords, API keys or shared accounts with privileged access.
Does it mean that secrets should not use passwords etc. whatsoever ? Or if for example passwords are rotated daily then its acceptable ? Since its not "unchanging credential" anymore. Then question comes what is acceptable rate of rotation of credentials ?

@tghosth tghosth assigned elarlang and tghosth and unassigned Sjord and jmanico Dec 7, 2022
@tghosth tghosth added _5.0 - prep This needs to be addressed to prepare 5.0 josh/elar labels Dec 7, 2022
@tghosth tghosth removed the josh/elar label Dec 12, 2022
@tghosth
Copy link
Collaborator

tghosth commented Dec 12, 2022

@set-reminder 5 weeks @tghosth to look at it

@octo-reminder
Copy link

octo-reminder bot commented Dec 12, 2022

Reminder
Monday, January 16, 2023 12:00 AM (GMT+01:00)

@tghosth to look at it

@tghosth
Copy link
Collaborator

tghosth commented Dec 19, 2022

I agree this requirement is unclear and it is not clear what it was based on to begin with.

Given the topic of this section, I would suggest simplifying to:

Verify that service authentication uses either mTLS or a credential which is unique to that service and rotatable. Where supported, temporary session tokens should be generated and used instead of sending the credential on each connection.

Thoughts/opinions?

@tghosth tghosth added 4) proposal for review Issue contains clear proposal for add/change something and removed 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet labels Dec 19, 2022
@tghosth tghosth removed their assignment Dec 19, 2022
@octo-reminder
Copy link

octo-reminder bot commented Jan 15, 2023

🔔 @tghosth

@tghosth to look at it

@Sjord
Copy link
Contributor Author

Sjord commented Jan 20, 2023

Verify that service authentication uses either ...

I think we should have a requirement that requires authentication between services.

either mTLS or a credential

I think other methods of authentication are also acceptable. Although "a credential" can of course mean a certificate or a JWT or anything.

which is unique to that service and rotatable

This should perhaps be moved to a more general requirement. All the secrets that the application uses itself should be unique, rotatable, non-default (2.10.2), etc.

Where supported

This is a pretty big escape hatch, if the developing party is developing both services that communicate with each other. They can choose to not support this.

temporary session tokens should be generated and used instead of sending the credential on each connection.

I don't understand this. Why would this be better?

@sseelmann
Copy link

Doesn't the current 2.10.1 contradict with 2.10.4 which defines how such "internal secrets" (if that's meant by "intra-service secrets") should be managed:

2.10.4 [GRAMMAR] Verify passwords, integrations with databases and third-party systems, seeds and internal secrets, and API keys are managed securely and not included in the source code or stored within source code repositories. Such storage should resist offline attacks. The use of a secure software key store (L1), hardware TPM, or an HSM (L3) is recommended for password storage.   798  

PS: and isn't 2.10.4 overlapping with 6.4.1? But that would be a separate issue to discuss.

6.4.1 [MODIFIED] Verify that a secrets management solution such as a key vault is used to securely create, store, control access to and destroy secrets such as service account or 3rd party application credentials. (C8)   798

@tghosth
Copy link
Collaborator

tghosth commented May 23, 2023

Adding some additional context:

History:

# Description L1 L2 L3 CWE NIST §
2.10.1 Verify that intra-service secrets do not rely on unchanging credentials such as passwords, API keys or shared accounts with privileged access. 287
2.10.1 Verify that intra-service secrets do not rely on unchanging credentials such as passwords, API keys or shared accounts with privileged access. OS assisted HSM 287 5.1.1.1
2.10.1 Verify that intra-service secrets do not rely on unchanging passwords, such as API keys or shared accounts with privileged access. OS assisted HSM 287 5.1.1.1
2.10.1 Verify that integration secrets do not rely on unchanging passwords, such as API keys or shared privileged accounts. OS assisted HSM 287 5.1.1.1
2.11.1 Verify that integration secrets do not rely on unchanging passwords, such as API keys or shared privileged accounts. Software OS assisted HSM 5.1.1.1
2.11.1 Integration secrets SHOULD NOT rely on unchanging passwords, such as API keys or shared privileged accounts. Software OS assisted HSM 5.1.1.1
2.11.1 Integration secrets SHOULD NOT rely on unchanging passwords, such as API keys or shared privileged accounts. If passwords are required, the credential should not be a default account and stored with sufficient protection to prevent offline recovery attacks, including local system access. Software OS assisted HSM 5.1.1.1
2.21 Integration secrets SHOULD NOT rely on unchanging memorized secrets, such as API keys or shared privileged accounts. If memorized secrets are required, the credential should not be a default account, and stored with sufficient protection to prevent offline recovery attacks, including local system access. Software OS assisted HSM 5.1.1.1
2.21 Integration memorized secrets SHOULD NOT rely on unchanging memorized secrets, such as API keys or shared privileged accounts. If memorized secrets are required, the credential should not be a default account, and stored with sufficient protection to prevent offline recovery attacks, including local system access. Software OS assisted HSM 4.0 5.1.1.1

From Andrew's deck:
image

@tghosth
Copy link
Collaborator

tghosth commented Feb 8, 2024

@elarlang to review the updated wording.

@tghosth tghosth removed the next meeting Filter for leaders label Feb 8, 2024
@Sjord
Copy link
Contributor Author

Sjord commented Feb 8, 2024

which is more secure than fixed credentials (such as mTLS, temporary or ephemeral credentials, or fixed credentials plus a session mechanism)

I think requiring something to be "more secure than" already requires security knowledge. It explains security in terms of itself.

Also, I know something about security and I still don't understand why a session mechanism is more secure than fixed credentials.

@jmanico
Copy link
Member

jmanico commented Feb 8, 2024

Also, I know something about security and I still don't understand why a session mechanism is more secure than fixed credentials.

session theft ends when that session goes idle or gets logged out of, and events that require re-authentication are a good blocked when I popped your session but dont have your creds.

credential theft is usually full account compromise forever

Both are nasty, but credential theft is way worse. Especially when no MFA is in play.

@tghosth
Copy link
Collaborator

tghosth commented Feb 8, 2024

which is more secure than fixed credentials (such as mTLS, temporary or ephemeral credentials, or fixed credentials plus a session mechanism)

I think requiring something to be "more secure than" already requires security knowledge. It explains security in terms of itself.

Also, I know something about security and I still don't understand why a session mechanism is more secure than fixed credentials.

Whilst it is less concerning for service to service components, fixed credentials are still more exposed than a session mechanism.

@tghosth
Copy link
Collaborator

tghosth commented Feb 8, 2024

In #1858, @jmanico noted:

I suggest we change "user accounts" to "service accounts" based on the context of this requirement.

# Description L1 L2 L3 CWE
1.2.2 [MODIFIED] Verify that communications between back-end application components, including APIs, middleware and data layers, are authenticated and use individual user accounts. (C3) 306

And @elarlang noted:

As it is implementation requirement, it should be moved away from V1 category.

But what exact attack scenario or weakness does the requirement address?

Not all services require authentication.

What is accepted authentication? Basic auth vs some application logic for auth vs certificate...

Personally, I think this feels like duplication with the modified requirements I drafted above #1032 (comment) which are based on 2.10.1 so I would be tempted to remove it altogether.

What do people think?

@jmanico
Copy link
Member

jmanico commented Feb 8, 2024

Agree this is a duplicate

@tghosth
Copy link
Collaborator

tghosth commented Feb 13, 2024

Ok so current proposal is as my comment here: #1032 (comment)

plus, remove requirement 1.2.2 #1032 (comment)

@Sjord
Copy link
Contributor Author

Sjord commented Feb 13, 2024

I think fixed API keys are fine. GitHub, GitLab, and AWS apparently think API keys are fine. I asked several times why a session mechanism would be more secure than API keys. I received some responses to this, but I am still not convinced. Arguments against API keys given above feel pretty handwavy to me. The proposal of @tghosth assumes that it is very clear that some things are "more secure than" fixed credentials, but this is not clear to me.

This doesn't feel ready for a PR to me. My main problem is not that we disagree, but that I don't understand the motivation to disallow API keys. I think other users of the ASVS would have an even harder time understanding it.

when I popped your session but dont have your creds

What is an attack scenario where an attacker can access the session token but not the credentials that started that session?

@tghosth
Copy link
Collaborator

tghosth commented Feb 18, 2024

What is an attack scenario where an attacker can access the session token but not the credentials that started that session?

Fixed credentials are used on every single API request whereas with a session mechanism the credential on an individual request is shorter lived. If an attacker gets access to an individual request, (log files, fluke interception, etc) they don't get access to the fixed credentials.

APIs which support OIDC would support this requirement as the client will generally go through the OIDC process to authenticate and then use an access token.

I do accept that this seems to be a less popular approach. It looks like AWS does this with STS. On the other hand, even Github seems to use some sort of fixed credentials for their API.

We currently have two proposed requirements to replace the existing one:

Aimed at service creators:

Verify that for a service designed to be consumed by other services, authentication uses either mTLS or offers a session mechanism to avoid use of fixed credentials each time.

Aimed at service consumers (L3 proposed):

Verify that consumers of a service are configured to use an authentication option which is more secure than fixed credentials (such as mTLS, temporary or ephemeral credentials, or fixed credentials plus a session mechanism) where this option is provided by the service being consumed.

I think in both cases, they explain what is considered more secure so I guess the question is whether you have a problem with one or both and how we can bridge that gap @Sjord .

@Sjord
Copy link
Contributor Author

Sjord commented Feb 20, 2024

You argue that the impact is lower if the attacker has access to a session token instead of an API key. Regardless as whether that is true or not, that is not the whole picture.

We don't control what data the attacker compromises. Using session tokens does not mean that the credentials are now necessarily more secure. An application can put credentials in a log file whether it uses a session mechanism or not.

Risk does not only consist of impact but also of probability. The probability that either the session token or the credentials are exposed is very small. I don't think the probability of credentials being intercepted scales linearly with the number of times they are used. In my opinion, it does not make much sense to reduce the number of times credentials are used.

The cost of a session mechanism is non-zero, both in terms of performance as in security. It adds multiple round trips to set up and tear down the session. It also requires keeping state, or doing cryptography, to manage session tokens. This can introduce additional bugs.

A world where an attacker has a session token may be preferable to a world where they have an API key. But that is not sufficient to say that session tokens are more secure than API keys.

If an attacker gets access to an individual request

This is not part of my threat model, and probably also not of ASVS threat model. We don't propose to do client-side password hashing, for example.

the question is whether you have a problem with one or both and how we can bridge that gap

I have a problem with both. I think you need to have a good reason to disallow fixed API keys. The current motivation seems insufficient to me.

What could possibly convince me are examples of attacks that would have been prevented by these requirements, or more arguments about the probability and attack scenarios and why the costs of a session mechanism way up against the risk.

@tghosth tghosth added next meeting Filter for leaders and removed next meeting Filter for leaders labels Feb 20, 2024
@tghosth
Copy link
Collaborator

tghosth commented Apr 3, 2024

You argue that the impact is lower if the attacker has access to a session token instead of an API key. Regardless as whether that is true or not, that is not the whole picture.

We don't control what data the attacker compromises. Using session tokens does not mean that the credentials are now necessarily more secure. An application can put credentials in a log file whether it uses a session mechanism or not.

Yes but it is a lot less likely if they are just using the credentials infrequently to generate the session as opposed to sending the credential on every request.

Risk does not only consist of impact but also of probability. The probability that either the session token or the credentials are exposed is very small. I don't think the probability of credentials being intercepted scales linearly with the number of times they are used. In my opinion, it does not make much sense to reduce the number of times credentials are used.

There are also options such as mTLS or locally generated in which the credentials are never transmitted so the probability of exposure in transit or in request leakage becomes zero.

The cost of a session mechanism is non-zero, both in terms of performance as in security. It adds multiple round trips to set up and tear down the session. It also requires keeping state, or doing cryptography, to manage session tokens. This can introduce additional bugs.

This depends on the implementation and certainly this is a well explored problem with minimal need to reinvent the wheel.

A world where an attacker has a session token may be preferable to a world where they have an API key. But that is not sufficient to say that session tokens are more secure than API keys.

They are certainly more secure in isolation. I think your point is more a question of does it justify the investment and what is the risk of other security vulnerabilities arising in the session mechanism. I think that having this as a L2 or L3 requirement reflects that trade off.

If an attacker gets access to an individual request

This is not part of my threat model, and probably also not of ASVS threat model. We don't propose to do client-side password hashing, for example.

I think I didn't explain myself clearly. My point is that with static keys, it is enough for an attacker to gain access to any individual request out of the 10s or 100s of requests which are sent to the API. If we are not using static keys then the attacker either has to gain access to the specific authentication request or there may be no authentication request to gain access to in the first place.

What could possibly convince me are examples of attacks that would have been prevented by these requirements, or more arguments about the probability and attack scenarios and why the costs of a session mechanism way up against the risk.

Leakage of sensitive data into logs is endemic and often a hard problem to solve. With static keys this is potentially even more problematic because even if a request body is not being logged then the headers may well be (where the fixed key will almost certainly be transmitted in a header).

For static keys there is also no defined standard for where they should be sent.

For example, in this GitLab documentation a static key can be sent in the following locations which logging sanitization of secrets would potentially have to take into account:

  • A private_token query parameter
  • A PRIVATE-TOKEN: header
  • A Authorization: Bearer header
  • A job_token query parameter
  • A JOB-TOKEN: header

GitHub seems to have less token types and be more consistent on the use of an Authorization: header.

In summary, static API or intra-service secrets are significantly more exposed than other solutions. I don't think it is unreasonable to have an L2 or L3 requirement on service creators for a more secure mechanism. I also don't think it is unreasonable to have a L3 on service consumers to use the most secure method available.

@Sjord
Copy link
Contributor Author

Sjord commented Apr 3, 2024

Yes but it is a lot less likely if they are just using the credentials infrequently

My point is that with static keys, it is enough for an attacker to gain access to any individual request out of the 10s or 100s of requests which are sent to the API. If we are not using static keys then the attacker either has to gain access to the specific authentication request

You seem to suggest that if you only send the credentials in 1% of the requests, the chance of compromise is reduced by ~100. Is that correct? This assumes that there is a non-zero per-message probability that it is compromised. I don't think that's true.

I consider TLS connections perfectly secure. I don't think it's possible "for an attacker to gain access to any individual request". Even if TLS weren't secure, the connection would be compromised, not a single HTTP interaction. In practice, f there is some TLS bug or misconfigurations, probably all connections would be compromised.

Both session tokens and credentials are being logged. You can log credentials, whether you use sessions or not. I am not sure that using sessions reduces the chance of logging credentials. If a log is compromised, it probably contains both the setup and use of the session.

I don't think it is as simple that using credentials less often directly reduces the risk that they are compromised.

@elarlang
Copy link
Collaborator

elarlang commented Apr 3, 2024

Feels in the loop...

I'm not convinced we need it as a requirement, it seems to be so edge-case and 2nd or 3rd layer of defense. From the ASVS side, the risk for me is - if we put this as a requirement to solve one certain problem for some edge-case scenario, we create a rule that is not suitable or is not really needed for other situations. But, maybe I missed something.

  • The likelihood - What an attacker must achieve first that this defense comes into play?
  • Is it reasonable - What risk does it solve vs what risk does it create to implement it?
  • Limitations - What are the situations and solutions it is suitable and what are situations and solutions where is it not suitable?

@jmanico
Copy link
Member

jmanico commented Apr 3, 2024 via email

@TobiasAhnoff
Copy link
Contributor

Thanks for sharing a very interesting discussion, I hope this will contribute!

When my colleagues and I do security reviews and advise customers on service-to-service communication (without user interaction) we often highlight the risk of not rotating secrets.

Having a plan for rotating secrets ("service credentials", not personal user passwords) is important for secure service integrations over time and I believe it would be a good thing if ASVS included a requirement for that (perhaps L2 or L3).

If a secret is well-protected and there are additional layers of defense, like least privilege secret scope, proper network segmentation, end-to-end securely configured TLS, monitoring and alarms on secret usage, then you could argue that a particular secret could be rotated less frequently, compared to a powerful secret for a service accessible from public internet (without additional defenses.)

But (I my opinion) all secrets should have an expiration and rotation plan (at least for L3.) In example this is recommended/required for all secrets managed in services like Azure Key Vault
https://learn.microsoft.com/en-us/azure/governance/policy/samples/cis-azure-1-1-0#ensure-that-the-expiration-date-is-set-on-all-secrets

Given this background and experiences, I have always interpreted that the key aspect of 2.10.1 is "unchanging", so a suggestion is to have a L2 or L3 requirement in ASVS that more clearly capture that, like this:

"Verify that for all secrets (except personal user account passwords) there is a plan to rotate the secret according to applicable threat models, policies and requirements. "

This does not address the "how often" question, as it depends...and is hard to express in a good way, for some scenarios a reasonable plan might be once a year or when someone leaves the group having access to the key (as part of an off-boarding procedure)

Hope this was good input!

@elarlang
Copy link
Collaborator

elarlang commented Apr 3, 2024

Seems, that Jim's chatbot kind of agrees with my views.

@tghosth - the discussion here is quite long and contains many different topics (sessions, rotating creds, etc). Just in case please repeat your latest proposals, to be sure that everyone is talking about and commenting on the same thing.

@tghosth
Copy link
Collaborator

tghosth commented Apr 4, 2024

Some comments from Twitter:


Client Assertion Grant....can be used e2e without the issues we see with mTLS termination gaps
And get Vault to manage the short lived certs

https://twitter.com/j_w_holland/status/1775519957964460360


I think it is a relatively useless debate. Risk appropriate secrets management is the control. Static keys in code are bad. Static restricted per application keys pulled from a vault as needed are not the same thing.

https://twitter.com/wimremes/status/1775419262934352068


Asymmetrical signed JWT + JWKS, with auto key pair rotation monthly + a NONCE claim attribute (ideally).

https://twitter.com/darraghduffy/status/1775633935365472590


@elarlang elarlang added 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet and removed 4) proposal for review Issue contains clear proposal for add/change something labels Apr 4, 2024
@tghosth tghosth added the V2 label Nov 6, 2024
@ImanSharaf
Copy link
Collaborator

I am going to close this as it seems it has been resolved. @tghosth

@tghosth
Copy link
Collaborator

tghosth commented Nov 7, 2024

The text is currently as follows. If there are still concerns, ping me here and I will reopen:

# Description L1 L2 L3 CWE
14.7.1 [MODIFIED, MOVED FROM 2.10.1, MERGED FROM 1.2.2] Verify that communications between back-end application components which don't support the application's standard user session mechanism, including APIs, middleware and data layers, are authenticated. Authentication should use individual service accounts, short-term tokens or certificate based authentication and not unchanging credentials such as passwords, API keys or shared accounts with privileged access. 287

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet V2 _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

No branches or pull requests

9 participants