openconfig / public

Repository for publishing OpenConfig models, documentation, and other material for the community.
Apache License 2.0
897 stars 656 forks source link

Auth-Password security concerns #876

Closed sms-juniper closed 3 months ago

sms-juniper commented 1 year ago

generally for all protocols (ISIS , BGP) state/auth-password the leaf description contradicts the typedef description in use.

Also streaming plain-text is not secure.

dplore commented 1 year ago

Thanks for the issue. gRPC has TLS built in and is recommended. So this wouldn't be plain text on the wire. I do agree that the type should probably be deprecated in favor of a hash based string. Any comments from the community are appreciated.

dplore commented 1 year ago

@sms-juniper taking another look at this I think routing-password is the type you are talking about?

The description uses the term plain-text, but it's intent is that the routing-password type is actually an encrypted hash in the format of a string. By plain-text, it means string. We could update the description to clarify?

sms-juniper commented 1 year ago

Yes Darren , I was talking about oc-types:routing-password . It would be great if description is clear with the exact requirement.

jsterne commented 1 year ago

Agree the description is a bit unclear. The use of "plain text" should probably be avoided (perhaps use "unencrypted") and I think we'd also want to avoid the term "hash" or "hashed" since this type is a reversible encryption (as opposed to the "openconfig-hashed-value" extension).

Maybe the following sentence is good to include somewhere in an updated description: Clients reading the configuration or applied configuration for the node should expect to receive only the encrypted value.

earies commented 1 year ago

Thanks for the issue. gRPC has TLS built in and is recommended. So this wouldn't be plain text on the wire. I do agree that the type should probably be deprecated in favor of a hash based string. Any comments from the community are appreciated.

I would like to step back to the security concern here which is first at the level of what should be considered "storage of secrets in a management db/datastore"

The above statement is in relation to network transport and assumes gRPC services (and thus TLS) as the only NBI as well (more on this later).

But to the prior point first - there are generally a few classes of "secrets" as stored in a mgmt db - those that are reversible (either known simple algorithms or by way of accessing private keys, etc..) and those that are not (e.g. only storing the one-way hashed val) - the prior is where some object chooses to store the ciphertext, makes this visible possibly to NBI views (CLI, other APIs such as gNMI, NETCONF, etc..) but access to the plaintext variant is necessary for backend processes and/or further wire protocol communications that may use other security measures over the wire.

Now, for those reversible secrets, we could consider those "insecure" (since they are somehow reversible) but I think that would be naive in some cases as there are often other preventative measures and demarcations in that a user for NBI access w/ varying access control cannot access encryption keys or plaintext variants. In addition, any plaintext variant that is stored and/or accessible in a management db, filesystem, etc.. whether or not it is secured at the network transport layer is a non-starter for almost all security operations teams and compliance.

And real quick - back to network transport, there tends to be alot of assumptions of gRPC services as the sole API methods to access and interact with schema data within OpenConfig and while preferred within this overarching project scope, there often needs to be consideration of where the demarcations lay on management datastore schemas vs. the NBI. Now as there are times where some features choose to sit in gRPC services vs. YANG schema, the trend starts to shift towards no longer being NBI agnostic and if this is the case, then there should be an update and definition to the OpenConfig charter to mention that all other NBI are out of scope.

jhaas-pfrc commented 1 year ago

I will admit to not being deeply versed in the current state of discussion about gnmi authentication and authorization for individual leafs in the models. I'll make a very basic comment and hope that someone can highlight the expected protocol mechanism that is responsible for its enforcement.

It's generally the case that users that have authorization to do device configuration will have authorization to read such configuration. However, this isn't universally true and is enforced by mechanisms such as TACACS.

Similarly, for operational state, not every user is authorized to see everything.

A high level challenge while doing modeling for operational state is that the consumers of such state are in a significant number of scenarios more plentiful than those with configuration authorization, and also not entitled in many cases to sensitive data. A NMS that is expected to pay attention to BGP neighbor statistics and liveness likely doesn't need the TCP authentication passwords for MD5/AO or ipsec transports. Distributing such sensitive data should be kept to parties that need it... and generally that's not going to be true for most network monitoring applications.

This generally means a few things:

dplore commented 1 year ago

@jhaas-pfrc these are good comments. gNSI is a recent development which includes the RPC pathz which allows one to define which users are able to read/write on a per OC path basis. This could be used to implement the policy you describe by defining who can read+write /config and read /state paths.

Back the original issue. I think the request is to clarify the description for what format of string is intended to be used when writing to the leaf routing-password.

