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

Add Secure Connections Standard #548

Merged
merged 57 commits into from
Nov 25, 2024
Merged

Add Secure Connections Standard #548

merged 57 commits into from
Nov 25, 2024

Conversation

markus-hentsch
Copy link
Contributor

Closes #547

@markus-hentsch markus-hentsch added the SCS-VP10 Related to tender lot SCS-VP10 label Apr 4, 2024
@markus-hentsch markus-hentsch changed the title Add Secure Communication Standard Add Secure Connections Standard Apr 4, 2024
Copy link
Contributor

@josephineSei josephineSei left a comment

Choose a reason for hiding this comment

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

nice first draft - I have a few questions inline

Standards/scs-0114-v1-secure-connections.md Outdated Show resolved Hide resolved
Standards/scs-0114-v1-secure-connections.md Outdated Show resolved Hide resolved
Standards/scs-0114-v1-secure-connections.md Outdated Show resolved Hide resolved
Standards/scs-0114-v1-secure-connections.md Outdated Show resolved Hide resolved
Standards/scs-0114-v1-secure-connections.md Outdated Show resolved Hide resolved
Standards/scs-0114-v1-secure-connections.md Outdated Show resolved Hide resolved
@markus-hentsch markus-hentsch marked this pull request as ready for review April 9, 2024 12:25
@bitkeks bitkeks self-requested a review April 10, 2024 08:38
@martinmo
Copy link
Member

@markus-hentsch I have general remark: Because TLS configuration and security is a moving target, have you considered to base the recommended configuration on one of the profiles offered by Mozilla SSL? For example, the "intermediate" profile, see https://ssl-config.mozilla.org/ and https://wiki.mozilla.org/Security/Server_Side_TLS. (AFAIK, these can be checked with sslyze.)

@artificial-intelligence
Copy link
Contributor

@markus-hentsch I have general remark: Because TLS configuration and security is a moving target, have you considered to base the recommended configuration on one of the profiles offered by Mozilla SSL? For example, the "intermediate" profile, see https://ssl-config.mozilla.org/ and https://wiki.mozilla.org/Security/Server_Side_TLS. (AFAIK, these can be checked with sslyze.)

lol, I had the same idea and actually checked our haproxy TLS implementation, seems there is some opportunity to do some hardening there:

COMPLIANCE AGAINST MOZILLA TLS CONFIGURATION
 --------------------------------------------

    Checking results against Mozilla's "MozillaTlsConfigurationEnum.INTERMEDIATE" configuration. See https://ssl-config.mozilla.org/ for more details.

    a.regiocloud.tech:443: FAILED - Not compliant.
        * ciphers: Cipher suites {'TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA', 'TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384', 'TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA', 'TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256'} are supported, but should be rejected.

@artificial-intelligence
Copy link
Contributor

working on a fix for upstream: https://bugs.launchpad.net/kolla-ansible/+bug/2060787

@anjastrunk anjastrunk self-requested a review April 15, 2024 07:21
Copy link
Contributor

@anjastrunk anjastrunk left a comment

Choose a reason for hiding this comment

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

Just some minor cosmetic changes:

  • Is prefer to write Cloud Service Provider instead of CSP, as "CSP" is not an official abbreviation
  • I do not like writing "SCS proposes..", "SCS decides...". AFAIK, SCS stands for Sovereign Cloud Stack, which is a Software Stack, which cannot decide something. I prefer to write "SCS project" or "SCS community"

But again. This is just cosmetics.

@markus-hentsch
Copy link
Contributor Author

Just some minor cosmetic changes:

* Is prefer to write Cloud Service Provider instead of CSP, as "CSP" is not an official abbreviation

* I do not like writing "SCS proposes..", "SCS decides...". AFAIK, SCS stands for Sovereign Cloud Stack, which is a Software Stack, which cannot decide something. I prefer to write "SCS project" or "SCS community"

But again. This is just cosmetics.

I adjusted the SCS references. I left "CSP" as-is and added a glossary instead, like I did with some other standards. We seem to use CSP a lot in other standards so I'd like to stay consistent. The glossary at the top should introduce the abbreviation sufficiently now.

Copy link
Contributor

@josephineSei josephineSei left a comment

Choose a reason for hiding this comment

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

Overall I think this is a good starting point. We need something for the manual audits, but this may something to discuss in the standards SIG or in the IAM and Security Meeting

Standards/scs-0114-v1-secure-connections.md Outdated Show resolved Hide resolved
Standards/scs-0114-v1-secure-connections.md Outdated Show resolved Hide resolved
Copy link
Contributor

@artificial-intelligence artificial-intelligence left a comment

Choose a reason for hiding this comment

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

Thanks for all the work done so far on this.

Unfortunately I think there's still some work left to do.

Thanks.

Standards/scs-01XX-v1-secure-connections.md Outdated Show resolved Hide resolved
Standards/scs-01XX-v1-secure-connections.md Outdated Show resolved Hide resolved
Standards/scs-01XX-v1-secure-connections.md Outdated Show resolved Hide resolved
Standards/scs-01XX-v1-secure-connections.md Outdated Show resolved Hide resolved
Standards/scs-01XX-v1-secure-connections.md Outdated Show resolved Hide resolved
Standards/scs-01XX-v1-secure-connections.md Outdated Show resolved Hide resolved
Standards/scs-01XX-v1-secure-connections.md Outdated Show resolved Hide resolved
Standards/scs-01XX-v1-secure-connections.md Outdated Show resolved Hide resolved
Standards/scs-01XX-v1-secure-connections.md Outdated Show resolved Hide resolved
Standards/scs-01XX-v1-secure-connections.md Outdated Show resolved Hide resolved
@markus-hentsch
Copy link
Contributor Author

Updated standard and test script to use the Mozilla TLS "intermediate" preset now.

Copy link
Contributor

@josephineSei josephineSei left a comment

Choose a reason for hiding this comment

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

The only thing left to consider, would be to convert the mentioning of OpenStack services to IaaS services. But as all tests we have written do always connect to OpenStack, I don't think that change would be necessary.

@anjastrunk anjastrunk self-assigned this Nov 6, 2024
@mbuechse
Copy link
Contributor

mbuechse commented Nov 6, 2024

The markdown lint issues should be addressed. The link checker issues should solve themselves when this is merged.

@anjastrunk
Copy link
Contributor

I removed the markdown linter errors. The only remaining errors coming from markdown link checker, who complain about the following two dead links:

The files are part if this PR and are not yet there.

@mbuechse Any idea how to fix this? Otherwise, I will merge PR anyway, as we have three approvals and all other checks were successfully.

@bitkeks bitkeks removed their request for review November 25, 2024 11:21
@anjastrunk
Copy link
Contributor

I removed the markdown linter errors. The only remaining errors coming from markdown link checker, who complain about the following two dead links:

* https://github.com/SovereignCloudStack/standards/blob/main/Tests/iaas/secure-connections/tls-checker.py → Status: 404

* https://github.com/SovereignCloudStack/standards/blob/main/Tests/iaas/secure-connections/README.md → Status: 404

The files are part if this PR and are not yet there.

@mbuechse Any idea how to fix this? Otherwise, I will merge PR anyway, as we have three approvals and all other checks were successfully.

I tried to replace absolute links with relative ones, to satisfy markdown link checker. This worked fine, but case markdown linter to fail, as relative links are not allowed. I reverted relative links and decided to merge this PR even markdown link checker fails. As dead links related to files added by this PR, merging will not break repo.

@anjastrunk anjastrunk merged commit c3dd463 into main Nov 25, 2024
6 of 7 checks passed
@anjastrunk anjastrunk deleted the feat/secure-communication branch November 25, 2024 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SCS-VP10 Related to tender lot SCS-VP10
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Secure communication standard for IaaS infastructure
7 participants