vmware-tanzu / pinniped

Pinniped is the easy, secure way to log in to your Kubernetes clusters.
https://pinniped.dev
Apache License 2.0
541 stars 65 forks source link

Allow user-limited TLS ciphers for TLS1.2 #1927

Closed joshuatcasey closed 3 months ago

joshuatcasey commented 4 months ago

Allow user-limited TLS ciphers for TLS1.2

Resolves #1605

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 96.89441% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 29.50%. Comparing base (d7849c7) to head (a64cf4a).

Files Patch % Lines
internal/crypto/ptls/config.go 95.71% 2 Missing and 1 partial :warning:
internal/concierge/server/server.go 0.00% 1 Missing :warning:
internal/supervisor/server/server.go 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1927 +/- ## ========================================== + Coverage 29.39% 29.50% +0.10% ========================================== Files 349 350 +1 Lines 58484 58575 +91 ========================================== + Hits 17193 17280 +87 - Misses 40777 40779 +2 - Partials 514 516 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

cfryanr commented 4 months ago

We want this feature in the Concierge too, right? So far, this PR seems to only focus on the Supervisor.

cfryanr commented 4 months ago

I feel like we should figure out if we can integration test this feature. There are already integrations tests about which ciphers we allow for both FIPS builds and non-FIPS builds. Although this might be tricky because the ConfigMap is only ready when the pods are restarted, so the test might need to restart the pods (similar to the TestPodShutdown_Disruptive).

joshuatcasey commented 4 months ago

Force-pushed to 2975d2a0 just to keep ahead of main@545df755, no PR feedback was addressed in that update.

joshuatcasey commented 4 months ago

Force-pushed to 40519bd1 to keep ahead of main and to resolve the godoc feedback above.

joshuatcasey commented 4 months ago

We want this feature in the Concierge too, right? So far, this PR seems to only focus on the Supervisor.

Yes. Will update with this feature exposed through the Concierge.

EDIT: pushed

joshuatcasey commented 3 months ago

Let's use these terms:

Take a look at making sure that FIPS code is minimal and business logic should be in common code (FIPS and non-FIPS).

joshuatcasey commented 3 months ago

Closing in favor of #1952