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

Unclear TODO "Flags (s.a. disabled) checks are not yet implemented!!!") #456

Open
dkg opened this issue Jul 22, 2023 · 7 comments
Open

Unclear TODO "Flags (s.a. disabled) checks are not yet implemented!!!") #456

dkg opened this issue Jul 22, 2023 · 7 comments

Comments

@dkg
Copy link
Contributor

dkg commented Jul 22, 2023

in pgpy/pgp.py, PGPKey.check_management contains this warning, added by @KOLANICH in b8c28c5:

warnings.warn("TODO: Flags (s.a. `disabled`) checks are not yet implemented!!!")

It's not clear what this means. I'm assuming "s.a." means "such as", but what is a "disabled" flag check? There is no such concept in RFC 4880 or in draft-ietf-openpgp-crypto-refresh-10.

GnuPG's --with-colons output may contain a D flag in field 12 to mean "disabled" (see /usr/share/doc/gnupg/DETAILS.gz), but this doesn't appear to be any sort of interoperable standard.

There is also no other way for a user of PGPy to mark a key as disabled.

If there's some plan to do this, the plan should be outlined in a bug report, rather than adding a noisy warning.

@KOLANICH
Copy link
Contributor

the plan should be outlined in a bug report, rather than adding a noisy warning.

The intent of the checks in this lib is similar to the intent of the checks of TLS certificates. If anything is not OK, then we just fail rather than allow potential compromise.

Currently we have some checks, but in order to inform users of this lib that the checks are not quite doing everything we expect them to do, so we try to warn them via warnings, because we know almost noone reads bug trackers if thay are not about the bugs the uset has encountered.

@dkg
Copy link
Contributor Author

dkg commented Jul 23, 2023

Thanks for the explanation, @KOLANICH, but i still don't get it. What flags aren't currently checked? we do indeed check usage flags (see the KeyAction decorator in pgpy/decorators.py). What is a disabled flag?

There is a separate (and correct) TODO emitted as a comment about not checking revocations. I think it is reasonable to keep that one until it gets resolved. But i'm trying to think about how to resolve this one, and i wouldn't even know how to begin.

A warning that just sounds scary without being actionable is worse than no warning at all.

@KOLANICH
Copy link
Contributor

I don't remember exactly what I meant then (quite a long time has passed since I have implemented verification), but I suspect that the meaning was that we don't check Trust packet (I think PGPy should support importing and using files created by gnupg, since gnupg is the major and de-facto standard Othough quite poor) implementation, though it doesn't mean that we always should stick to GPG).

@dkg
Copy link
Contributor Author

dkg commented Jul 23, 2023

the Trust packet's defnition explicitly says that the meaning is only defined for a given implementation, and that it generally shouldn't be exported or imported between implementations. I think PGPy should ignore it entirely, and never produce it.

@KOLANICH
Copy link
Contributor

I have read the most recent draft of RFC 4880 bis, and I think it can be improved. IMHO it leaves too many things up to an implementer. While it allows some flexibility, IMHO it should be addressed in a more constrained way.

I propose to replace the loose words with code in Haxe (one can say "pseudocode", but I think pseudocode usually has all the disadvantages of real code that it has to be translated into the target language, but real code allows to skip the translation at least for the case when the target language is the same as the code used, and Haxe was created with the goal of transpilation in mind) doing the needed things and require the usage of the direct close-to-original translations into target languages (with necessary modifications done for the exact project and taking into accou t programming language specifics, but the meaning of the code MUST be the same). In other words, standardize the implementation. The implementers should be encouraged to create scripts transforming the code from the RFC into the code within their project source tree.

  1. The spec says human words about which cryptoprimitives are allowed to be used and which way they should be ranked. That words should be replaced with Haxe code, which should have points doing look-ups in collections and comparison to numerical values provided as arguments to the function. The sets of such parameters are profiles, and are defined as a string containing JSON-serializable objects. The JSONSchema of such an object for collection of profiles should be defined first, and then should be placed a string containing the exact profiles. Implementations should be encouraged to copy those directly from the RFC. The root object is a mapping from a profile name to a profile object. A profile has a name of 2 components separated by a dash. The first one is the name of the device, and the second one is the security level. It should be possible to update the RFC by replacing the part of it defining profiles with the new ones.

The RFC should define at least 2 profiles. The personal-SOTA profile defining the best practices. And the legacy-SOTA profile matches behavour of the previous version of the RFC. There can be other profiles matching the needs of industry, like embedded for embedded devices.

  1. The RFC doesn't define which keys should be considered trusted. IMHO the spec should define trust models as objects in Haxe. An object for a trust model should have at least 2 functions. The first one is a predicate accepting boolean arguments and checks if such trust model applies to the use case of implementer and his users. It is not intended to be implemented in implementer's software, but may be used to implement applications advising users certain trust models based on their needs. The second one implements the actual trust model and returns true if a "key" is trusted in the context.

  2. The RFC doesn't define the exact algorithms doing other complex actions that can be tricky even to define right. Like verification of keys. I think that the RFC should contain the high level algorithms doing that in Haxe. And should contain the prescription to always use such algorythms from the latest RFC.

  3. The RFC should contain certain algorithms related to decision making related to the RFC itself. For example, the prescription from 3. can be expressed as a function getRFCVersionForAnAction returning the version of the RFC, a mapping from versions of RFC to the "backends" rfcVersions, each of which contains the logic for each implemented version of the RFC, and a function getAFunctionForAnAction selecting a function that checks for the special case, and for the rest returns rfcVersions[getRFCVersionForAnAction(ctx, action)][action].

  4. I don't like the ways file formats are described. I think that the packets should be described in DDLs like Kaitai Struct and relationships between them by mapping the AST resulted from parsing of binary packets into JSON-like objects with some synthetic keys for subpackets and then describing their relations via a JSONSchema document.

@dkg
Copy link
Contributor Author

dkg commented Jul 24, 2023

fwiw, the draft you're looking at is not an IETF draft, it's something that Werner Koch is developing indepenently. The draft that has been submitted to the IESG for publication is draft-ietf-openpgp-crypto-refresh.

I understand that you want the specification to be more formal, but that's not generally how IETF specs are written, for better or for worse.

If you're interested in trying to write up a more formal specification, I would be interested in reading it. Perhaps that's something that the OpenPGP working group would consider if the group decides to recharter. There will be a meeting on Friday if you're interested in joining. you may need to enroll in the meeting if you want full audio/video participation, but i think you might be able to join the audio stream and the text chat without fully subscribing.

Whether access to the meeting works or not, you should go ahead and formulate your desired outcome and send it to the workingn group mailing list, [email protected]

@KOLANICH
Copy link
Contributor

Thanks for the info and for the invitation, but unfortunately I cannot be fully involved into OpenPGP standardization in observable future.

About the warning: feel free to remove it.

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

No branches or pull requests

2 participants