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.

Sign up for GitHub

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

certificate propagation throughout the build process #647

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

certificate propagation throughout the build process #647

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# ----- EIB Builder Image -----
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no need to add the certificates as part of the EIB Dockerfile itself. The EIB Dockerfile is running a fixed set of configurations that are proven to successfully build an EIB container image.

Any configurations for EIB or its underlying RPM resolution process must be done only from EIB's image configuration directory.

Based on this I suggest we remove the certificate inclusion from here.

Copy link
Author

Choose a reason for hiding this comment

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

In my environment, the outgoing traffic is reencrypted at the edge and monitored, that means I have a cert that I need to add to the trust for building of the base EIB image. When using the upstream image it is pretty simple to just add my needed certs though.

Copy link
Author

Choose a reason for hiding this comment

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

basically an accepted MITM :)

FROM registry.suse.com/bci/golang:1.22-1.36.1

COPY certificates/. /etc/pki/trust/anchors/
RUN update-ca-certificates

# Dependency uses by line
# 1. Podman Go library
RUN zypper install -y \
Expand All @@ -20,6 +23,9 @@ RUN --mount=type=cache,id=gomod,target=/go/pkg/mod \
# ----- Deliverable Image -----
FROM opensuse/leap:15.6

COPY certificates/. /etc/pki/trust/anchors/
RUN update-ca-certificates

# Dependency uses by line
# 1. ISO image building
# 2. RAW image modification on x86_64 and aarch64
Expand Down
1 change: 1 addition & 0 deletions certificates/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
*
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as the comment above, can we remove this file and directory. EIB configurations should be done only from EIB's image configuration directory.

2 changes: 1 addition & 1 deletion pkg/combustion/combustion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ type kubernetesArtefactDownloader interface {
}

type rpmResolver interface {
Resolve(packages *image.Packages, localRPMConfig *image.LocalRPMConfig, outputDir string) (rpmDirPath string, pkgList []string, err error)
Resolve(packages *image.Packages, localRPMConfig *image.LocalRPMConfig, certsPath, outputDir string) (rpmDirPath string, pkgList []string, err error)
}

