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

Poison pill string context #8388

Open
roberth opened this issue May 23, 2023 · 1 comment
Open

Poison pill string context #8388

roberth opened this issue May 23, 2023 · 1 comment
Labels
feature Feature request or proposal language The Nix expression language; parser, interpreter, primops, evaluation, etc security Security-related issues

Comments

@roberth
Copy link
Member

roberth commented May 23, 2023

Is your feature request related to a problem? Please describe.

Some values should not be used, or at least not in certain use cases. This can be correctly modeled with the string context.
Doing so makes the language safer and more reproducible, and let us implement features relying on these properties confidently.

Specific use cases so far:

  • Secrets. When Nix is used as a configuration language; one that outputs values and not just store paths, it is somewhat ok for it to process confidential values. By adding an ability to mark values as secret, we can prevent them from being added to the store. Denied usages: anything that adds to the store.
  • Impurities. A few values are quite useful to report in exception messages, but should not be used in other cases. For example, the version of the Nix evaluator must not become part of a NixOS configuration. Permissible usages: throw, trace, abort, but also compareVersions, as a compromise to allow expressions such as the Nixpkgs library to provide compatibility.
  • Exception messages. These could be considered to be impurities. Exception messages are encountered arbitrarily. The language does not have access to exception messages, but it would be ok for Nix expressions to work with these messages, as long as they don't end up in places that are supposed to be deterministic and stable. For instance, use of such strings could be permitted in only throw, trace and abort, as well as operations that don't extract information into types that can't carry the string context, such as ++ and substring, but not stringLength.

Describe the solution you'd like

Extend StringContextElem with constructors for use cases such as the above. Implement restrictions in the primops, and in the CLI.

Change builtins.unsafeDiscardStringContext to only discard derivation context items. Stripping of context should be done with specific usages in mind, and current usages do not consider confidentiality for example. By stripping everything, it would be easy to leak a secret into a derivation name for example, as those are stripped of context by the name sanitizing function.

Perhaps have a primop builtins.unsafeExposeSecret to strip confidentiality context anyway. Not all secrets are highly valuable, so careful application makes the system more useful. It should be possible to disable this function, so that more security focused users still have the absolute guarantee.

Describe alternatives you've considered

Exception messages. Exception messages could perhaps be handled by a function a -> (String -> String) -> a, such that the original message is only available within the callback.

Hiding things voluntarily. Something object capability style could be leveraged inside the Nix language; not something that's been done yet afaik. Speaking of "exploit", we could use the method inequality hack for this.

Use method inequality hack to enforce control over secrets unpacking
nix-repl> secrets = let t = mkToken ""; in { mkSecret = s: t2: assert t == t2; s; key = t; }

nix-repl> example = secrets.mkSecret "hi"

# Try to make a mistake
nix-repl> toString example
error: cannot coerce a function to a string

       at «string»:1:1:

            1| toString example
             | ^

nix-repl> "${example}"
error: cannot coerce a function to a string

       at «string»:1:2:

            1| "${example}"
             |  ^

# Try to attack it
nix-repl> example { token = x: x; }
error: assertion '(t == t2)' failed

       at «string»:1:45:

            1|  let t = mkToken ""; in { mkSecret = s: t2: assert t == t2; s; key = t; }
             |                                             ^

# Can access with the key
nix-repl> example secrets.key      
"hi"

This is kind of neat, but a gimmick, as management of the "key" burdens the user, while not really guaranteeing that the secret isn't written to the store, as it always has to be unpacked and protections won't apply after that; after any logic that was designed for plain strings.
tl;dr need less inconvenience, more safety.

Additional context

Priorities

Add 👍 to issues you find important.

@roberth roberth added feature Feature request or proposal language The Nix expression language; parser, interpreter, primops, evaluation, etc security Security-related issues labels May 23, 2023
@roberth
Copy link
Member Author

roberth commented May 25, 2023

fromJSON should preserve context, although this is not really possible without also allowing the exposure of attribute names, the number of items in lists, and the values of numbers, bools, and null.
While this is not great, a possible remedy is to convert non-string primitive types to strings, which is lossy, but possibly quite rare in secret json files that have to be parsed by Nix. This could be done in a Nix library by calling unsafeExposeSecret and appendContext. The library should inherit the "unsafe" and/or "expose" parts of the function name.

A more correct solution would be to make context an implicit wrapper for any type, but that's a very significant change that I wouldn't want to discuss before extra string contexts are implemented and learned from.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature request or proposal language The Nix expression language; parser, interpreter, primops, evaluation, etc security Security-related issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant