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

put tls cipher info into Received header according to rfc8314 #2903

Closed
celesteking opened this issue Jan 14, 2021 · 2 comments · Fixed by #2916
Closed

put tls cipher info into Received header according to rfc8314 #2903

celesteking opened this issue Jan 14, 2021 · 2 comments · Fixed by #2916

Comments

@celesteking
Copy link
Contributor

celesteking commented Jan 14, 2021

Instead of inscribing TLS cipher info into comment like it's done now, follow this new RFC instead and put it into tls clause. This will probably require:

  • grooming ciphersuite names from openssl's format into IANA's if nodejs doesn't support it by other means.

Before: (cipher=ECDHE-RSA-AES256-GCM-SHA384)
After: tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 . (not sure if dh group could possibly be ever extracted)

@Juerd
Copy link
Collaborator

Juerd commented Jan 24, 2021

Node.js from v12.16 and v13.4 support cipher.standardName in addition to cipher.name to get the IANA formatted cipher name. Since Haraka already specifies >= 12.20.1, it is already available.

I think adhering to the RFC is a good idea, even if this means we lose the additional information that Haraka does now log (TLS version and whether (client?) verification succeeded). The additional information, including the embedded value of authheader does not seem to be compliant along the Stamp EBNF in RFC 5321 section 4.4; maybe this is a good opportunity to remove that as well. I'm not sure about envelope-from which is formatted like an additional clause but I can't find whether it's actually registered.

@msimerson
Copy link
Member

I was planning to lower the min node version back to 10 as we have resolved the issue requiring the bump. It would be nice if a PR implementing this is added progressively.

Juerd added a commit to Juerd/Haraka that referenced this issue Jan 25, 2021
Juerd added a commit to Juerd/Haraka that referenced this issue Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants