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

Allow disabling greasing the joint via feature flag #365

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nickray
Copy link

@nickray nickray commented Feb 1, 2023

It's a little off-putting in actual use to find "weird" recipients in the stanza, and the only way to ensure there's no "backdoor" hiding there is verifying the code.

If you want to grease by default, why do the keys get greased but not the passphrases?

I also wasn't sure if you'd like Cargo.lock checked in with PRs or not.

@str4d
Copy link
Owner

str4d commented Feb 1, 2023

I'd like to see if there's a way to make the grease more palatable before considering whether we merge this. I know that grease in non-interactive protocols like the age file format (particularly with a human-readable header in its binary format) is more likely to be seen by users than with interactive protocols like TLS, but I think there is a strong benefit in both protocol kinds to applying grease.

I definitely agree that if we did merge this, it would be off by default.

It's a little off-putting in actual use to find "weird" recipients in the stanza, and the only way to ensure there's no "backdoor" hiding there is verifying the code.

The -grease suffix on the stanza tag is intended to indicate to users what is going on. Could we modify the grease in some way to better clarify its purpose?

I'll note that I already considered and rejected using a grease- prefix, as that enables lazy parsers to just ignore anything between that prefix and the next ->. (They could still try to avoid parsing grease by splitting the header on -> and ignoring stanzas not matching known patterns, but at that point the parser is actively trying to sidestep the spec, and there's not much I can do.)

If you want to grease by default, why do the keys get greased but not the passphrases?

Because the age specification prohibits it:

An scrypt stanza, if present, MUST be the only stanza in the header. In other words, scrypt stanzas MAY NOT be mixed with other scrypt stanzas or stanzas of other types. This is to uphold an expectation of authentication that is implicit in password-based encryption. The identity implementation MUST reject headers where an scrypt stanza is present alongside any other stanza.

IIRC @FiloSottile wants to extend the spec to allow other stanza types to have this property (to enable custom symmetric stanzas). If that is implemented, those stanzas would similarly be impossible (and unnecessary) to grease.

@nickray
Copy link
Author

nickray commented Feb 5, 2023

A bit independent of your comments, it might be better to trigger the grease behaviour via --cfg flags instead of (negative) feature flags (unless it actually becomes some kind of generic argument which seems overkill), as it's ultimately the final binary that might want to enforce grease on/off, and not potentially intermediate libraries that could make conflicting (irreversible) choices.

My main argument to allow turning grease off remains: It's weird to find a recipient in data intended for interchange or long-term storage that "comes out of nowhere"; again, how does an auditor know this is not some backdoor to access the data.

If the age specification were to explicitly mention grease, that would be a helpful place to point to (so at least the concept/name has a reference explaining its motivation). You probably don't want to specify a grease format in that spec to avoid lazy parsers explicitly just skipping these.

Finally... I'm somewhat wondering if having to grease at all here is a kind of design smell. If I understand the spec correctly, the first word after --> denotes an implicit "code point" for a non-rendered registry of decentralized recipient types, and without reading the code in detail, I assume each recipient receives all the stanzas and picks out the ones it understands. So, maybe, recipients could define a prefix of one or more words that allow the rage framework to pick out the (most specific) applicable recipient, instead of all parsers repeating this "detect myself" code. That would need a mechanism to register such plugins at compile time, perhaps along the lines of dtolnay/inventory or dtolnay/linkme. Or it could just be a builder for constructing a Decryptor.

@FiloSottile
Copy link
Contributor

If the age specification were to explicitly mention grease, that would be a helpful place to point to (so at least the concept/name has a reference explaining its motivation). You probably don't want to specify a grease format in that spec to avoid lazy parsers explicitly just skipping these.

FWIW I'd be definitely happy with a age-grease spec in C2SP that explains the mechanism. If a parser goes out of its way to special case it and it's broken on multi-recipient files, there was no saving them. In fact, I am preparing some file introspection tooling where I plan to special case grease to avoid polluting the output, and the spec would help with that.

If I understand the spec correctly, the first word after --> denotes an implicit "code point" for a non-rendered registry of decentralized recipient types, and without reading the code in detail, I assume each recipient receives all the stanzas and picks out the ones it understands.

This is intentionally left implicit, and identities implementing their own "detect myself" logic is by design: it allows an identity to operate over recipient stanzas it doesn't "own" as well as apply logic based on stanzas it doesn't even know about. For example, the scrypt identity will reject files that have other non-scrypt recipients as that would break the authentication expectation. Finally, it doesn't ossify the stanza matching logic into whatever we implemented in age itself.

@str4d
Copy link
Owner

str4d commented Aug 6, 2023

This is going to interact with #403. In a 30-second chat with @FiloSottile we figured it will make sense to define grease as having an empty label set, so it doesn't ever get applied if labels are present. This would help the corp setting, but it might also be slightly annoying for e.g. the postquantum label.

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.

None yet

3 participants