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

Fix up RHEL kickstarts #10499

Merged
merged 10 commits into from
May 4, 2023
Merged

Conversation

comps
Copy link
Collaborator

@comps comps commented Apr 26, 2023

Description:

Since these are 3 mostly independent commits, I'm copying out the commit descriptions here to give you an idea of what they do.


remove @Base from RHEL kickstarts

The @base group is normally hidden (does not show up in dnf group list) and generally meant to be used as a base for other, more visible, groups.

While the @base group itself is not deprecated, it likely makes more sense to just rely on the default, which is "Minimal install", a.k.a. @core, and let the users opt-into additional packages.

install RHEL stig_gui kickstarts with 'Server with GUI' packages

The @^graphical-server-environment is just an internal name for the human name, Server with GUI, which is consistent across RHEL releases.

The need for ^ on RHEL-7 is due to it being an "environment group" as opposed to a regular "package group". RHEL-8+ Anaconda does not have this distinction.

remove unnecessary --pesize from RHEL kickstarts

This is an optional optimization, setting the Physical Extent size of the LVM Volume Group, a.k.a. the smallest atomic object. When set properly, this can lessen the impact of LV fragmentation, but the default of 4MB is already reasonable.

The current --pesize=4096 doesn't even change this default, so literally nothing changes with this change.


There were more commits that I removed for one reason or another, noting here just FYI.

  • Making stig_gui compatible with 20GB VMs thanks to /var/log/audit using --size=512 --grow --maxsize=10240, but that only scales the partition in proportion to other partitions with --grow, meaning auditd_audispd_configure_sufficiently_large_partition would PASS only on cca 200GB+ disks, so I removed those changes.
  • Dropping --device eth0 because Anaconda's autodetection works really well, but the official docs still mention it should be explicitly specified, so I removed that change too.

Rationale:

  • Using @core (empty %packages) just seems like a more sane default. It is impossible to install just @base via Anaconda UI or other systems, meaning that configuration is likely very untested - instead, @base is included in ie. Virtualization Host, as part of a bigger package set.
  • RHEL-8 and 9, in contrast to RHEL-7, did not install Server with GUI group packages for stig_gui and they likely should have, so that's I believe a straight bug fix.
  • The --pesize removal is just removing a compliance-unrelated option and leaving defaults, reducing visual clutter.

The @base group is normally hidden (does not show up in
'dnf group list') and generally meant to be used as a base
for other, more visible, groups.

While the @base group itself is not deprecated, it likely
makes more sense to just rely on the default, which is
"Minimal install", a.k.a. @core, and let the users opt-into
additional packages.

Signed-off-by: Jiri Jaburek <comps@nomail.dom>
The '@^graphical-server-environment' is just an internal name for
the human name, 'Server with GUI', which is consistent across RHEL
releases.

The need for '^' on RHEL-7 is due to it being an "environment group"
as opposed to a regular "package group". RHEL-8+ Anaconda does not
have this distinction.

Signed-off-by: Jiri Jaburek <comps@nomail.dom>
This is an optional optimization, setting the Physical Extent
size of the LVM Volume Group, a.k.a. the smallest atomic object.
When set properly, this can lessen the impact of LV fragmentation,
but the default of 4MB is already reasonable.

The current --pesize=4096 doesn't even change this default,
so literally nothing changes with this change.

Signed-off-by: Jiri Jaburek <comps@nomail.dom>
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Apr 26, 2023
@openshift-ci
Copy link

openshift-ci bot commented Apr 26, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@github-actions
Copy link

Start a new ephemeral environment with changes proposed in this pull request:

Fedora Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

This is unnecessary - Anaconda automatically detects the type of
bootloader. In fact, the documentation clearly says that:

    In most cases, this option does not need to be specified.

Signed-off-by: Jiri Jaburek <comps@nomail.dom>
'rhgb' and 'quiet' are already added by Anaconda by default,
as its documentation mentions:

    The rhgb and quiet parameters are always used, even if
    you do not specify them here or do not use the --append=
    command at all.

And the 'crashkernel=auto' is already the default for RHEL-7+,
with specific sizes (Anaconda-generated) sometimes appearing
instead of 'auto', likely on low-memory systems, see also:
https://access.redhat.com/solutions/59432

In either case, we likely shouldn't touch the default.

In fact, specifying it actually reduces security somewhat, and
some profiles already disable kdump anyway.

Signed-off-by: Jiri Jaburek <comps@nomail.dom>
The Anaconda documentation, even RHEL-7, states that:

    Either the --vckeymap or the --xlayouts option must be used.

The older syntax was likely supported due to ancient RHEL
compatibility, but is no longer valid for recent Fedoras,
and therefore probably RHEL-10+.

Signed-off-by: Jiri Jaburek <comps@nomail.dom>
@comps comps changed the title Clean up RHEL kickstarts Fix up RHEL kickstarts Apr 27, 2023
@comps
Copy link
Collaborator Author

comps commented Apr 27, 2023

Okay, so once I started digging into kickstarts, it was clear they are more broken than I realized.

Still in Draft as I would like to test all (well, most) kickstarts on all supported RHELs.

Added a few more commits:


remove --location=mbr from RHEL kickstarts

This is unnecessary - Anaconda automatically detects the type of bootloader. In fact, the documentation clearly says that:

In most cases, this option does not need to be specified.

remove default --append arguments from RHEL kickstarts

rhgb and quiet are already added by Anaconda by default, as its documentation mentions:

The rhgb and quiet parameters are always used, even if
you do not specify them here or do not use the --append=
command at all.

And the crashkernel=auto is already the default for RHEL-7+, with specific sizes (Anaconda-generated) sometimes appearing instead of auto, likely on low-memory systems, see also: https://access.redhat.com/solutions/59432

In either case, we likely shouldn't touch the default.

In fact, specifying it actually reduces security somewhat, and some profiles already disable kdump anyway.

use keyboard --vckeymap in RHEL kickstarts

The Anaconda documentation, even RHEL-7, states that:

Either the --vckeymap or the --xlayouts option must be used.

The older syntax was likely supported due to ancient RHEL compatibility, but is no longer valid for recent Fedoras, and therefore probably RHEL-10+.

fix broken bootloader passwords in RHEL kickstarts

There's more layers of breakage.

First, the missing --iscrypted made Anaconda treat the provided crypt-style string as plaintext, resulting in the password never actually working.

Second, the referenced https://pykickstart.readthedocs.io/en/latest/commands.html#rootpw is not even for the right kickstart command (!!!), referencing rootpw instead of bootloader.

When this crypt-style password is used (correctly with --iscrypted), the installer actually catches this issue:

GRUB2 encrypted password must be in grub.pbkdf2 format.

The RHEL-7+ Anaconda documentation mentions the correct procedure:

To generate an encrypted password, use the grub2-mkpasswd-pbkdf2
command, enter the password you want to use, and copy the command's
output (the hash starting with grub.pbkdf2) into the Kickstart file.

Curiously enough, this was correctly done for ssg-rhel8-cis-ks.cfg, but it is broken everywhere else.

To make things consistent and avoid generating another salt, I have just copy/pasted the hashed password from ssg-rhel8-cis-ks.cfg to others.

remove wrong temporary password disablement in pci-dss RHEL kickstarts

This comes from #3660 which mentions RHBZ#1651624 (from 2018) as the reason.

That bugzilla was never valid - the original bootloader line used incorrect password syntax, likely copy/pasted from a SCAP Content kickstart.

All that was needed was to just use the correct syntax, so re-enable the grub2 password, using a hash identical to other kickstarts.

@comps
Copy link
Collaborator Author

comps commented Apr 27, 2023

Note that this is not the end of fixes, at least some of the CIS profiles are really non-functional as Anaconda aborts on OSCAP addon missing some partitions.

There's more layers of breakage.

First, the missing --iscrypted made Anaconda treat the provided
crypt-style string as plaintext, resulting in the password
never actually working.

Second, the referenced

    https://pykickstart.readthedocs.io/en/latest/commands.html#rootpw

