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

Make RSA key length configurable #506

Merged
merged 37 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
b2d77b7
make key length configurable
maltesander Sep 30, 2024
4e665b0
adapted changelog
maltesander Sep 30, 2024
9ff3c3c
rename key length enum variants
maltesander Sep 30, 2024
cec6a8d
cargo fmt
maltesander Sep 30, 2024
e7d344c
fix unit tests
maltesander Sep 30, 2024
c0f96c2
fmt
maltesander Sep 30, 2024
f2276c0
improve crd schema
maltesander Sep 30, 2024
b0d7ae8
add comments
maltesander Sep 30, 2024
4f0fe53
yaml lint
maltesander Sep 30, 2024
a89ecd4
add documentation
maltesander Sep 30, 2024
58f9773
improvements
maltesander Sep 30, 2024
abcd337
fixes
maltesander Sep 30, 2024
e89fc60
Apply suggestions from code review
maltesander Oct 1, 2024
dd123af
set default key length to 2048
maltesander Oct 1, 2024
9f00f66
adapt key length default and values
maltesander Oct 1, 2024
ed80854
use constants for key length
maltesander Oct 1, 2024
0fefa56
trailing whitespace
maltesander Oct 1, 2024
a4ee380
Apply suggestions from code review
maltesander Oct 1, 2024
43edafe
explicitly use serde default
maltesander Oct 1, 2024
0c8b9d1
Merge branch 'feat/make-tls-key-length-configurable' of github.com:st…
maltesander Oct 1, 2024
5fd3a46
improve docs
maltesander Oct 1, 2024
e048894
fix clippy /test
maltesander Oct 1, 2024
5506a75
improve docs
maltesander Oct 1, 2024
b1909e4
fmt
maltesander Oct 1, 2024
1c9c728
fix test
maltesander Oct 1, 2024
5873540
Update rust/operator-binary/src/crd.rs
maltesander Oct 2, 2024
24bf088
regenerate charts
maltesander Oct 2, 2024
ccbfa4b
bump tonic due to cargo deny
maltesander Oct 2, 2024
077d2bb
rename struct
maltesander Oct 2, 2024
85ab8c4
improve doc header
maltesander Oct 2, 2024
9a0b0b8
Update deploy/helm/secret-operator/crds/crds.yaml
maltesander Oct 2, 2024
610cac8
consistently switched to key pair instead of keypair
maltesander Oct 2, 2024
9b114c3
Merge branch 'feat/make-tls-key-length-configurable' of github.com:st…
maltesander Oct 2, 2024
a819d63
improve docs
maltesander Oct 2, 2024
18173d5
regenerate charts
maltesander Oct 2, 2024
2ea7649
fix
maltesander Oct 2, 2024
b36cb6d
fix
maltesander Oct 2, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ All notable changes to this project will be documented in this file.

