Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
187 changes: 184 additions & 3 deletions pkg/controller/common/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"os"
"path/filepath"
"reflect"

"sort"
"strings"
"text/template"
Expand Down Expand Up @@ -759,7 +758,6 @@ func decompressPayload(r io.Reader) ([]byte, error) {
// Units have one exception: dropins are concat'ed

func removeIgnDuplicateFilesUnitsUsers(ignConfig ign2types.Config) (ign2types.Config, error) {

files := ignConfig.Storage.Files
units := ignConfig.Systemd.Units
users := ignConfig.Passwd.Users
Expand Down Expand Up @@ -1238,7 +1236,6 @@ func (n namespacedEventRecorder) AnnotatedEventf(object runtime.Object, annotati
func DoARebuild(pool *mcfgv1.MachineConfigPool) bool {
_, ok := pool.Labels[RebuildPoolLabel]
return ok

}

// isSubdirectory checks if targetPath is a subdirectory of dirPath.
Expand Down Expand Up @@ -1312,6 +1309,190 @@ func GetSecurityProfileCiphers(profile *configv1.TLSSecurityProfile) (string, []
return string(profileSpec.MinTLSVersion), crypto.OpenSSLToIANACipherSuites(profileSpec.Ciphers)
}

// cipherComponents holds the Fedora crypto-policy component names that an

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

suggestion (non-blocking): To keep the contents of this directory more organized, let's put this and the associated tests into a separate file called crypto.go or cryptopolicy.go / crypto_test.go or cryptopolicy_test.go, respectively.

// OpenSSL cipher suite decomposes into.
type cipherComponents struct {
cipher string // e.g. "AES-256-GCM"
mac string // e.g. "AEAD", "SHA256"
}

// opensslCipherInfo maps OpenSSL cipher suite names to their decomposed
// Fedora crypto-policy components. The source of truth for which OpenSSL
// cipher names exist is library-go's `openSSLToIANACiphers` map in
// github.com/openshift/library-go/pkg/crypto/crypto.go — this table must
// stay in sync with it.
var opensslCipherInfo = map[string]cipherComponents{
Comment on lines +1319 to +1324

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Note to reviewer: There is a plan to automate keeping this list up to date in https://redhat.atlassian.net/browse/MCO-2444.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The automation for this and the other automation comment seems like it could be pretty interesting!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I was thinking about this one. Could us use a go generator to inject them at build time instead of maintaing a job?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think that's a fair option, but I think then we'd need some helpers to automatically split the cipher into the policy components. Maybe I can schedule a meeting for us to chat about options here?

// TLS 1.3
"TLS_AES_128_GCM_SHA256": {cipher: "AES-128-GCM", mac: "AEAD"},
"TLS_AES_256_GCM_SHA384": {cipher: "AES-256-GCM", mac: "AEAD"},
"TLS_CHACHA20_POLY1305_SHA256": {cipher: "CHACHA20-POLY1305", mac: "AEAD"},
// TLS 1.2 ECDHE GCM
"ECDHE-ECDSA-AES128-GCM-SHA256": {cipher: "AES-128-GCM", mac: "AEAD"},
"ECDHE-RSA-AES128-GCM-SHA256": {cipher: "AES-128-GCM", mac: "AEAD"},
"ECDHE-ECDSA-AES256-GCM-SHA384": {cipher: "AES-256-GCM", mac: "AEAD"},
"ECDHE-RSA-AES256-GCM-SHA384": {cipher: "AES-256-GCM", mac: "AEAD"},
// TLS 1.2 ECDHE ChaCha20
"ECDHE-ECDSA-CHACHA20-POLY1305": {cipher: "CHACHA20-POLY1305", mac: "AEAD"},
"ECDHE-RSA-CHACHA20-POLY1305": {cipher: "CHACHA20-POLY1305", mac: "AEAD"},
// TLS 1.2 ECDHE CBC (SHA256/SHA384 variants require TLS 1.2 PRF)
"ECDHE-ECDSA-AES128-SHA256": {cipher: "AES-128-CBC", mac: "HMAC-SHA2-256"},
"ECDHE-RSA-AES128-SHA256": {cipher: "AES-128-CBC", mac: "HMAC-SHA2-256"},
"ECDHE-ECDSA-AES256-SHA384": {cipher: "AES-256-CBC", mac: "HMAC-SHA2-384"}, // from ciphersUnsupportedByGo
"ECDHE-RSA-AES256-SHA384": {cipher: "AES-256-CBC", mac: "HMAC-SHA2-384"}, // from ciphersUnsupportedByGo
// TLS 1.0 ECDHE CBC (predate TLS 1.2 but usable with it)
"ECDHE-ECDSA-AES128-SHA": {cipher: "AES-128-CBC", mac: "HMAC-SHA1"},
"ECDHE-RSA-AES128-SHA": {cipher: "AES-128-CBC", mac: "HMAC-SHA1"},
"ECDHE-ECDSA-AES256-SHA": {cipher: "AES-256-CBC", mac: "HMAC-SHA1"},
"ECDHE-RSA-AES256-SHA": {cipher: "AES-256-CBC", mac: "HMAC-SHA1"},
// TLS 1.2 RSA key exchange
"AES128-GCM-SHA256": {cipher: "AES-128-GCM", mac: "AEAD"},
"AES256-GCM-SHA384": {cipher: "AES-256-GCM", mac: "AEAD"},
"AES128-SHA256": {cipher: "AES-128-CBC", mac: "HMAC-SHA2-256"},
"AES256-SHA256": {cipher: "AES-256-CBC", mac: "HMAC-SHA2-256"}, // from ciphersUnsupportedByGo
// TLS 1.0 RSA key exchange (predate TLS 1.2 but usable with it)
"AES128-SHA": {cipher: "AES-128-CBC", mac: "HMAC-SHA1"},
"AES256-SHA": {cipher: "AES-256-CBC", mac: "HMAC-SHA1"},
// Legacy (3DES is removed from OpenSSL on RHCOS but harmless in the .pmod)
"DES-CBC3-SHA": {cipher: "3DES-CBC", mac: "HMAC-SHA1"},
"ECDHE-RSA-DES-CBC3-SHA": {cipher: "3DES-CBC", mac: "HMAC-SHA1"},
}

// protocolVersionsBelowMinimum maps a TLS version to the protocol versions
// that must be removed from the base policy via subtractive syntax (-VERSION).
// This approach preserves any protocol versions in the base policy that are at
// or above the minimum (including DTLS peers), and automatically inherits new
// versions (e.g. DTLS1.3) when they appear in future base policies.
// TLS 1.0 and 1.1 are clamped to TLS 1.2 because RHCOS cannot deliver them:
// OpenSSL 3.x enforces @SECLEVEL=2 which forbids TLS < 1.2.
var protocolVersionsBelowMinimum = map[configv1.TLSProtocolVersion][]string{
configv1.VersionTLS10: {"TLS1.0", "TLS1.1", "DTLS1.0"},
configv1.VersionTLS11: {"TLS1.0", "TLS1.1", "DTLS1.0"},
configv1.VersionTLS12: {"TLS1.0", "TLS1.1", "DTLS1.0"},
configv1.VersionTLS13: {"TLS1.0", "TLS1.1", "TLS1.2", "DTLS1.0", "DTLS1.2"},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ack: Seems fine to have DTLS1.2 as discussed to allow DTLS1.3 to show up if added in Fedora.

}

// tlsVersionsClamped contains TLS versions that are requested but cannot be
// delivered on RHCOS, used to emit a warning log.
var tlsVersionsClamped = map[configv1.TLSProtocolVersion]bool{
configv1.VersionTLS10: true,
configv1.VersionTLS11: true,
}

// tlsGroupToCryptoPolicy maps OpenShift TLSGroup enum values to Fedora
// crypto-policy group names. The canonical names are in the .pol files at
// https://gitlab.com/redhat-crypto/fedora-crypto-policies/-/tree/master/policies
var tlsGroupToCryptoPolicy = map[configv1.TLSGroup]string{

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Note to reviewer: There is a plan to automate keeping this list up to date in https://redhat.atlassian.net/browse/MCO-2445.

configv1.TLSGroupX25519: "X25519",
configv1.TLSGroupSecP256r1: "SECP256R1",
configv1.TLSGroupSecP384r1: "SECP384R1",
configv1.TLSGroupSecP521r1: "SECP521R1",
configv1.TLSGroupX25519MLKEM768: "MLKEM768-X25519",
configv1.TLSGroupSecP256r1MLKEM768: "P256-MLKEM768",
configv1.TLSGroupSecP384r1MLKEM1024: "P384-MLKEM1024",
}

// buildCustomSubPolicy generates the content of a .pmod file from a custom
// TLS profile spec. Cipher, MAC, and group directives use override syntax;
// protocol uses subtractive syntax (-VERSION) to preserve base policy DTLS.
func buildCustomSubPolicy(spec *configv1.TLSProfileSpec) string {
cipherSet := make(map[string]struct{})
macSet := make(map[string]struct{})

for _, c := range spec.Ciphers {
info, ok := opensslCipherInfo[c]
if !ok {
klog.Warningf("Unknown cipher %q in custom TLS profile, skipping crypto-policy mapping", c)
continue
}
cipherSet[info.cipher] = struct{}{}
macSet[info.mac] = struct{}{}
}

var lines []string

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thought (non-blocking): Whenever I construct a multi-line string like this, I like to use the strings.Builder object in the stdlib. My reasoning is because it allows me to use the fmt.Fprint(), fmt.Fprintf(), and fmt.Fprintln() functions along with format directives in a clear way.

Just to be clear: I'm not asking you to change this, I'm just pointing it out for the future 😄.


if len(cipherSet) > 0 {
lines = append(lines, "cipher@TLS = "+sortedKeys(cipherSet))
}
if len(macSet) > 0 {
lines = append(lines, "mac@TLS = "+sortedKeys(macSet))
}
if toRemove, ok := protocolVersionsBelowMinimum[spec.MinTLSVersion]; ok {
if tlsVersionsClamped[spec.MinTLSVersion] {
klog.Warningf("TLS profile requests %s but RHCOS enforces TLS 1.2 minimum; clamping protocol@TLS to TLS1.2+", spec.MinTLSVersion)
}
var removals []string
for _, v := range toRemove {
removals = append(removals, "-"+v)
}
lines = append(lines, "protocol@TLS = "+strings.Join(removals, " "))
}
if len(spec.Groups) > 0 {
var groups []string
for _, g := range spec.Groups {
if cpName, ok := tlsGroupToCryptoPolicy[g]; ok {
groups = append(groups, cpName)
}
}
if len(groups) > 0 {
lines = append(lines, "group@TLS = "+strings.Join(groups, " "))
}
}

return strings.Join(lines, "\n")
}

func sortedKeys(m map[string]struct{}) string {
keys := make([]string, 0, len(m))
for k := range m {
keys = append(keys, k)
}
sort.Strings(keys)
return strings.Join(keys, " ")
}

const (
cryptoPolicyDefault = "DEFAULT"
cryptoPolicyDefaultOpenShift = "DEFAULT:OPENSHIFT"
cryptoPolicyLegacy = "LEGACY"
cryptoPolicyLegacyOpenShift = "LEGACY:OPENSHIFT"
)

// GetCryptoPolicyFromTLSProfile maps an OpenShift TLS security profile to a
// Fedora crypto-policy name and optional sub-policy module content.
func GetCryptoPolicyFromTLSProfile(profile *configv1.TLSSecurityProfile) (string, string) {
profileType := configv1.TLSProfileIntermediateType
if profile != nil {
profileType = profile.Type
}

switch profileType {
case configv1.TLSProfileModernType:
spec := configv1.TLSProfiles[configv1.TLSProfileModernType]
content := buildCustomSubPolicy(spec)
if content != "" {
return cryptoPolicyDefaultOpenShift, content
}
return cryptoPolicyDefault, ""
case configv1.TLSProfileOldType:
spec := configv1.TLSProfiles[configv1.TLSProfileOldType]
content := buildCustomSubPolicy(spec)
if content != "" {
return cryptoPolicyLegacyOpenShift, content
}
return cryptoPolicyLegacy, ""
case configv1.TLSProfileCustomType:
if profile.Custom != nil {
content := buildCustomSubPolicy(&profile.Custom.TLSProfileSpec)
if content != "" {
return cryptoPolicyDefaultOpenShift, content
}
}
return cryptoPolicyDefault, ""
default:
return cryptoPolicyDefault, ""
}
}

Comment thread
coderabbitai[bot] marked this conversation as resolved.
// Converts tlsMinVersion and tlscipherSuites flags to a tlsConfig object that is used
// by the http.Server() call used in apiserver.NewAPIServer() & apiserver.Serve()
//
Expand Down
Loading