Skip to content
Draft
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
6 changes: 5 additions & 1 deletion config/v3_6_experimental/translate/translate.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ func Translate(old old_types.Config) (ret types.Config) {
tr.AddCustomTranslator(translateIgnition)
tr.AddCustomTranslator(translateDirectoryEmbedded1)
tr.AddCustomTranslator(translateFileEmbedded1)
tr.Translate(&old, &ret)
tr.Translate(&old.Ignition, &ret.Ignition)
tr.Translate(&old.KernelArguments, &ret.KernelArguments)
tr.Translate(&old.Passwd, &ret.Passwd)
tr.Translate(&old.Storage, &ret.Storage)
tr.Translate(&old.Systemd, &ret.Systemd)
return
}
8 changes: 1 addition & 7 deletions config/v3_6_experimental/types/clevis.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,7 @@ func (cu ClevisCustom) Validate(c path.ContextPath) (r report.Report) {
if util.NilOrEmpty(cu.Pin) && util.NilOrEmpty(cu.Config) && !util.IsTrue(cu.NeedsNetwork) {
return
}
if util.NotEmpty(cu.Pin) {
switch *cu.Pin {
case "tpm2", "tang", "sss":
default:
r.AddOnError(c.Append("pin"), errors.ErrUnknownClevisPin)
}
} else {
if util.NilOrEmpty(cu.Pin) {
r.AddOnError(c.Append("pin"), errors.ErrClevisPinRequired)
Comment on lines +36 to 37

Choose a reason for hiding this comment

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

medium

If cu.Pin is nil or empty, the validation logic adds an error indicating that a Clevis pin is required. However, the code does not validate the value of cu.Pin against a list of known pins, potentially leading to runtime errors if an invalid pin is provided. It's better to validate the pin against known values to provide early feedback to the user.

Consider adding a validation step to ensure that the pin value is one of the supported types (e.g., "tpm2", "tang", "sss").

}
if util.NilOrEmpty(cu.Config) {
Expand Down
2 changes: 1 addition & 1 deletion config/v3_6_experimental/types/clevis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TestClevisCustomValidate(t *testing.T) {
Pin: util.StrToPtr("z"),
},
at: path.New("", "pin"),
out: errors.ErrUnknownClevisPin,
out: nil,
},
{
in: ClevisCustom{
Expand Down
14 changes: 14 additions & 0 deletions config/v3_6_experimental/types/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ type Config struct {
Passwd Passwd `json:"passwd,omitempty"`
Storage Storage `json:"storage,omitempty"`
Systemd Systemd `json:"systemd,omitempty"`
Attestation Attestation `json:"attestation,omitempty"`
}

type Device string
Expand Down Expand Up @@ -262,3 +263,16 @@ type Unit struct {
type Verification struct {
Hash *string `json:"hash,omitempty"`
}

type Attestation struct {
AttestationKey AttestationKey `json:"attestation_key,omitempty"`
}

type AttestationKey struct {
Registration Registration `json:"registration,omitempty"`
}

type Registration struct {
Url *string `json:"url,omitempty"`
Certificate *string `json:"certificat,omitempty"`

Choose a reason for hiding this comment

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

high

The field name certificat in the Registration struct has a typo. It should be certificate to match the intended meaning and avoid confusion.

Suggested change
Certificate *string `json:"certificat,omitempty"`
Certificate *string `json:"certificate,omitempty"`

}
234 changes: 234 additions & 0 deletions internal/attestation/attestation.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,234 @@
// Copyright 2025 CoreOS, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package attestation

import (
"bytes"
"crypto/tls"
"crypto/x509"
"encoding/json"
"encoding/pem"
"errors"
"fmt"
"net"
"net/http"
"os"
"os/exec"
"syscall"
"time"

"github.com/coreos/ignition/v2/config/util"
"github.com/coreos/ignition/v2/config/v3_6_experimental/types"
"github.com/coreos/ignition/v2/internal/log"
"github.com/coreos/ignition/v2/internal/resource"
)

const (
TPMDir = "/var/tpm"
AKPath = "/var/tpm/ak.pub"
AKCtxPath = "/var/tpm/ak.ctx"
AKRegisterd = "/var/tpm/ak.registerd"
AKHandle = "0x81010002"
EKHandle = "0x81010001"
)

func HandleAttestation(logger *log.Logger, cfg *types.Config, platformName string, needNetPath string) error {
if !util.NilOrEmpty(cfg.Attestation.AttestationKey.Registration.Url) {
// Generate and persist the AK
if err := GenerateAndPersistAK(logger); err != nil {
return err
}

attestationKeyBytes, err := os.ReadFile(AKPath)
if err != nil {
return err
}
attestationKey := string(attestationKeyBytes)

// Check if the neednet file exists to determine our retry behavior
_, needNetErr := os.Stat(needNetPath)
needNetExists := (needNetErr == nil)
if needNetExists {
logger.Info("neednet file exists, network should be available for attestation")
} else {
logger.Info("neednet file does not exist, will return ErrNeedNet if network is unavailable")
}

err = AttestationKeyRegistration(logger, cfg.Attestation.AttestationKey.Registration,
attestationKey, platformName)
if err != nil {
// If we got ErrNeedNet and the neednet file doesn't exist, propagate it
// (we're in fetch-offline and need to signal for network)
if err == resource.ErrNeedNet && !needNetExists {
return err
}
// If we got ErrNeedNet but neednet file exists, we're in fetch stage
// Retry the registration with delays to allow network to come up
if err == resource.ErrNeedNet && needNetExists {
logger.Info("Network not ready yet in fetch stage, retrying with delays...")
// Retry up to 10 times with increasing delays
maxRetries := 20
for attempt := 2; attempt <= maxRetries; attempt++ {
delay := time.Duration(min(attempt*2, 10)) * time.Second
logger.Info("Waiting %v before retry attempt %d/%d", delay, attempt, maxRetries)
time.Sleep(delay)

err = AttestationKeyRegistration(logger, cfg.Attestation.AttestationKey.Registration,
attestationKey, platformName)
if err == nil {
break
}
logger.Info("Attestation registration attempt %d/%d failed: %v", attempt, maxRetries, err)
}
if err != nil {
return fmt.Errorf("failed to register attestation key after retries: %w", err)
}
} else {
return err
}
}
}
return nil
}

// GenerateAndPersistAK creates and persists the Attestation Key in the TPM
func GenerateAndPersistAK(logger *log.Logger) error {
if err := os.MkdirAll(TPMDir, 0755); err != nil {
return fmt.Errorf("couldn't create %s directory: %w", TPMDir, err)
}

if _, err := os.Stat(AKPath); err == nil {
logger.Info("Attestation Key already exists, skipping generation")
return nil
}

logger.Info("Generating Attestation Key")
cmd := exec.Command("tpm2_createak", "-C", EKHandle,
"-c", AKCtxPath, "-G", "rsa", "-g", "sha256",
"-s", "rsassa", "-u", AKPath, "-f", "pem")
if _, err := logger.LogCmd(cmd, "creating attestation key"); err != nil {
return fmt.Errorf("failed to create attestation key: %w", err)
}

cmd = exec.Command("tpm2_evictcontrol", "-c", AKCtxPath, AKHandle)
if _, err := logger.LogCmd(cmd, "persisting attestation key"); err != nil {
return fmt.Errorf("failed to persist attestation key: %w", err)
}

return nil
}

// AttestationKeyRegistration sends a request to register an attestation key
func AttestationKeyRegistration(logger *log.Logger, registration types.Registration, attestationKey string, platform string) error {
if registration.Url == nil || *registration.Url == "" {
return fmt.Errorf("registration URL is required")
}
// Check if AK was already generated
if _, err := os.Stat(AKRegisterd); err == nil {
return nil
Comment on lines +139 to +140

Choose a reason for hiding this comment

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

medium

The code checks if the AK was already generated by checking for the existence of the AKRegisterd file. However, the comment says "Check if AK was already generated", which is misleading. The file indicates that the AK was registered, not generated. This could cause confusion when debugging.

}

requestBody := map[string]string{
"attestation_key": attestationKey,
"platform": platform,
}

jsonBody, err := json.Marshal(requestBody)
if err != nil {
return fmt.Errorf("failed to marshal request body: %w", err)
}

client := &http.Client{}

if !util.NilOrEmpty(registration.Certificate) {
tlsConfig, err := createTLSConfig(*registration.Certificate)
if err != nil {
return fmt.Errorf("failed to create TLS config: %w", err)
}

client.Transport = &http.Transport{
TLSClientConfig: tlsConfig,
}
Comment on lines +155 to +163

Choose a reason for hiding this comment

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

medium

The code creates a TLS configuration only if registration.Certificate is not nil or empty. However, it does not handle the case where the certificate string is invalid or malformed. This could lead to a crash or unexpected behavior at runtime.

Consider adding error handling to check if the certificate is valid and return an error if it is not.

}

// Single attempt - caller (HandleAttestation) handles retries
req, err := http.NewRequest(http.MethodPut, *registration.Url, bytes.NewBuffer(jsonBody))
if err != nil {
return fmt.Errorf("failed to create request: %w", err)
}

req.Header.Set("Content-Type", "application/json")

resp, err := client.Do(req)
if err != nil {
// Return network errors as ErrNeedNet for caller to handle
if isNetworkUnreachable(err) {
return resource.ErrNeedNet
}
return fmt.Errorf("failed to register attestation key: %w", err)
}

defer resp.Body.Close()

Check failure on line 183 in internal/attestation/attestation.go

View workflow job for this annotation

GitHub Actions / Test (1.24.x)

Error return value of `resp.Body.Close` is not checked (errcheck)

if resp.StatusCode < 200 || resp.StatusCode >= 300 {
return fmt.Errorf("registration failed with status code: %d", resp.StatusCode)
Comment on lines +185 to +186

Choose a reason for hiding this comment

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

medium

The code checks if the response status code is outside the range of 200-299. However, it does not provide any context or information about the specific status code. This makes it difficult to debug registration failures.

Consider logging the status code and response body to provide more context for debugging.

}

// Registration successful
if err := os.WriteFile(AKRegisterd, []byte{}, 0644); err != nil {
return fmt.Errorf("failed to create AK registered file: %w", err)
}
Comment on lines +190 to +192

Choose a reason for hiding this comment

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

medium

The code creates an empty file AKRegisterd to indicate successful registration. However, it does not handle potential errors during file creation. This could lead to a false positive if the file creation fails.

Consider adding error handling to check if the file creation was successful and return an error if it was not.

logger.Info("Register successfully the AK")
return nil
}

// isNetworkUnreachable checks if the error indicates network is unreachable
func isNetworkUnreachable(err error) bool {
var opErr *net.OpError
if errors.As(err, &opErr) {
// Check for ENETUNREACH (network unreachable)
if errors.Is(opErr.Err, syscall.ENETUNREACH) {
return true
}
// Check for EHOSTUNREACH (host unreachable)
if errors.Is(opErr.Err, syscall.EHOSTUNREACH) {
return true
}
// Check for "connect: network is unreachable" string
if opErr.Err != nil && opErr.Err.Error() == "network is unreachable" {
return true
}
}
return false
}

func createTLSConfig(certPEM string) (*tls.Config, error) {
block, _ := pem.Decode([]byte(certPEM))
if block == nil {
return nil, fmt.Errorf("failed to decode PEM certificate")
}

cert, err := x509.ParseCertificate(block.Bytes)
if err != nil {
return nil, fmt.Errorf("failed to parse certificate: %w", err)
}

certPool := x509.NewCertPool()
certPool.AddCert(cert)

return &tls.Config{
RootCAs: certPool,
}, nil
}
Comment on lines +217 to +234

Choose a reason for hiding this comment

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

medium

The createTLSConfig function decodes and parses the certificate but doesn't handle potential errors during PEM decoding or certificate parsing. This could lead to unexpected behavior if the certificate is invalid.

20 changes: 16 additions & 4 deletions internal/exec/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/coreos/ignition/v2/config/shared/errors"
latest "github.com/coreos/ignition/v2/config/v3_6_experimental"
"github.com/coreos/ignition/v2/config/v3_6_experimental/types"
"github.com/coreos/ignition/v2/internal/attestation"
"github.com/coreos/ignition/v2/internal/exec/stages"
executil "github.com/coreos/ignition/v2/internal/exec/util"
"github.com/coreos/ignition/v2/internal/log"
Expand Down Expand Up @@ -176,7 +177,7 @@ func logStructuredJournalEntry(cfgInfo state.FetchedConfig) error {
func (e *Engine) acquireConfig(stageName string) (cfg types.Config, err error) {
switch {
case strings.HasPrefix(stageName, "fetch"):
cfg, err = e.acquireProviderConfig()
cfg, err = e.acquireProviderConfig(stageName)

// if we've successfully fetched and cached the configs, log about them
if err == nil && journal.Enabled() {
Expand Down Expand Up @@ -216,7 +217,7 @@ func (e *Engine) acquireCachedConfig() (cfg types.Config, err error) {

// acquireProviderConfig attempts to fetch the configuration from the
// provider.
func (e *Engine) acquireProviderConfig() (cfg types.Config, err error) {
func (e *Engine) acquireProviderConfig(stageName string) (cfg types.Config, err error) {
// Create a new http client and fetcher with the timeouts set via the flags,
// since we don't have a config with timeout values we can use
timeout := int(e.FetchTimeout.Seconds())
Expand All @@ -228,7 +229,7 @@ func (e *Engine) acquireProviderConfig() (cfg types.Config, err error) {
}

// (Re)Fetch the config if the cache is unreadable.
cfg, err = e.fetchProviderConfig()
cfg, err = e.fetchProviderConfig(stageName)
if err == errors.ErrEmpty {
// Continue if the provider config was empty as we want to write an empty
// cache config for use by other stages.
Expand Down Expand Up @@ -285,7 +286,7 @@ func (e *Engine) acquireProviderConfig() (cfg types.Config, err error) {
// it checks the config engine's provider. An error is returned if the provider
// is unavailable. This will also render the config (see renderConfig) before
// returning.
func (e *Engine) fetchProviderConfig() (types.Config, error) {
func (e *Engine) fetchProviderConfig(stageName string) (types.Config, error) {
platformConfigs := []platform.Config{
cmdline.Config,
system.Config,
Expand Down Expand Up @@ -315,6 +316,17 @@ func (e *Engine) fetchProviderConfig() (types.Config, error) {
Referenced: false,
})

if err := attestation.HandleAttestation(e.Logger, &cfg, e.PlatformConfig.Name(), e.NeedNet); err != nil {
if err == resource.ErrNeedNet && stageName == "fetch-offline" {
err = e.signalNeedNet()
if err != nil {
e.Logger.Crit("failed to signal neednet: %v", err)
}
return cfg, resource.ErrNeedNet
}
return types.Config{}, err
Comment on lines +319 to +327

Choose a reason for hiding this comment

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

medium

The code calls HandleAttestation and checks for resource.ErrNeedNet specifically in the fetch-offline stage. If this error occurs in other stages, it will not be handled in the same way, potentially leading to unexpected behavior or failures. It's important to ensure that network-related errors are handled consistently across all stages.

Consider handling resource.ErrNeedNet in a more generic way, or ensuring that signalNeedNet is called in all relevant stages.

}

// Replace the HTTP client in the fetcher to be configured with the
// timeouts of the config
err = e.Fetcher.UpdateHttpTimeoutsAndCAs(cfg.Ignition.Timeouts, cfg.Ignition.Security.TLS.CertificateAuthorities, cfg.Ignition.Proxy)
Expand Down
Loading