- Active Directory's `samAccountName` generation can now be customized ([#454]).
- Added experimental cert-manager backend ([#482]).
- Make RSA key length configurable ([#506]).

### Fixed

Expand All @@ -25,6 +26,7 @@ All notable changes to this project will be documented in this file.
[#495]: https://github.com/stackabletech/secret-operator/pull/495
[#497]: https://github.com/stackabletech/secret-operator/pull/497
[#505]: https://github.com/stackabletech/secret-operator/pull/505
[#506]: https://github.com/stackabletech/secret-operator/pull/506

## [24.7.0] - 2024-07-24

Expand Down
30 changes: 11 additions & 19 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 23 additions & 1 deletion deploy/helm/secret-operator/crds/crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ spec:
description: |-
The [`autoTls` backend](https://docs.stackable.tech/home/nightly/secret-operator/secretclass#backend-autotls) issues a TLS certificate signed by the Secret Operator. The certificate authority can be provided by the administrator, or managed automatically by the Secret Operator.

A new certificate and keypair will be generated and signed for each Pod, keys or certificates are never reused.
A new certificate and key pair will be generated and signed for each Pod, keys or certificates are never reused.
properties:
ca:
description: Configures the certificate authority used to issue Pod certificates.
Expand All @@ -58,6 +58,28 @@ spec:

If `autoGenerate: true` then the Secret Operator will prepare a new CA certificate the old CA approaches expiration. If `autoGenerate: false` then the Secret Operator will log a warning instead.
type: string
keyGeneration:
default:
rsa:
length: 2048
description: The algorithm used to generate a key pair and required configuration settings. Currently only RSA and a key length of 2048, 3072 or 4096 bits can be configured.
oneOf:
- required:
- rsa
properties:
rsa:
properties:
length:
description: The amount of bits used for generating the RSA keypair. Currently, `2048`, `3072` and `4096` are supported. Defaults to `2048` bits.
enum:
- 2048
- 3072
- 4096
type: integer
required:
- length
type: object
type: object
secret:
description: Reference (name and namespace) to a Kubernetes Secret object where the CA certificate and key is stored in the keys `ca.crt` and `ca.key` respectively.
properties:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
# Migrates the TLS CA keypair from the hard-coded default namespace to the operator namespace
# Migrates the TLS CA key pair from the hard-coded default namespace to the operator namespace
# See https://github.com/stackabletech/secret-operator/issues/453
apiVersion: batch/v1
kind: Job
Expand Down Expand Up @@ -52,4 +52,4 @@ spec:
echo "$source_ca_secret" | jq 'del(.metadata["namespace","creationTimestamp","resourceVersion","selfLink","uid"])' | kubectl apply -n $TARGET_NAMESPACE -f -
fi
fi
restartPolicy: Never
restartPolicy: Never
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
apiVersion: secrets.stackable.tech/v1alpha1
kind: SecretClass
metadata:
name: tls
spec:
backend:
autoTls:
ca:
secret:
name: secret-provisioner-tls-ca
namespace: default
keyGeneration: # <1>
rsa: # <2>
length: 4096 # <3>
24 changes: 22 additions & 2 deletions docs/modules/secret-operator/pages/secretclass.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,33 @@ Each SecretClass is a associated with a single backend, which dictates the mecha
Issues a TLS certificate signed by the Secret Operator.
The certificate authority can be provided by the administrator, or managed automatically by the Secret Operator.

A new certificate and keypair will be generated and signed for each Pod, keys or certificates are never reused.
A new certificate and key pair will be generated and signed for each Pod, keys or certificates are never reused.

CAUTION: Attributes of the certificate (such as the expiration date, fingerprint, or serial number) will be regenerated for each
Pod, and should not be expected to be stable.

xref:scope.adoc[Scopes] are used to populate the claims (such as `subjectAlternateName`) of the provisioned certificates.

[#backend-tls-certificate-key-pair-generation]
=== `TLS certificate key pair generation`

Currently, only RSA is supported in the `autoTls` backend.
You can however configure the key length for generated private keys. If not specified it will default to `2048` bits.

----
include::example$secretclass-tls-key-length.yaml[]
----
<1> `autoTls.ca.keyGeneration` specifies which algorithm and additional parameters are used
<2> `autoTls.ca.keyGeneration.rsa` specifies the RSA key pair algorithm (RSA currently is the only one supported)
<3> `autoTls.ca.keyGeneration.rsa.length` specifies the amount of bits used for generating the RSA key pair. Currently, `2048`, `3072` and `4096` are supported. Defaults to `2048` bits.

CAUTION: Using more than `2048` bits will significantly increase the computation time to create new key pairs.
The SSL Labs https://github.com/ssllabs/research/wiki/SSL-and-TLS-Deployment-Best-Practices[SSL and TLS Deployment Best Practices] as of 2024-10-01 recommend

> For most websites, using RSA keys stronger than 2,048 bits and ECDSA keys stronger than 256 bits is a waste of CPU power and might impair user experience

If options higher than `2048` are chosen, the CPU resources for the secret operator should be increased in order to avoid Pods being stuck in `Pending` waiting for the computation of their key pair.

==== Certificate lifetime

By default the Secret Operator will generally aim to use as short-lived certificates as possible.
Expand Down Expand Up @@ -375,7 +395,7 @@ The secret contains the following files:
Both stores are encrypted, with an empty string as the passphrase.

NOTE: When using the xref:#backend-k8ssearch[] backend, it is _strongly_ recommended to store secrets in the use the xref:#format-tls-pem[] format instead.
The secret operator supports converting PEM keypairs into PKCS#12, but not the other way around.
The secret operator supports converting PEM key pairs into PKCS#12, but not the other way around.

[#format-kerberos]
=== Kerberos
Expand Down
2 changes: 1 addition & 1 deletion rust/operator-binary/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ fn main() {
let out_dir = PathBuf::from(std::env::var("OUT_DIR").expect("OUT_DIR is required"));
tonic_build::configure()
.file_descriptor_set_path(out_dir.join("file_descriptor_set.bin"))
.compile(&["vendor/csi/csi.proto"], &["vendor/csi"])
.compile_protos(&["vendor/csi/csi.proto"], &["vendor/csi"])
.unwrap();
built::write_built_file().unwrap();
}
11 changes: 10 additions & 1 deletion rust/operator-binary/src/backend/tls/ca.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use tracing::{info, info_span, warn};

use crate::{
backend::SecretBackendError,
crd::CertificateKeyGeneration,
utils::{asn1time_to_offsetdatetime, Asn1TimeParseError, Unloggable},
};

Expand Down Expand Up @@ -153,6 +154,9 @@ pub struct Config {
/// Hence, this value _should_ be larger than the PKI's maximum certificate lifetime,
/// and smaller than [`Self::ca_certificate_lifetime`].
pub rotate_if_ca_expires_before: Option<Duration>,

/// Configuration how TLS private keys should be created.
pub key_generation: CertificateKeyGeneration,
}

/// A single certificate authority certificate.
Expand Down Expand Up @@ -188,7 +192,12 @@ impl CertificateAuthority {
let not_before = now - Duration::from_minutes_unchecked(5);
let not_after = now + config.ca_certificate_lifetime;
let conf = Conf::new(ConfMethod::default()).unwrap();
let private_key = Rsa::generate(2048)

let private_key_length = match config.key_generation {
CertificateKeyGeneration::Rsa { length } => length,
};

let private_key = Rsa::generate(private_key_length)
.and_then(PKey::try_from)
.context(GenerateKeySnafu)?;
let certificate = X509Builder::new()
Expand Down
17 changes: 13 additions & 4 deletions rust/operator-binary/src/backend/tls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use stackable_operator::{
use time::OffsetDateTime;

use crate::{
crd,
crd::{self, CertificateKeyGeneration},
format::{well_known, SecretData, WellKnownSecretData},
utils::iterator_try_concat_bytes,
};
Expand Down Expand Up @@ -132,12 +132,13 @@ impl SecretBackendError for Error {
pub struct TlsGenerate {
ca_manager: ca::Manager,
max_cert_lifetime: Duration,
key_generation: CertificateKeyGeneration,
}

impl TlsGenerate {
/// Check if a signing CA has already been instantiated in a specified Kubernetes secret - if
/// one is found the key is loaded and used for signing certs.
/// If no current authority can be found, a new keypair and self signed certificate is created
/// If no current authority can be found, a new key pair and self signed certificate is created
/// and stored for future use.
/// This allows users to provide their own CA files, but also enables secret-operator to generate
/// an independent self-signed CA.
Expand All @@ -147,6 +148,7 @@ impl TlsGenerate {
secret: ca_secret,
auto_generate: auto_generate_ca,
ca_certificate_lifetime,
key_generation,
}: &crd::AutoTlsCa,
max_cert_lifetime: Duration,
) -> Result<Self> {
Expand All @@ -158,11 +160,13 @@ impl TlsGenerate {
manage_ca: *auto_generate_ca,
ca_certificate_lifetime: *ca_certificate_lifetime,
rotate_if_ca_expires_before: Some(*ca_certificate_lifetime / 2),
key_generation: key_generation.clone(),
},
)
.await
.context(LoadCaSnafu)?,
max_cert_lifetime,
key_generation: key_generation.clone(),
})
}
}
Expand All @@ -171,7 +175,7 @@ impl TlsGenerate {
impl SecretBackend for TlsGenerate {
type Error = Error;

/// Generate a keypair and sign it with the CA key.
/// Generate a key pair and sign it with the CA key.
/// Then add the ca certificate and return these files for provisioning to the volume.
async fn get_secret_data(
&self,
Expand Down Expand Up @@ -233,7 +237,12 @@ impl SecretBackend for TlsGenerate {
}

let conf = Conf::new(ConfMethod::default()).unwrap();
let pod_key = Rsa::generate(2048)

let pod_key_length = match self.key_generation {
CertificateKeyGeneration::Rsa { length } => length,
};

let pod_key = Rsa::generate(pod_key_length)
.and_then(PKey::try_from)
.context(GenerateKeySnafu)?;
let mut addresses = Vec::new();
Expand Down
Loading
Loading