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

age-plugin: Add labels extension to recipient-v1 #37

Merged
merged 5 commits into from
Aug 23, 2024

Conversation

str4d
Copy link
Contributor

@str4d str4d commented Aug 5, 2023

Designed with @FiloSottile.

This extension to recipient-v1 enables clients to require that plugins provide a set of labels at encryption time. These labels are used to restrict which stanzas can appear together in the same file header.

We don't make any changes to identity-v1 because decryption time restrictions are pointless: an attacker can drop stanzas or reuse the single stanza.

This can be used for

  • Authentication (auth X25519, scrypt) by setting a random label.
  • PQC by setting a well-known label.
  • Corp approval by setting a private but potentially shared label.

@str4d str4d mentioned this pull request Aug 6, 2023
age-plugin.md Outdated
Comment on lines 226 to 228
- The `x25519` recipient stanza has an implicit empty label set.
- The `scrypt` recipient stanza has an implicit label set containing a single
random label.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In str4d/rage#365 (comment) I noted that we also need to decide on how file header grease (currently unspecified) interacts with the label set, because what it results in is extra stanzas.

The simplest approach, and the one compatible with the behaviour of pre-labels clients, is that grease has an implicit empty label set. This lets it be added alongside x25519 and any pre-labels plugins, but not alongside scrypt. However, it means that plugins which start using labels would as a side-effect never have their headers greased. This might be fine (and in fact would probably be a nice side-effect for corp usage, where you probably don't want random extra stanzas), but would be annoying for e.g. postquantum-only headers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's indeed a greasing loss, but probably worth it to keep the implementation general. Empty label set sounds right.

FiloSottile
FiloSottile previously approved these changes Aug 24, 2023
age-plugin.md Outdated
Comment on lines 226 to 228
- The `x25519` recipient stanza has an implicit empty label set.
- The `scrypt` recipient stanza has an implicit label set containing a single
random label.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's indeed a greasing loss, but probably worth it to keep the implementation general. Empty label set sounds right.

age-plugin.md Outdated Show resolved Hide resolved
age-plugin.md Outdated Show resolved Hide resolved
age-plugin.md Outdated Show resolved Hide resolved
age-plugin.md Show resolved Hide resolved
age-plugin.md Outdated Show resolved Hide resolved
age-plugin.md Outdated Show resolved Hide resolved
Co-authored-by: Filippo Valsorda <hi@filippo.io>
age-plugin.md Outdated Show resolved Hide resolved
@str4d str4d mentioned this pull request Aug 19, 2024
We originally returned `(fail)` if the labels command was misformed, but
removed this in favour of MUST (NOT) language. In `age::plugin` this is
implemented by recording errors each time a violation occurs, and then
once the second phase is complete, ignoring any outputs from the plugin
if errors occurred.

Filippo made a PR suggestion to allow returning "an error" in the case
that the client detects that the provided labels would result in a label
set conflict. The idea was to enable communicating to the plugin that the
client would not accept any stanzas it provided, enabling the plugin to
avoid any potentially expensive computations for stanzas that won't be
used (matching the bullet point immediately underneath). However, this
approach is incompatible with clients that implement their plugin support
as custom implementations of the `Recipient` trait (as the `age` crate
does) or equivalent, because `Recipient::wrap_with_labels` by design does
not tell the recipient what other recipients are available or what label
set the client is currently checking. It is also a backwards-compatible
change to later allow clients to return `(fail)` if the provided label
set causes encryption to fail (older plugins might just return a less
useful error), so we can change our mind later.

The bullet point itself is separately unusable, because phase two of the
recipient-v1 protocol has this language:

> The plugin is the controller of this phase; clients should not close the
> connection if, for example, a user fails to respond in the way the plugin
> wants for a particular request. Instead, the client returns `(fail)` to
> indicate this; the plugin then decides whether this response is fatal.

Given that the `labels` command must be unique, the client knows there is
no way for the plugin to recover if an incompatible label set is provided,
and no way for the plugin to adaptively choose a different label set. But
the plugin might have other things it needs to tidy up before finishing,
so the client arbitrarily closing the connection is a bit rude and does
violate the plugin protocol.
@str4d str4d merged commit be16f49 into main Aug 23, 2024
1 check passed
@str4d str4d deleted the str4d/age-plugin-labels branch August 23, 2024 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants