search

trusteddomainproject / OpenDKIM

Other
97 stars 52 forks source link

Parsing of RSAPublicKey format from DNS #52

Closed sp-andwei closed 1 year ago

sp-andwei commented 5 years ago

see #51

If I haven't missed any important update to the DKIM spec, the current spec says:

RFC6376, Section 3.6.1:

The "rsa" key type indicates that an ASN.1 DER-encoded [ITU-X660-1997] RSAPublicKey (see [RFC3447], Sections 3.1 and A.1.1) is being used in the "p=" tag. (Note: the "p=" tag further encodes the value using the base64 algorithm.) Unrecognized key types MUST be ignored.

Errata exist (https://www.rfc-editor.org/errata/eid3017) that extend this to

The "rsa" key type indicates that an ASN.1 DER-encoded [ITU-X660-1997] RSAPublicKey (see [RFC3447], Sections 3.1 and A.1.1), which MAY be contained in a SubjectPublicKeyInfo (see [RFC5280], Section A.1), is being used in the "p=" tag.

The errata is set to "Held for Document Update":

The erratum is not a necessary update to the RFC. However, any future update of the document might consider this erratum, and determine whether it is correct and merits including in the update.

To my understanding, this may mean anything with regard to a future version of the spec.

I'm aware that appearantly the whole world uses SubjectPublicKeyInfo in published DNS records and I also noticed that, e.g. dkimpy is in its current state is also not able to parse a RSAPublicKey which is not contained in a SubjectPublicKeyInfo from the 'p=' tag in a TXT record. But still, the RFC defines the RSAPublicKey-represenation definitely as valid and therefore I think it would be somehow correct to support it.

sp-andwei commented 4 years ago

fyi: same suggestion made to dkimpy, has been merged for some time now: https://bugs.launchpad.net/dkimpy/+bug/1851862

thegushi commented 1 year ago

This is rare, and useful for a future release, but I'm flagging this as enhancement for the moment, pending patches.

thegushi commented 1 year ago

As discussed, openDKIM presently doesn't parse the "containered" format when in DNS. The errata also suggests (in notes) that: (1) Accordingly, most current implementations will accept such keys. (2) Furthermore, it is trivial to distinguish whether a key is encapsulated in a SubjectPublicKeyInfo. (Annotations mine.)

(1) We don't. (2) It's not trivial, visually, when the result has been base64-encapsulated, masking the usual ----BEGIN RSA PUBLIC KEY---- stuff. Additionally, things are moving well in favor of smaller, less-bloated DNS replies, as large packets still break things in 2023.

If the next version of the spec adopts this, we'll revisit, but I feel like having two different versions of the key with the same effective key material, only one of which is known to work with all versions of the software, is a helpful thing.

sp-andwei commented 1 year ago

I'm a bit confused. OpenDKIM does accept keys in SubjectPublicKeyInfo format. Currently, this is the only format it accepts for RSA keys, making it one of those cited "most current implementations".

But libopendkim it does not accept keys in plain RSAPublicKey format, which is the one it 'MUST' understand according to the original RFC. This was the reason for my PR #51, which extends libopendkim to accept the RSAPublicKey format as well.

They messed up the RFC by giving examples in the Appendix producing key strings in SubjectPublicKeyInfo format as an example, contradicting what they put into the specification. Everyone implemented that and later they added the errata to state that SubjectPublicKeyInfo format MAY be presented as p= Tag in the DNS.