My expectation is a non-reversible, encrypted string should be provided as input. Shall we update the description to require a non-reversible, encrypted string?

@morrowc for comment.

jhaas-pfrc commented 1 year ago

Back the original issue. I think the request is to clarify the description for what format of string is intended to be used when writing to the leaf routing-password.

I presume that part of this is intending to also break the idea that this should be read-only? From the description: "Leaves specified with this type should not be modified by the system"

Or was the intent of that phrase the storing device rather than preventing modification by the end-user?

My expectation is a non-reversible, encrypted string should be provided as input. Shall we update the description to require a non-reversible, encrypted string?

There are, I think, different expectations depending on the direction of the interaction:

For write, the input needs to be:

  1. Plain-text so that the receiving system can do whatever it needs to do.
  2. Some form of obscured but reversable plain-text that the receiving system can do its appropriate management to.
  3. Store the result of a matching bit of cryptographic behavior that is executed by the client. This likely would involve disclosing keys to the client and probably isn't a reasonable expectation.

(Expected caveat: I'm not a cryptographer, and I'm rusty on even older exchange mechanisms.)

For read, the operational cases can vary:

  1. The plain-text is desired. As noted in prior discussion, this is highly sensitive.
  2. Some form of obscured plain-text. Better, but also a known issue, especially when many such mechanisms are trivially reversable.
  3. A value that is sufficient for testing whether the stored password changes. A one-way hash may be sufficient for this purpose!

My recommendation for read is a flavor of 3. It's reasonable for the model to suggest something along these lines: description "... For read, this leaf - if present - SHOULD contain a cryptographically obscured value of the configured password. Implementations SHOULD use an appropriate modern cipher. Such an example would be SHA-256. Implementations MUST only return this leaf to authorized users."

I'm sure someone can find an appropriate cryptographic cipher recommendation that is considered reasonable for this year.

jsterne commented 1 year ago

My expectation is a non-reversible, encrypted string should be provided as input

The routing-password typedef is used for BGP and IGP authentication keys. My understanding is that routers generally need to obtain the clear text version of these internally. So I believe this needs to be a reversible encrypted string (not a non-reversible hash).

I'd also recommend that the following be added to the description: Clients reading the configuration or applied configuration for the node should expect to receive only the encrypted value.

jhaas-pfrc commented 1 year ago

The routing-password typedef is used for BGP and IGP authentication keys. My understanding is that routers generally need to obtain the clear text version of these internally. So I believe this needs to be a reversible encrypted string (not a non-reversible hash).

Yes, plaintext. An obscured plaintext means that the mechanism for producing it must be known. I don't think that's what anyone wants for operations.

The write operation, as long as it goes over a secure channel, in plaintext isn't an unreasonable thing.

I'd also recommend that the following be added to the description: Clients reading the configuration or applied configuration for the node should expect to receive only the encrypted value.

There's a subtlety that it shouldn't be "THE encrypted value", rather "A encrypted value". There's zero need for operations where you're just seeing if the password has changed to have anything that resembles the internal storage format of the password. Text that suggests the cryptography may be reversible is likely not what is intended; a hash is an example of something with the appropriate properties.

jsterne commented 1 year ago

I think maybe a useful property though is that the value that is returned/output (in CLI, or a machine interface like gNMI) can also be provided back to the server as an input value. I think that means it can't be a hash. It needs to be reversible encryption that the server can decrypt (and then store in whatever internal format it wants, obtain the clear text value, etc).

jhaas-pfrc commented 1 year ago

current mechanisms that allow for replay are known insecure.

Let's be crystal clear here: Are you trying to enshrine in a new operational mechanism all of the insecurities of the past?

Please pay special note to the comments about "not everything needs this leaf". Solve that, and the equations start to move. While stuff like "pathz" might help, security out of the box should be considered.

In the presence of per-leaf authorization, the answer is to stop playing the obfuscation game and simply return the plain text leaf.

There are thus at least three operational scenarios:

  1. You're not allowed this info, the leaf simply isn't returned.
  2. You're allowed to monitor change behavior. You may be return a non-reversible encrypted leaf.
  3. You're in god-mode and allowed to get the actual configured value for replay for configuration.

Where 2 and 3 become messy is the value may be contextual and the retrieving node needs to know whether it's getting the plaintext or not.

github-actions[bot] commented 5 months ago

This issue is stale because it has been open 180 days with no activity. If you wish to keep this issue active, please remove the stale label or add a comment, otherwise will be closed in 14 days.