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 28 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.

22 changes: 22 additions & 0 deletions deploy/helm/secret-operator/crds/crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 keypair 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 key or certs. Currently, `2048`, `3072` and `4096` are supported. Defaults to `2048` bits.
maltesander marked this conversation as resolved.
Show resolved Hide resolved
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
@@ -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>
20 changes: 20 additions & 0 deletions docs/modules/secret-operator/pages/secretclass.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,26 @@ CAUTION: Attributes of the certificate (such as the expiration date, fingerprint

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

[#backend-tls-key-pair-generation]
=== `TLS 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 key or certs. Currently, `2048`, `3072` and `4096` are supported. Defaults to `2048` bits.
Techassi marked this conversation as resolved.
Show resolved Hide resolved

CAUTION: Using a key length higher than `2048` will significantly increase the computation time to create new key-pairs.
Techassi marked this conversation as resolved.
Show resolved Hide resolved
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.
Techassi marked this conversation as resolved.
Show resolved Hide resolved

==== Certificate lifetime

By default the Secret Operator will generally aim to use as short-lived certificates as possible.
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::TlsKeyGeneration,
Techassi marked this conversation as resolved.
Show resolved Hide resolved
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: TlsKeyGeneration,
maltesander marked this conversation as resolved.
Show resolved Hide resolved
}

/// 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 {
TlsKeyGeneration::Rsa { length } => length,
};

let private_key = Rsa::generate(private_key_length)
.and_then(PKey::try_from)
.context(GenerateKeySnafu)?;
let certificate = X509Builder::new()
Expand Down
13 changes: 11 additions & 2 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, TlsKeyGeneration},
format::{well_known, SecretData, WellKnownSecretData},
utils::iterator_try_concat_bytes,
};
Expand Down Expand Up @@ -132,6 +132,7 @@ impl SecretBackendError for Error {
pub struct TlsGenerate {
ca_manager: ca::Manager,
max_cert_lifetime: Duration,
key_generation: TlsKeyGeneration,
}

impl TlsGenerate {
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 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 {
TlsKeyGeneration::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
71 changes: 69 additions & 2 deletions rust/operator-binary/src/crd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use serde::{Deserialize, Serialize};
use snafu::Snafu;
use stackable_operator::{
kube::CustomResource,
schemars::{self, JsonSchema},
schemars::{self, schema::Schema, JsonSchema},
time::Duration,
};
use stackable_secret_operator_crd_utils::SecretReference;
Expand Down Expand Up @@ -123,6 +123,11 @@ pub struct AutoTlsCa {
/// If `autoGenerate: false` then the Secret Operator will log a warning instead.
#[serde(default = "AutoTlsCa::default_ca_certificate_lifetime")]
pub ca_certificate_lifetime: Duration,

/// The algorithm used to generate a keypair and required configuration settings.
/// Currently only RSA and a key length of 2048, 3072 or 4096 bits can be configured.
#[serde(default)]
pub key_generation: TlsKeyGeneration,
}

impl AutoTlsCa {
Expand All @@ -131,6 +136,61 @@ impl AutoTlsCa {
}
}

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
#[serde(rename_all = "camelCase")]
pub enum TlsKeyGeneration {
Rsa {
maltesander marked this conversation as resolved.
Show resolved Hide resolved
/// The amount of bits used for key or certs. Currently, `2048`, `3072` and `4096` are
/// supported. Defaults to `2048` bits.
#[schemars(schema_with = "TlsKeyGeneration::tls_key_length_schema")]
length: u32,
},
}

impl TlsKeyGeneration {
pub const TLS_RSA_KEY_LENGTH_2048: u32 = 2048;
maltesander marked this conversation as resolved.
Show resolved Hide resolved
pub const TLS_RSA_KEY_LENGTH_3072: u32 = 3072;
pub const TLS_RSA_KEY_LENGTH_4096: u32 = 4096;

// Could not get a "standard" enum with assigned values/discriminants to work as integers in the schema
// The following was generated and requires the length to be provided as string (we want an integer)
// keyGeneration:
// default:
// rsa:
// length: '2048'
// oneOf:
// - required:
// - rsa
// properties:
// rsa:
// properties:
// length:
// enum:
// - '2048'
// - '3072'
// - '4096'
// type: string
pub fn tls_key_length_schema(_: &mut schemars::gen::SchemaGenerator) -> Schema {
serde_json::from_value(serde_json::json!({
"type": "integer",
"enum": [
Self::TLS_RSA_KEY_LENGTH_2048,
Self::TLS_RSA_KEY_LENGTH_3072,
Self::TLS_RSA_KEY_LENGTH_4096
]
}))
.expect("Failed to parse JSON of custom tls key length schema")
}
}

impl Default for TlsKeyGeneration {
fn default() -> Self {
Self::Rsa {
length: Self::TLS_RSA_KEY_LENGTH_2048,
}
}
}

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
#[serde(rename_all = "camelCase")]
pub struct CertManagerBackend {
Expand Down Expand Up @@ -368,6 +428,9 @@ mod test {
secret:
name: secret-provisioner-tls-ca
namespace: default
keyGeneration:
rsa:
length: 3072
Techassi marked this conversation as resolved.
Show resolved Hide resolved
"#;
let deserializer = serde_yaml::Deserializer::from_str(input);
let secret_class: SecretClass =
Expand All @@ -383,6 +446,9 @@ mod test {
},
auto_generate: false,
ca_certificate_lifetime: DEFAULT_CA_CERT_LIFETIME,
key_generation: TlsKeyGeneration::Rsa {
length: TlsKeyGeneration::TLS_RSA_KEY_LENGTH_3072
}
},
max_certificate_lifetime: DEFAULT_MAX_CERT_LIFETIME,
})
Expand Down Expand Up @@ -418,7 +484,8 @@ mod test {
namespace: "default".to_string(),
},
auto_generate: true,
ca_certificate_lifetime: Duration::from_days_unchecked(100)
ca_certificate_lifetime: Duration::from_days_unchecked(100),
key_generation: TlsKeyGeneration::default()
},
max_certificate_lifetime: Duration::from_days_unchecked(31),
})
Expand Down
Loading
Loading