Skip to content

Conversation

comawill
Copy link
Contributor

@comawill comawill commented Feb 8, 2022

https://wearezeta.atlassian.net/browse/FS-444

Checklist

  • The PR Title explains the impact of the change.
  • The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
  • If HTTP endpoint paths have been added or renamed, the endpoint / config-flag checklist (see Wire-employee only backend wiki page) has been followed.
  • If a cassandra schema migration has been added, I ran make git-add-cassandra-schema to update the cassandra schema documentation.
  • changelog.d contains the following bits of information (details):
    • A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.

Copy link
Member

@jschaul jschaul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a one liner to the changelog? Something like

echo "Change the default set of TLS ciphers (both for the client and the federation APIs) to be compliant to the recommendations of TR-02102-2." > ./changelog.d/0-release-notes/TR-02102-2
git add ./changelog.d/0-release-notes/TR-02102-2

Otherwise looks good! 🚀

TLS.cipher_ECDHE_RSA_CHACHA20POLY1305_SHA256
TLS.cipher_ECDHE_RSA_AES128GCM_SHA256
-- TLS.cipher_ECDHE_ECDSA_CHACHA20POLY1305_SHA256, -- not compliant to TR-02102-2
-- TLS.cipher_ECDHE_RSA_CHACHA20POLY1305_SHA256 -- not compliant to TR-02102-2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you entirely remove the lines of code, rather than leaving them commented out?

Also, if this applies a general recommendation, could you link to the recommendation (perhaps this is about https://www.bsi.bund.de/SharedDocs/Downloads/EN/BSI/Publications/TechGuidelines/TG02102/BSI-TR-02102-2.html ?) in a comment above the blessedCiphers definition for future reference?

Copy link
Member

@jschaul jschaul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some comments linking to the relevant docs.

@jschaul jschaul merged commit 6972e3d into develop Feb 16, 2022
@jschaul jschaul deleted the comawill/tr02102-2 branch February 16, 2022 16:44
@fisx fisx mentioned this pull request Feb 18, 2022
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.

2 participants