type rpmRepoCreator interface {
Expand Down
4 changes: 3 additions & 1 deletion pkg/combustion/rpm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,10 @@ func (c *Combustion) configureRPMs(ctx *image.Context) ([]string, error) {
return nil, fmt.Errorf("creating rpm artefacts path: %w", err)
}

certsPath := filepath.Join(ctx.ImageConfigDir, certsConfigDir)

log.Audit("Resolving package dependencies...")
repoPath, pkgsList, err := c.RPMResolver.Resolve(packages, localRPMConfig, artefactsPath)
repoPath, pkgsList, err := c.RPMResolver.Resolve(packages, localRPMConfig, certsPath, artefactsPath)
if err != nil {
log.AuditComponentFailed(rpmComponentName)
return nil, fmt.Errorf("resolving rpm/package dependencies: %w", err)
Expand Down
6 changes: 3 additions & 3 deletions pkg/combustion/rpm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ import (
)

type mockRPMResolver struct {
resolveFunc func(packages *image.Packages, localRPMConfig *image.LocalRPMConfig, outputDir string) (rpmDir string, pkgList []string, err error)
resolveFunc func(packages *image.Packages, localRPMConfig *image.LocalRPMConfig, certsPath, outputDir string) (rpmDir string, pkgList []string, err error)
Copy link
Contributor

Choose a reason for hiding this comment

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

This change needs to be reflected in the resolveFunc functions of the TestConfigureRPMs_ResolutionFailures and the TestConfigureRPMs_SuccessfulConfig test cases.

}

func (m mockRPMResolver) Resolve(packages *image.Packages, localRPMConfig *image.LocalRPMConfig, outputDir string) (rpmDir string, pkgList []string, err error) {
func (m mockRPMResolver) Resolve(packages *image.Packages, localRPMConfig *image.LocalRPMConfig, certsPath, outputDir string) (rpmDir string, pkgList []string, err error) {
if m.resolveFunc != nil {
return m.resolveFunc(packages, localRPMConfig, outputDir)
return m.resolveFunc(packages, localRPMConfig, certsPath, outputDir)
}

panic("not implemented")
Expand Down
38 changes: 35 additions & 3 deletions pkg/rpm/resolver/resolver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func New(workDir string, podman Podman, baseImageBuilder BaseResolverImageBuilde
// - localRPMConfig - configuration for locally provided RPMs
//
// - outputDir - directory in which the resolver will create a directory containing the resolved rpms.
func (r *Resolver) Resolve(packages *image.Packages, localRPMConfig *image.LocalRPMConfig, outputDir string) (rpmDirPath string, pkgList []string, err error) {
func (r *Resolver) Resolve(packages *image.Packages, localRPMConfig *image.LocalRPMConfig, certsPath, outputDir string) (rpmDirPath string, pkgList []string, err error) {
zap.L().Info("Resolving package dependencies...")

revert, err := mount.DisableDefaultMounts(r.overrideMountsPath)
Expand All @@ -98,7 +98,7 @@ func (r *Resolver) Resolve(packages *image.Packages, localRPMConfig *image.Local
return "", nil, fmt.Errorf("building base resolver image: %w", err)
}

if err = r.prepare(localRPMConfig, packages); err != nil {
if err = r.prepare(localRPMConfig, packages, certsPath); err != nil {
return "", nil, fmt.Errorf("generating context for the resolver image: %w", err)
}

Expand All @@ -122,7 +122,7 @@ func (r *Resolver) Resolve(packages *image.Packages, localRPMConfig *image.Local
return filepath.Join(outputDir, rpmRepoName), r.generatePKGInstallList(packages), nil
}

func (r *Resolver) prepare(localRPMConfig *image.LocalRPMConfig, packages *image.Packages) error {
func (r *Resolver) prepare(localRPMConfig *image.LocalRPMConfig, packages *image.Packages, certsPath string) error {
zap.L().Info("Preparing resolver image context...")

buildContext := r.generateBuildContextPath()
Expand All @@ -136,6 +136,10 @@ func (r *Resolver) prepare(localRPMConfig *image.LocalRPMConfig, packages *image
}
}

if err := r.prepareCertificates(certsPath); err != nil {
return fmt.Errorf("preparing certificates for resolver image build: %w", err)
}

if err := r.writeRPMResolutionScript(localRPMConfig, packages); err != nil {
return fmt.Errorf("writing rpm resolution script: %w", err)
}
Expand All @@ -148,6 +152,29 @@ func (r *Resolver) prepare(localRPMConfig *image.LocalRPMConfig, packages *image
return nil
}

func (r *Resolver) prepareCertificates(certsPath string) error {
certsDest := r.generateCertificatePathInBuildContext()

if err := os.MkdirAll(certsDest, os.ModePerm); err != nil {
return fmt.Errorf("creating certificates dir %s: %w", certsDest, err)
}

if _, err := os.Stat(certsPath); os.IsNotExist(err) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest moving the validation before the actual resolver-image-build/certificates directory creation. No need to create a directory if the root certificates directory is empty.

Also we could probably use the existing fileio.FileExists function here.

zap.S().Info("skipping certificates, no certificates provided")
return nil
}

if err := fileio.CopyFiles(certsPath, certsDest, ".crt", false, &fileio.NonExecutablePerms); err != nil {
return fmt.Errorf("copying crt files to %s: %w", certsDest, err)
}

if err := fileio.CopyFiles(certsPath, certsDest, ".pem", false, &fileio.NonExecutablePerms); err != nil {
return fmt.Errorf("copying pem files to %s: %w", certsDest, err)
}

return nil
}

func (r *Resolver) prepareLocalRPMs(localRPMConfig *image.LocalRPMConfig) error {
rpmDest := r.generateRPMPathInBuildContext()
if err := fileio.CopyFiles(localRPMConfig.RPMPath, rpmDest, ".rpm", false, &fileio.NonExecutablePerms); err != nil {
Expand Down Expand Up @@ -274,6 +301,11 @@ func (r *Resolver) generateBuildContextPath() string {
return filepath.Join(r.dir, "resolver-image-build")
}

// path to the certificates directory in the resolver build context, as seen in the EIB image
func (r *Resolver) generateCertificatePathInBuildContext() string {
return filepath.Join(r.generateBuildContextPath(), "certificates")
}

// path to the rpms directory in the resolver build context, as seen in the EIB image
func (r *Resolver) generateRPMPathInBuildContext() string {
return filepath.Join(r.generateBuildContextPath(), "rpms")
Expand Down
2 changes: 2 additions & 0 deletions pkg/rpm/resolver/templates/Dockerfile.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ COPY {{ .FromGPGPath }} {{ .ToGPGPath }}
{{ end -}}
{{ end }}

COPY certificates/. /etc/pki/trust/anchors/
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest to avoid hardcoding the path to the certificates directory in the Dockerfile.tpl file. Instead pass the path from the code and have a template here as it is for the other properties. That way if a change needs to happen it could only be done in a single place. Templating for the resolver's Dockerfile.tpl is happening in the writeDockerfile function.


RUN ./{{ .RPMResolutionScriptName }}

CMD ["/bin/bash"]
2 changes: 2 additions & 0 deletions pkg/rpm/resolver/templates/rpm-resolution.sh.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ set -euo pipefail
# Arch - sets the architecture of the rpm packages to pull
# EnableExtras - registers the SL-Micro-Extras repo for use in resolution

update-ca-certificates -v

{{ if ne .RegCode "" }}
suseconnect -r {{ .RegCode }}
{{ if $.EnableExtras -}}
Expand Down