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

RSA Public Key Format Inconsistency #43

Closed
AGWA opened this issue Mar 15, 2021 · 4 comments · Fixed by #45
Closed

RSA Public Key Format Inconsistency #43

AGWA opened this issue Mar 15, 2021 · 4 comments · Fixed by #45

Comments

@AGWA
Copy link
Contributor

AGWA commented Mar 15, 2021

RFC 6376 Section 3.6.1 states that RSA public keys are encoded in the TXT record as "an ASN.1 DER-encoded [ITU-X660-1997] RSAPublicKey". This is the encoding produced by the dkim-keygen command.

Unfortunately, RFC 6376 contradicts itself in Appendix C, where it shows a public key in SubjectPublicKeyInfo format instead. Although Appendix C is only informative, it seems that other implementations have adopted it. opendkim's key generation tool uses SubjectPublicKeyInfo, as do the DKIM records for Gmail and Fastmail. Erratum 3017 has been filed against RFC 6376 proposing that both RSAPublicKey and SubjectPublicKeyInfo be allowed in the TXT record.

Currently, go-msgauth's DKIM verifier expects RSA public keys to use SubjectPublicKeyInfo format. If you try to verify a signature from a domain which uses RSAPublicKey format instead, you get this error:

dkim: key syntax error: x509: failed to parse public key (use ParsePKCS1PublicKey instead for this key format)

I propose the following fix:

  1. go-msgauth's verifier should accept both formats (i.e. it should try parsing the key with both x509.ParsePKIXPublicKey and x509.ParsePKCS1PublicKey) in accordance with Erratum 3017.
  2. dkim-keygen should produce SubjectPublicKeyInfo format. Given the use of SubjectPublicKeyInfo with providers like Gmail and Fastmail, there are probably more DKIM implementations that only accept SubjectPublicKeyInfo than implementations that only accept RSAPublicKey.

I'm happy to submit a PR for this.

@emersion
Copy link
Owner

Thanks for the detailed explanation. Looks like a good plan to me!

@emersion
Copy link
Owner

Though I've never hit this "failed to parse public key" error I think, even when verifying Gmail/Fastmail emails. I wonder why?

@AGWA
Copy link
Contributor Author

AGWA commented Mar 15, 2021

Though I've never hit this "failed to parse public key" error I think, even when verifying Gmail/Fastmail emails. I wonder why?

Gmail and Fastmail use SubjectPublicKeyInfo, which is what go-msgauth currently expects.

I'll work on a PR.

@emersion
Copy link
Owner

Oh, I see, makes sense.

AGWA added a commit to AGWA-forks/go-msgauth that referenced this issue Mar 15, 2021
RFC 6376 is inconsistent about whether RSA public keys should be
formatted as RSAPublicKey or SubjectPublicKeyInfo.  Erratum 3017
(https://www.rfc-editor.org/errata/eid3017) proposes allowing both.

This commit changes the verifier to accept both formats, and changes
dkim-keygen to generate keys in SubjectPublicKeyInfo format for
consistency with other implementations including opendkim, Gmail,
and Fastmail.

Closes: emersion#43
emersion pushed a commit that referenced this issue Mar 15, 2021
RFC 6376 is inconsistent about whether RSA public keys should be
formatted as RSAPublicKey or SubjectPublicKeyInfo.  Erratum 3017
(https://www.rfc-editor.org/errata/eid3017) proposes allowing both.

This commit changes the verifier to accept both formats, and changes
dkim-keygen to generate keys in SubjectPublicKeyInfo format for
consistency with other implementations including opendkim, Gmail,
and Fastmail.

Closes: #43
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 a pull request may close this issue.

2 participants