is not even for the right kickstart command (!!!), referencing
'rootpw' instead of 'bootloader'.

When this crypt-style password is used (correctly with --iscrypted),
the installer actually catches this issue:

    GRUB2 encrypted password must be in grub.pbkdf2 format.

The RHEL-7+ Anaconda documentation mentions the correct procedure:

    To generate an encrypted password, use the grub2-mkpasswd-pbkdf2
    command, enter the password you want to use, and copy the command's
    output (the hash starting with grub.pbkdf2) into the Kickstart file.

Curiously enough, this was correctly done for 'ssg-rhel8-cis-ks.cfg',
but it is broken everywhere else.

To make things consistent and avoid generating another salt, I have
just copy/pasted the hashed password from 'ssg-rhel8-cis-ks.cfg'
to others.

Signed-off-by: Jiri Jaburek <comps@nomail.dom>
This comes from

    ComplianceAsCode#3660

which mentions RHBZ#1651624 (from 2018) as the reason.

That bugzilla was never valid - the original 'bootloader' line used
incorrect password syntax, likely copy/pasted from a SCAP Content
kickstart.

All that was needed was to just use the correct syntax,
so re-enable the grub2 password, using a hash identical to
other kickstarts.

Signed-off-by: Jiri Jaburek <comps@nomail.dom>
The new additions are based on the 'cis' (L2) profiles for each
respective RHEL major individually.

Signed-off-by: Jiri Jaburek <comps@nomail.dom>
@comps
Copy link
Collaborator Author

comps commented Apr 27, 2023

Two more:


add required partitions to CIS server/workstation l1 RHEL kickstarts

The new additions are based on the cis (L2) profiles for each respective RHEL major individually.

unify logvol naming in RHEL kickstarts

Get rid of the vague LogVol123 names, use something meaningful.

Done via an ad-hoc single-use script:

#!/usr/bin/python3

import re
import sys
from pathlib import Path

for fpath in sys.argv[1:]:
    p = Path(fpath)
    cont = p.read_text().split('\n')
    new = []
    for line in cont:
        if not line.startswith('logvol'):
            new.append(line)
            continue
        m = re.match('^logvol ([^ ]+)', line)
        if not m:
            new.append(line)
            continue
        if m.group(1) == '/':
            n = 'root'
        elif m.group(1) == 'swap':
            n = 'swap'
        else:
            n = m.group(1).replace('/', '')
        line = re.sub(' --name=[^ ]+', f' --name={n}', line)
        new.append(line)
    p.write_text('\n'.join(new))

Get rid of the vague LogVol123 names, use something meaningful.

Signed-off-by: Jiri Jaburek <comps@nomail.dom>
@comps
Copy link
Collaborator Author

comps commented Apr 27, 2023

Probably fixes #10024 (at least).

@codeclimate
Copy link

codeclimate bot commented Apr 27, 2023

Code Climate has analyzed commit ee32f2c and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 52.4% (0.0% change).

View more on Code Climate.

@comps
Copy link
Collaborator Author

comps commented Apr 28, 2023

Switching out of Draft because I managed to test these changes on all RHEL-7, RHEL-8 and RHEL-9 kickstarts, for all profiles that had ssg-rhel*-$profile-ks.cfg kickstarts, and I didn't find any regressions caused by this change.

Note that this PR doesn't make the kickstarts fully "clean" or "up-to-date", there are many small differences across RHELs that likely shouldn't be there (partition size differences, missing comments, network --device inconsistencies, the install keyword, etc.), but I figured this PR has to be cut off somewhere, or it could go on forever.

@comps comps marked this pull request as ready for review April 28, 2023 13:19
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Apr 28, 2023
@Mab879 Mab879 added this to the 0.1.68 milestone Apr 28, 2023
@Mab879 Mab879 added the Kickstart Kickstart file updates. label Apr 28, 2023
@Mab879 Mab879 self-assigned this May 3, 2023
@Mab879 Mab879 merged commit d83c581 into ComplianceAsCode:master May 4, 2023
@comps comps deleted the cleanup_kickstarts branch May 4, 2023 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Kickstart Kickstart file updates.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants