From 80975e2c75f4c72a5a4a7c6afbe320634f5e1ad4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Wed, 17 Dec 2025 13:12:30 +0100 Subject: [PATCH 1/5] remotes/docker: Propagate registry warnings to resolver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit According to the OCI distribution spec registries may include informational warnings in HTTP Warning headers: - https://github.com/opencontainers/distribution-spec/blob/e612a6e1e1bc717f9fa7e1feb4f05c8b6568754a/spec.md#warnings - https://www.rfc-editor.org/rfc/rfc7234#section-5.5 This change implements support for handling these warnings and propagating them to the resolver. This patch adds a new, optional WarningHandler interface field to ResolverOptions that allows callers to receive and process warnings sent by registries via HTTP Warning headers with warn-code 299. Signed-off-by: Paweł Gronowski --- core/remotes/docker/fetcher.go | 14 + core/remotes/docker/pusher.go | 6 + core/remotes/docker/resolver.go | 104 ++++--- core/remotes/docker/warnings.go | 223 +++++++++++++++ core/remotes/docker/warnings_test.go | 392 +++++++++++++++++++++++++++ 5 files changed, 708 insertions(+), 31 deletions(-) create mode 100644 core/remotes/docker/warnings.go create mode 100644 core/remotes/docker/warnings_test.go diff --git a/core/remotes/docker/fetcher.go b/core/remotes/docker/fetcher.go index b5c29b1070c2e..3c7f9d30c5055 100644 --- a/core/remotes/docker/fetcher.go +++ b/core/remotes/docker/fetcher.go @@ -231,6 +231,13 @@ func (r dockerFetcher) Fetch(ctx context.Context, desc ocispec.Descriptor) (io.R return nil, err } + if r.warningHandler != nil { + ctx = context.WithValue(ctx, warningSourceKey{}, WarningSource{ + Desc: &desc, + Digest: &desc.Digest, + }) + } + return newHTTPReadSeeker(desc.Size, func(offset int64) (io.ReadCloser, error) { // firstly try fetch via external urls for _, us := range desc.URLs { @@ -243,6 +250,7 @@ func (r dockerFetcher) Fetch(ctx context.Context, desc ocispec.Descriptor) (io.R log.G(ctx).Debug("non-http(s) alternative url is unsupported") continue } + ctx = log.WithLogger(ctx, log.G(ctx).WithField("url", u)) log.G(ctx).Info("request") @@ -379,6 +387,12 @@ func (r dockerFetcher) FetchByDigest(ctx context.Context, dgst digest.Digest, op return nil, desc, err } + if r.warningHandler != nil { + ctx = context.WithValue(ctx, warningSourceKey{}, WarningSource{ + Digest: &dgst, + }) + } + var ( getReq *request sz int64 diff --git a/core/remotes/docker/pusher.go b/core/remotes/docker/pusher.go index 94ef661d90d85..79cee392d06f1 100644 --- a/core/remotes/docker/pusher.go +++ b/core/remotes/docker/pusher.go @@ -82,6 +82,12 @@ func (p dockerPusher) push(ctx context.Context, desc ocispec.Descriptor, ref str if err != nil { return nil, err } + if p.dockerBase.warningHandler != nil { + ctx = context.WithValue(ctx, warningSourceKey{}, WarningSource{ + Desc: &desc, + Digest: &desc.Digest, + }) + } status, err := p.tracker.GetStatus(ref) if err == nil { if status.Committed && status.Offset == status.Total { diff --git a/core/remotes/docker/resolver.go b/core/remotes/docker/resolver.go index 45ce4dd59ddf9..ab0a6276766bc 100644 --- a/core/remotes/docker/resolver.go +++ b/core/remotes/docker/resolver.go @@ -102,6 +102,13 @@ type ResolverOptions struct { // mechanism for getting blob upload status is expensive. Tracker StatusTracker + // WarningHandler is called for each warning received from the registry. + // Warnings are reported via HTTP Warning headers with warn-code 299. + // It may be called concurrently from multiple goroutines, so + // implementations must be safe for concurrent use. + // If nil, warnings are ignored. + WarningHandler WarningHandler + // Authorizer is used to authorize registry requests // // Deprecated: use Hosts. @@ -139,11 +146,12 @@ func DefaultHost(ns string) (string, error) { } type dockerResolver struct { - hosts RegistryHosts - header http.Header - resolveHeader http.Header - tracker StatusTracker - config transfer.ImageResolverOptions + hosts RegistryHosts + header http.Header + resolveHeader http.Header + tracker StatusTracker + config transfer.ImageResolverOptions + warningHandler WarningHandler } // NewResolver returns a new resolver to a Docker registry @@ -198,10 +206,11 @@ func NewResolver(options ResolverOptions) remotes.Resolver { options.Hosts = ConfigureDefaultRegistries(opts...) } return &dockerResolver{ - hosts: options.Hosts, - header: options.Headers, - resolveHeader: resolveHeader, - tracker: options.Tracker, + hosts: options.Hosts, + header: options.Headers, + resolveHeader: resolveHeader, + tracker: options.Tracker, + warningHandler: options.WarningHandler, } } @@ -316,6 +325,10 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp "method": req.method, "url": req.sanitizedURL(), })) + + // Don't report warnings during resolution; will report after descriptor construction + req.warningHandler = nil + log.G(ctx).Debug("resolving") resp, err := req.doWithRetries(ctx, i == len(hosts)-1) if err != nil { @@ -329,6 +342,10 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp log.G(ctx).WithError(err).Info(nextHostOrFail(i)) continue // try another host } + var respHeaders http.Header + if r.warningHandler != nil { + respHeaders = resp.Header.Clone() + } resp.Body.Close() // don't care about body contents. if resp.StatusCode > 299 { @@ -398,6 +415,9 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp req.header[key] = append(req.header[key], value...) } + // Don't report warnings during resolution + req.warningHandler = nil + resp, err := req.doWithRetries(ctx, true) if err != nil { return "", ocispec.Descriptor{}, err @@ -409,6 +429,11 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp return "", ocispec.Descriptor{}, unexpectedResponseErr(resp) } + // Save response headers for warning reporting later + if r.warningHandler != nil { + respHeaders = resp.Header.Clone() + } + bodyReader := countingReader{reader: resp.Body} contentType = getManifestMediaType(resp) @@ -447,6 +472,13 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp Size: size, } + // Report any warnings with the warning source + reportWarningsWithSource(ctx, respHeaders, r.warningHandler, WarningSource{ + Ref: refspec, + Desc: &desc, + Digest: &dgst, + }) + log.G(ctx).WithField("desc.digest", desc.Digest).Debug("resolved") return ref, desc, nil } @@ -501,12 +533,13 @@ func (r *dockerResolver) resolveDockerBase(ref string) (*dockerBase, error) { } type dockerBase struct { - refspec reference.Spec - repository string - hosts []RegistryHost - header http.Header - performances transfer.ImageResolverPerformanceSettings - limiter *semaphore.Weighted + refspec reference.Spec + repository string + hosts []RegistryHost + header http.Header + performances transfer.ImageResolverPerformanceSettings + limiter *semaphore.Weighted + warningHandler WarningHandler } func (r *dockerBase) Acquire(ctx context.Context, weight int64) error { @@ -529,12 +562,13 @@ func (r *dockerResolver) base(refspec reference.Spec) (*dockerBase, error) { return nil, err } return &dockerBase{ - refspec: refspec, - repository: strings.TrimPrefix(refspec.Locator, host+"/"), - hosts: hosts, - header: r.header, - performances: r.config.Performances, - limiter: r.config.DownloadLimiter, + refspec: refspec, + repository: strings.TrimPrefix(refspec.Locator, host+"/"), + hosts: hosts, + header: r.header, + performances: r.config.Performances, + limiter: r.config.DownloadLimiter, + warningHandler: r.warningHandler, }, nil } @@ -568,10 +602,12 @@ func (r *dockerBase) request(host RegistryHost, method string, ps ...string) *re p = p + "/" } return &request{ - method: method, - path: p, - header: header, - host: host, + method: method, + path: p, + header: header, + host: host, + refspec: r.refspec, + warningHandler: r.warningHandler, } } @@ -616,12 +652,14 @@ func (r *request) addNamespace(ns string) error { } type request struct { - method string - path string - header http.Header - host RegistryHost - body func() (io.ReadCloser, error) - size int64 + method string + path string + header http.Header + host RegistryHost + body func() (io.ReadCloser, error) + size int64 + refspec reference.Spec + warningHandler WarningHandler } func (r *request) clone() *request { @@ -681,6 +719,7 @@ func (r *request) do(ctx context.Context) (*http.Response, error) { return nil, fmt.Errorf("failed to do request: %w", err) } log.G(ctx).WithFields(responseFields(resp)).Debug("fetch response received") + reportWarnings(ctx, resp.Header, r.warningHandler) return resp, nil } @@ -734,6 +773,9 @@ const maxAttempts = 5 func (r *request) doWithRetries(ctx context.Context, lastHost bool, checks ...doChecks) (resp *http.Response, err error) { attempts := maxAttempts + if r.warningHandler != nil { + ctx = updateWarningSource(ctx, r.refspec) + } resp, err = r.doWithRetriesInner(ctx, nil, &attempts, lastHost) if err != nil { return nil, err diff --git a/core/remotes/docker/warnings.go b/core/remotes/docker/warnings.go new file mode 100644 index 0000000000000..4b4b4657c7346 --- /dev/null +++ b/core/remotes/docker/warnings.go @@ -0,0 +1,223 @@ +/* + Copyright The containerd Authors. + + 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 docker + +import ( + "context" + "encoding/hex" + "net/http" + "strings" + + "github.com/containerd/log" + digest "github.com/opencontainers/go-digest" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" + + "github.com/containerd/containerd/v2/pkg/reference" +) + +// WarningSource contains the information about the request that caused the warning. +type WarningSource struct { + // Ref is the reference specification of the content that the warning is for. + Ref reference.Spec + + // Desc is the descriptor of the content that the warning is for. + // Can be nil if the warning is not for a specific content. + Desc *ocispec.Descriptor + + // Digest is the digest of the content that the warning is for. + // Can be nil if the warning is not for a specific content. + Digest *digest.Digest +} + +// WarningHandler is a function that receives warnings from the registry. +// Warnings are extracted from HTTP Warning headers as defined in RFC 7234. +// The src parameter contains information about the request that the warning is +// for. If the warning is for specific content, its descriptor is available via +// src.Desc. src.Desc is nil if the warning is not for a specific content. +type WarningHandler interface { + Warn(ctx context.Context, src WarningSource, warning string) +} + +type warningHandlerFunc func(ctx context.Context, src WarningSource, warning string) + +func (f warningHandlerFunc) Warn(ctx context.Context, src WarningSource, warning string) { + f(ctx, src, warning) +} + +type warningSourceKey struct{} + +// reportWarnings extracts and reports warnings from HTTP Warning headers. +// Per RFC 7234 and OCI distribution spec, warnings use warn-code 299, +// warn-agent "-", and the format: Warning: 299 - "message" +func reportWarnings(ctx context.Context, header http.Header, handler WarningHandler) { + if handler == nil { + return + } + + var warnSrc WarningSource + if v, ok := ctx.Value(warningSourceKey{}).(WarningSource); ok { + warnSrc = v + } + + reportWarningsWithSource(ctx, header, handler, warnSrc) +} + +func reportWarningsWithSource(ctx context.Context, header http.Header, handler WarningHandler, warnSrc WarningSource) { + if handler == nil { + return + } + + for _, warning := range header.Values("Warning") { + // Parse RFC 7234 warning format: warn-code warn-agent "warn-text" + // Expected format for OCI: 299 - "message" + if text := parseWarningText(warning); text != "" { + handler.Warn(ctx, warnSrc, text) + } + } +} + +// parseWarningText extracts the warn-text from an RFC 7234 Warning header value. +// Only the OCI distribution spec recommended format is supported: +// 299 - "message" +// Other warn-agent values (tokens other than "-") and optional warn-date +// suffixes defined in RFC 7234 are not supported and will cause the +// warning to be silently ignored. +// Characters are validated per RFC 7230 section 3.2.6 quoted-string rules: +// - qdtext: HTAB / SP / %x21 / %x23-5B / %x5D-7E / obs-text +// - quoted-pair: "\" ( HTAB / SP / VCHAR / obs-text ) +// - percent-encoding: %xx where xx is a hex value; decoded byte must be valid qdtext +func parseWarningText(warning string) string { + text, ok := strings.CutPrefix(warning, "299 - ") + if !ok { + return "" + } + + text = strings.TrimSpace(text) + + ln := len(text) + if ln == 0 || text[0] != '"' || text[ln-1] != '"' { + return "" + } + + out := strings.Builder{} + idx := 1 // skip opening quote + end := ln - 1 + + for idx < end { + c := text[idx] + + if c == '\\' { + if idx+1 >= end { + return "" + } + nextC := text[idx+1] + if !isQuotedPairChar(nextC) { + return "" + } + out.WriteByte(nextC) + idx += 2 + continue + } + + // Handle percent-encoding (%xx) + if c == '%' && idx+2 < end { + decoded, err := hex.DecodeString(text[idx+1 : idx+3]) + if err == nil && len(decoded) == 1 { + if !isQdtext(decoded[0]) { + return "" + } + out.WriteByte(decoded[0]) + idx += 3 + continue + } + // Invalid percent-encoding, treat as literal + } + + if !isQdtext(c) { + return "" + } + out.WriteByte(c) + idx++ + } + + return out.String() +} + +func updateWarningSource(ctx context.Context, ref reference.Spec) context.Context { + var warnSrc WarningSource + if v, ok := ctx.Value(warningSourceKey{}).(WarningSource); ok { + warnSrc = v + } + warnSrc.Ref = ref + ctx = context.WithValue(ctx, warningSourceKey{}, warnSrc) + return ctx +} + +// isQdtext returns whether c is valid unescaped inside a quoted-string per RFC 7230 §3.2.6. +// +// qdtext = HTAB / SP / %x21 / %x23-5B / %x5D-7E / %x80-FF +func isQdtext(c byte) bool { + if c == 0x09 || c == 0x20 { // HTAB, SP + return true + } + if c == 0x21 { // '!' + return true + } + if c >= 0x23 && c <= 0x5B { // '#' to '[' + return true + } + if c >= 0x5D && c <= 0x7E { // ']' to '~' + return true + } + if c >= 0x80 { // obs-text + return true + } + return false +} + +// isQuotedPairChar returns whether c is valid after a backslash per RFC 7230 §3.2.6. +// +// quoted-pair = "\" ( HTAB / SP / VCHAR / %x80-FF ) +// VCHAR = %x21-7E +func isQuotedPairChar(c byte) bool { + if c == 0x09 || c == 0x20 { // HTAB, SP + return true + } + if c >= 0x21 && c <= 0x7E { // VCHAR + return true + } + if c >= 0x80 { // obs-text + return true + } + return false +} + +// LogWarningHandler is a WarningHandler that logs warnings using the +// containerd log package. +type LogWarningHandler struct{} + +// Warn logs the warning message with source information. +func (LogWarningHandler) Warn(ctx context.Context, src WarningSource, warning string) { + fields := log.Fields{} + if ref := src.Ref.String(); ref != "" { + fields["ref"] = ref + } + if src.Digest != nil { + fields["digest"] = src.Digest.String() + } + log.G(ctx).WithFields(fields).Warn("registry warning: " + warning) +} diff --git a/core/remotes/docker/warnings_test.go b/core/remotes/docker/warnings_test.go new file mode 100644 index 0000000000000..f89f65b68612b --- /dev/null +++ b/core/remotes/docker/warnings_test.go @@ -0,0 +1,392 @@ +/* + Copyright The containerd Authors. + + 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 docker + +import ( + "context" + "fmt" + "io" + "net/http" + "net/http/httptest" + "reflect" + "strings" + "testing" + + ocispec "github.com/opencontainers/image-spec/specs-go/v1" + + "github.com/containerd/containerd/v2/pkg/reference" +) + +func TestWarningHandler(t *testing.T) { + name := "testname" + tag := "latest" + + m := newManifest( + newContent(ocispec.MediaTypeImageConfig, []byte("{}")), + newContent(ocispec.MediaTypeImageLayerGzip, []byte("layer content")), + ) + mc := newContent(ocispec.MediaTypeImageManifest, m.OCIManifest()) + + mux := http.NewServeMux() + addWarning := func(h http.Handler, msg string) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Add("Warning", fmt.Sprintf(`299 - "%s"`, msg)) + h.ServeHTTP(w, r) + }) + } + + for _, c := range []testContent{m.config, m.references[0]} { + mux.Handle(fmt.Sprintf("/v2/%s/blobs/%s", name, c.Digest()), addWarning(c, "Blob access warning")) + } + mux.Handle(fmt.Sprintf("/v2/%s/manifests/%s", name, tag), addWarning(mc, "Manifest access warning")) + mux.Handle(fmt.Sprintf("/v2/%s/manifests/%s", name, mc.Digest()), addWarning(mc, "Manifest access warning")) + + s := httptest.NewServer(mux) + defer s.Close() + + image := fmt.Sprintf("%s/%s:%s", strings.TrimPrefix(s.URL, "http://"), name, tag) + + // Create expected descriptors and digests + mcDesc := mc.Descriptor() + mcDigest := mc.Digest() + configDesc := m.config.Descriptor() + configDigest := m.config.Digest() + layer0Desc := m.references[0].Descriptor() + layer0Digest := m.references[0].Digest() + + type expectedWarning struct { + warning string + src WarningSource + } + + tests := []struct { + name string + resolve bool + descriptors []ocispec.Descriptor + expectedWarnings []expectedWarning + }{ + { + name: "Resolve", + resolve: true, + expectedWarnings: []expectedWarning{ + { + warning: "Manifest access warning", + src: WarningSource{ + Ref: mustParseRef(t, image), + Desc: &mcDesc, + Digest: &mcDigest, + }, + }, + }, + }, + { + name: "Fetch manifest", + descriptors: []ocispec.Descriptor{mc.Descriptor()}, + expectedWarnings: []expectedWarning{ + { + warning: "Manifest access warning", + src: WarningSource{ + Ref: mustParseRef(t, image), + Desc: &mcDesc, + Digest: &mcDigest, + }, + }, + }, + }, + { + name: "Fetch blob", + descriptors: []ocispec.Descriptor{m.config.Descriptor(), m.references[0].Descriptor()}, + expectedWarnings: []expectedWarning{ + { + warning: "Blob access warning", + src: WarningSource{ + Ref: mustParseRef(t, image), + Desc: &configDesc, + Digest: &configDigest, + }, + }, + { + warning: "Blob access warning", + src: WarningSource{ + Ref: mustParseRef(t, image), + Desc: &layer0Desc, + Digest: &layer0Digest, + }, + }, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + type capturedWarning struct { + src WarningSource + warning string + } + // No synchronization needed: Resolve and Fetch calls are + // sequential, so the handler is never called concurrently here. + var warnings []capturedWarning + resolver := NewResolver(ResolverOptions{ + WarningHandler: warningHandlerFunc(func(ctx context.Context, src WarningSource, warning string) { + warnings = append(warnings, capturedWarning{src: src, warning: warning}) + }), + }) + + ctx := t.Context() + + if tc.resolve { + if _, _, err := resolver.Resolve(ctx, image); err != nil { + t.Fatal(err) + } + } + + if len(tc.descriptors) > 0 { + fetcher, err := resolver.Fetcher(ctx, image) + if err != nil { + t.Fatal(err) + } + for _, desc := range tc.descriptors { + rc, err := fetcher.Fetch(ctx, desc) + if err != nil { + t.Fatal(err) + } + if _, err := io.Copy(io.Discard, rc); err != nil { + t.Fatal(err) + } + if err := rc.Close(); err != nil { + t.Fatal(err) + } + } + } + + if len(warnings) != len(tc.expectedWarnings) { + t.Fatalf("Expected %d warnings, got %d: %v", len(tc.expectedWarnings), len(warnings), warnings) + } + + for i, w := range warnings { + expected := tc.expectedWarnings[i] + if w.warning != expected.warning { + t.Fatalf("Expected warning %q, got: %s", expected.warning, w.warning) + } + if w.src.Ref.String() != expected.src.Ref.String() { + t.Fatalf("Expected warning source ref %q, got: %s", expected.src.Ref.String(), w.src.Ref.String()) + } + // Check descriptor if present in expected + if expected.src.Desc != nil { + if w.src.Desc == nil { + t.Fatalf("Expected warning source descriptor %+v, got nil", expected.src.Desc) + } + if !reflect.DeepEqual(*w.src.Desc, *expected.src.Desc) { + t.Fatalf("Expected warning source descriptor %+v, got: %+v", expected.src.Desc, w.src.Desc) + } + } + // Check digest if present in expected + if expected.src.Digest != nil { + if w.src.Digest == nil { + t.Fatalf("Expected warning source digest %v, got nil", expected.src.Digest) + } + if *w.src.Digest != *expected.src.Digest { + t.Fatalf("Expected warning source digest %v, got: %v", expected.src.Digest, w.src.Digest) + } + } + } + }) + } +} + +func TestParseWarningText(t *testing.T) { + tests := []struct { + name string + header string + want string + }{ + { + name: "simple warning", + header: `299 - "message"`, + want: "message", + }, + { + name: "prefixed header text invalid", + header: `foobar 299 - "prefixed message"`, + want: "", + }, + { + name: "escaped quotes preserved", + header: `299 - "quoted \"text\" inside"`, + want: `quoted "text" inside`, + }, + { + name: "escaped backslash", + header: `299 - "path\\to\\file"`, + want: `path\to\file`, + }, + { + name: "escaped tab", + header: "299 - \"escaped\\\ttab\"", + want: "escaped\ttab", + }, + { + name: "escaped space", + header: `299 - "escaped\ space"`, + want: "escaped space", + }, + { + name: "backslash followed by NUL rejected", + header: "299 - \"bad\\\x00text\"", + want: "", + }, + { + name: "backslash followed by newline rejected", + header: "299 - \"bad\\\ntext\"", + want: "", + }, + { + name: "backslash followed by DEL rejected", + header: "299 - \"bad\\\x7ftext\"", + want: "", + }, + { + name: "bare control char NUL in qdtext rejected", + header: "299 - \"bad\x00text\"", + want: "", + }, + { + name: "bare control char BEL in qdtext rejected", + header: "299 - \"bad\x07text\"", + want: "", + }, + { + name: "bare DEL in qdtext rejected", + header: "299 - \"bad\x7ftext\"", + want: "", + }, + { + name: "obs-text byte in qdtext accepted", + header: "299 - \"caf\xe9\"", + want: "caf\xe9", + }, + { + name: "multi-byte UTF-8 accepted as obs-text bytes", + header: "299 - \"\xc3\xa9l\xc3\xa8ve\"", + want: "\xc3\xa9l\xc3\xa8ve", + }, + { + name: "backslash followed by obs-text accepted", + header: "299 - \"test\\\x80value\"", + want: "test\x80value", + }, + { + name: "percent-encoded space", + header: `299 - "hello%20world"`, + want: "hello world", + }, + { + name: "percent-encoded special chars", + header: `299 - "100%25 complete"`, + want: "100% complete", + }, + { + name: "percent-encoded quote rejected", + header: `299 - "say %22hello%22"`, + want: "", + }, + { + name: "invalid percent-encoding treated as literal", + header: `299 - "100%ZZ invalid"`, + want: "100%ZZ invalid", + }, + { + name: "incomplete percent-encoding at end", + header: `299 - "trailing%2"`, + want: "trailing%2", + }, + { + name: "percent-encoded newline rejected", + header: `299 - "inject%0aline"`, + want: "", + }, + { + name: "percent-encoded CR rejected", + header: `299 - "inject%0dline"`, + want: "", + }, + { + name: "percent-encoded NUL rejected", + header: `299 - "inject%00byte"`, + want: "", + }, + { + name: "trailing characters invalid", + header: `299 - "warn text" extra data`, + want: "", + }, + { + name: "second quoted string invalid", + header: `299 - "warn text" "extra data"`, + want: "", + }, + { + name: "non oci warning code", + header: `199 - "ignored"`, + want: "", + }, + { + name: "missing closing quote", + header: `299 - "incomplete`, + want: "", + }, + { + name: "missing opening quote", + header: `299 - warn text"`, + want: "", + }, + { + name: "empty input", + header: "", + want: "", + }, + { + name: "trailing backslash before closing quote", + header: `299 - "trailing\"`, + want: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := parseWarningText(tt.header) + if got != tt.want { + t.Fatalf("parseWarningText(%q) = %q, want %q", tt.header, got, tt.want) + } + }) + } +} + +func TestReportWarningsWithNilHandler(t *testing.T) { + header := http.Header{} + header.Add("Warning", `299 - "warning message"`) + + reportWarningsWithSource(t.Context(), header, nil, WarningSource{}) +} + +func mustParseRef(t *testing.T, ref string) reference.Spec { + spec, err := reference.Parse(ref) + if err != nil { + t.Fatalf("failed to parse reference %q: %v", ref, err) + } + return spec +} From 416749988858ac217df215a27687f4acf4117068 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 12 May 2026 11:53:05 -0700 Subject: [PATCH 2/5] deps: bump oc/selinux to v1.14.1 Switch away from deprecated functionality. Signed-off-by: Kir Kolyshkin --- go.mod | 2 +- go.sum | 4 +- internal/cri/server/service_linux.go | 4 +- internal/cri/seutil/seutil.go | 5 +- .../cri/store/container/container_test.go | 3 +- internal/cri/store/label/label.go | 12 +- internal/cri/store/label/label_test.go | 6 +- .../selinux/go-selinux/label/label_linux.go | 92 ++-- .../selinux/go-selinux/label/label_stub.go | 1 - .../selinux/go-selinux/selinux.go | 82 ++- .../selinux/go-selinux/selinux_linux.go | 486 +++++++++++------- .../selinux/go-selinux/selinux_stub.go | 34 +- .../selinux/pkg/pwalkdir/pwalkdir.go | 5 +- vendor/modules.txt | 4 +- 14 files changed, 465 insertions(+), 275 deletions(-) diff --git a/go.mod b/go.mod index 79fa290d79564..18d960a9b198c 100644 --- a/go.mod +++ b/go.mod @@ -56,7 +56,7 @@ require ( github.com/opencontainers/image-spec v1.1.1 github.com/opencontainers/runtime-spec v1.3.0 github.com/opencontainers/runtime-tools v0.9.1-0.20251114084447-edf4cb3d2116 - github.com/opencontainers/selinux v1.13.1 + github.com/opencontainers/selinux v1.14.1 github.com/pelletier/go-toml/v2 v2.4.3 github.com/prometheus/client_golang v1.24.0 github.com/prometheus/client_model v0.6.2 diff --git a/go.sum b/go.sum index 13c8c41760890..7d2d0b775d1ef 100644 --- a/go.sum +++ b/go.sum @@ -269,8 +269,8 @@ github.com/opencontainers/runtime-spec v1.3.0 h1:YZupQUdctfhpZy3TM39nN9Ika5CBWT5 github.com/opencontainers/runtime-spec v1.3.0/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0= github.com/opencontainers/runtime-tools v0.9.1-0.20251114084447-edf4cb3d2116 h1:tAKu3NkKWZYpqBSOJKwTxT1wIGueiF7gcmcNgr5pNTY= github.com/opencontainers/runtime-tools v0.9.1-0.20251114084447-edf4cb3d2116/go.mod h1:DKDEfzxvRkoQ6n9TGhxQgg2IM1lY4aM0eaQP4e3oElw= -github.com/opencontainers/selinux v1.13.1 h1:A8nNeceYngH9Ow++M+VVEwJVpdFmrlxsN22F+ISDCJE= -github.com/opencontainers/selinux v1.13.1/go.mod h1:S10WXZ/osk2kWOYKy1x2f/eXF5ZHJoUs8UU/2caNRbg= +github.com/opencontainers/selinux v1.14.1 h1:a7XlXV/nN/l5zFP1FWZYoExpClu1QOPMfWUV2CZ8kEQ= +github.com/opencontainers/selinux v1.14.1/go.mod h1:LenyElirjUHszfxrjuFqC85HIeXZKumHcKMQtnaDlQQ= github.com/pelletier/go-toml/v2 v2.4.3 h1:GTRvJQutkOSftxIFD5xw9aepkYNuPWmVJpffdDPYVpY= github.com/pelletier/go-toml/v2 v2.4.3/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= github.com/petermattis/goid v0.0.0-20240813172612-4fcff4a6cae7 h1:Dx7Ovyv/SFnMFw3fD4oEoeorXc6saIiQ23LrGLth0Gw= diff --git a/internal/cri/server/service_linux.go b/internal/cri/server/service_linux.go index 1398a7dd889a6..367694fcf1789 100644 --- a/internal/cri/server/service_linux.go +++ b/internal/cri/server/service_linux.go @@ -50,7 +50,9 @@ func (c *criService) initPlatform() (err error) { log.L.Warn("Selinux is not supported") } if r := c.config.SelinuxCategoryRange; r > 0 { - selinux.CategoryRange = uint32(r) + if err := selinux.SetCategoryRange(uint32(r)); err != nil { + return fmt.Errorf("SelinuxCategoryRange: %w", err) + } } } else { selinux.SetDisabled() diff --git a/internal/cri/seutil/seutil.go b/internal/cri/seutil/seutil.go index c69065d20f77a..2cd1208943562 100644 --- a/internal/cri/seutil/seutil.go +++ b/internal/cri/seutil/seutil.go @@ -25,7 +25,10 @@ func ChangeToKVM(l string) (string, error) { if l == "" || !selinux.GetEnabled() { return "", nil } - proc, _ := selinux.KVMContainerLabels() + proc, err := selinux.KVMContainerLabel() + if err != nil { + return "", err + } selinux.ReleaseLabel(proc) current, err := selinux.NewContext(l) diff --git a/internal/cri/store/container/container_test.go b/internal/cri/store/container/container_test.go index a3204e56b7796..31f6933fea52a 100644 --- a/internal/cri/store/container/container_test.go +++ b/internal/cri/store/container/container_test.go @@ -165,8 +165,9 @@ func TestContainerStore(t *testing.T) { s := NewStore(label.NewStore(), nil) reserved := map[string]bool{} - s.labels.Reserver = func(label string) { + s.labels.Reserver = func(label string) error { reserved[strings.SplitN(label, ":", 4)[3]] = true + return nil } s.labels.Releaser = func(label string) { reserved[strings.SplitN(label, ":", 4)[3]] = false diff --git a/internal/cri/store/label/label.go b/internal/cri/store/label/label.go index d58fa4dbed96e..c987bb73f0741 100644 --- a/internal/cri/store/label/label.go +++ b/internal/cri/store/label/label.go @@ -17,6 +17,7 @@ package label import ( + "errors" "sync" "github.com/opencontainers/selinux/go-selinux" @@ -27,7 +28,7 @@ type Store struct { sync.Mutex levels map[string]int Releaser func(string) - Reserver func(string) + Reserver func(string) error } // NewStore creates a new SELinux process label store @@ -35,7 +36,7 @@ func NewStore() *Store { return &Store{ levels: map[string]int{}, Releaser: selinux.ReleaseLabel, - Reserver: selinux.ReserveLabel, + Reserver: selinux.ReserveLabelV2, } } @@ -57,7 +58,12 @@ func (s *Store) Reserve(label string) error { } if _, ok := s.levels[level]; !ok { - s.Reserver(label) + // The label may already be reserved, e.g. because selinux.ContainerLabels() + // just generated (and reserved) it, or because another container in the + // same pod is reusing the sandbox's label. + if err := s.Reserver(label); err != nil && !errors.Is(err, selinux.ErrMCSAlreadyExists) { + return err + } } s.levels[level]++ diff --git a/internal/cri/store/label/label_test.go b/internal/cri/store/label/label_test.go index cc2c214bfadc2..7d4b3e464038a 100644 --- a/internal/cri/store/label/label_test.go +++ b/internal/cri/store/label/label_test.go @@ -37,10 +37,11 @@ func TestAddThenRemove(t *testing.T) { releaseCount++ assert.Equal(1, releaseCount) } - store.Reserver = func(label string) { + store.Reserver = func(label string) error { assert.Contains(label, ":c1,c2") reserveCount++ assert.Equal(1, reserveCount) + return nil } t.Log("should count to two level") @@ -78,8 +79,9 @@ func TestJunkData(t *testing.T) { releaseCount++ } reserveCount := 0 - store.Reserver = func(label string) { + store.Reserver = func(label string) error { reserveCount++ + return nil } t.Log("should ignore empty label") diff --git a/vendor/github.com/opencontainers/selinux/go-selinux/label/label_linux.go b/vendor/github.com/opencontainers/selinux/go-selinux/label/label_linux.go index 95f29e21f4e84..2145571780811 100644 --- a/vendor/github.com/opencontainers/selinux/go-selinux/label/label_linux.go +++ b/vendor/github.com/opencontainers/selinux/go-selinux/label/label_linux.go @@ -22,60 +22,68 @@ var ErrIncompatibleLabel = errors.New("bad SELinux option: z and Z can not be us // InitLabels returns the process label and file labels to be used within // the container. A list of options can be passed into this function to alter -// the labels. The labels returned will include a random MCS String, that is -// guaranteed to be unique. +// the labels. +// +// Unless the "level" option is provided (to set a custom level), the labels +// returned will include a random MCS string guaranteed to be unique in the +// scope of the process using this package. If the "level" option is provided, +// the custom level set is reserved but not checked to be unique. +// // If the disabled flag is passed in, the process label will not be set, but the mount label will be set // to the container_file label with the maximum category. This label is not usable by any confined label. func InitLabels(options []string) (plabel string, mlabel string, retErr error) { if !selinux.GetEnabled() { return "", "", nil } - processLabel, mountLabel := selinux.ContainerLabels() - if processLabel != "" { - defer func() { - if retErr != nil { - selinux.ReleaseLabel(mountLabel) - } - }() - pcon, err := selinux.NewContext(processLabel) - if err != nil { - return "", "", err + processLabel, mountLabel := selinux.ContainerLabels() //nolint:staticcheck // ContainerLabels will be moved to an internal package. + if processLabel == "" { + // processLabel is required; if empty, do nothing. + return processLabel, mountLabel, nil + } + defer func() { + if retErr != nil { + selinux.ReleaseLabel(mountLabel) } - mcsLevel := pcon["level"] - mcon, err := selinux.NewContext(mountLabel) - if err != nil { - return "", "", err + }() + pcon, err := selinux.NewContext(processLabel) + if err != nil { + return "", "", err + } + mcsLevel := pcon["level"] + mcon, err := selinux.NewContext(mountLabel) + if err != nil { + return "", "", err + } + for _, opt := range options { + if opt == "disable" { + selinux.ReleaseLabel(mountLabel) + return "", selinux.PrivContainerMountLabel(), nil } - for _, opt := range options { - if opt == "disable" { - selinux.ReleaseLabel(mountLabel) - return "", selinux.PrivContainerMountLabel(), nil - } - if i := strings.Index(opt, ":"); i == -1 { - return "", "", fmt.Errorf("bad label option %q, valid options 'disable' or \n'user, role, level, type, filetype' followed by ':' and a value", opt) - } - con := strings.SplitN(opt, ":", 2) - if !validOptions[con[0]] { - return "", "", fmt.Errorf("bad label option %q, valid options 'disable, user, role, level, type, filetype'", con[0]) - } - if con[0] == "filetype" { - mcon["type"] = con[1] - continue - } - pcon[con[0]] = con[1] - if con[0] == "level" || con[0] == "user" { - mcon[con[0]] = con[1] - } + k, v, ok := strings.Cut(opt, ":") + if !ok || !validOptions[k] { + return "", "", fmt.Errorf("bad label option %q, valid options 'disable' or \n'user, role, level, type, filetype' followed by ':' and a value", opt) + } + if k == "filetype" { + mcon["type"] = v + continue } - if pcon.Get() != processLabel { - if pcon["level"] != mcsLevel { - selinux.ReleaseLabel(processLabel) + pcon[k] = v + if k == "level" || k == "user" { + mcon[k] = v + } + } + if p := pcon.Get(); p != processLabel { + if pcon["level"] != mcsLevel { + selinux.ReleaseLabel(processLabel) + // Ignore ErrMCSAlreadyExists as label is user-specified and might be + // already reserved (e.g. when containers in a pod use the same label). + if err := selinux.ReserveLabelV2(p); err != nil && !errors.Is(err, selinux.ErrMCSAlreadyExists) { + return "", "", err } - processLabel = pcon.Get() - selinux.ReserveLabel(processLabel) } - mountLabel = mcon.Get() + processLabel = p } + mountLabel = mcon.Get() return processLabel, mountLabel, nil } diff --git a/vendor/github.com/opencontainers/selinux/go-selinux/label/label_stub.go b/vendor/github.com/opencontainers/selinux/go-selinux/label/label_stub.go index 7a54afc5e6d16..6cbf8876e397a 100644 --- a/vendor/github.com/opencontainers/selinux/go-selinux/label/label_stub.go +++ b/vendor/github.com/opencontainers/selinux/go-selinux/label/label_stub.go @@ -1,5 +1,4 @@ //go:build !linux -// +build !linux package label diff --git a/vendor/github.com/opencontainers/selinux/go-selinux/selinux.go b/vendor/github.com/opencontainers/selinux/go-selinux/selinux.go index 15150d4752819..1935bf69ee7c0 100644 --- a/vendor/github.com/opencontainers/selinux/go-selinux/selinux.go +++ b/vendor/github.com/opencontainers/selinux/go-selinux/selinux.go @@ -26,11 +26,6 @@ var ( // ErrInvalidLabel is returned when an invalid label is specified. ErrInvalidLabel = errors.New("invalid Label") - // InvalidLabel is returned when an invalid label is specified. - // - // Deprecated: use [ErrInvalidLabel]. - InvalidLabel = ErrInvalidLabel - // ErrIncomparable is returned two levels are not comparable ErrIncomparable = errors.New("incomparable levels") // ErrLevelSyntax is returned when a sensitivity or category do not have correct syntax in a level @@ -45,7 +40,9 @@ var ( // is not the thread group leader. ErrNotTGLeader = errors.New("calling thread is not the thread group leader") - // CategoryRange allows the upper bound on the category range to be adjusted + // CategoryRange allows the upper bound on the category range to be adjusted. + // + // Deprecated: use [SetCategoryRange] instead. CategoryRange = DefaultCategoryRange privContainerMountLabel string @@ -64,6 +61,16 @@ func GetEnabled() bool { return getEnabled() } +// SetCategoryRange allows to adjust the upper bound of the category range. +// It affects subsequent calls to [KVMContainerLabel] and [InitContainerLabel]. +func SetCategoryRange(upper uint32) error { + if upper > DefaultCategoryRange { + return errors.New("can't have more than DefaultCategoryRange categories") + } + CategoryRange = upper + return nil +} + // ClassIndex returns the int index for an object class in the loaded policy, // or -1 and an error func ClassIndex(class string) (int, error) { @@ -107,12 +114,12 @@ func SetFSCreateLabel(label string) error { // FSCreateLabel returns the default label the kernel which the kernel is using // for file system objects created by this task. "" indicates default. func FSCreateLabel() (string, error) { - return fsCreateLabel() + return readConThreadSelf("attr/fscreate") } // CurrentLabel returns the SELinux label of the current process thread, or an error. func CurrentLabel() (string, error) { - return currentLabel() + return readConThreadSelf("attr/current") } // PidLabel returns the SELinux label of the given pid, or an error. @@ -123,7 +130,7 @@ func PidLabel(pid int) (string, error) { // ExecLabel returns the SELinux label that the kernel will use for any programs // that are executed by the current process thread, or an error. func ExecLabel() (string, error) { - return execLabel() + return readConThreadSelf("attr/exec") } // CanonicalizeContext takes a context string and writes it to the kernel @@ -180,7 +187,7 @@ func SocketLabel() (string, error) { // PeerLabel retrieves the label of the client on the other side of a socket func PeerLabel(fd uintptr) (string, error) { - return peerLabel(fd) + return peerLabel(int(fd)) //#nosec G115 -- ignore "integer overflow conversion uintptr -> int". } // SetKeyLabel takes a process label and tells the kernel to assign the @@ -216,9 +223,26 @@ func ClearLabels() { clearLabels() } -// ReserveLabel reserves the MLS/MCS level component of the specified label +// ReserveLabel reserves the MLS/MCS level component of the specified label. +// +// Deprecated: use [ReserveLabelV2] instead. func ReserveLabel(label string) { - reserveLabel(label) + _ = reserveLabel(label) +} + +// ReserveLabelV2 reserves the MLS/MCS level component of the specified label. +// Returns an error if the label can't be reserved. +// +// Callers that are intentionally reusing an existing level/MCS (e.g. multiple +// container in a pod sharing a label) may safely ignore [ErrMCSAlreadyExists] +// error. +func ReserveLabelV2(label string) error { + return reserveLabel(label) +} + +// CheckLabel check the MLS/MCS level component of the specified label +func CheckLabel(label string) error { + return checkLabel(label) } // MLSEnabled checks if MLS is enabled. @@ -250,25 +274,47 @@ func ReleaseLabel(label string) { releaseLabel(label) } -// ROFileLabel returns the specified SELinux readonly file label +// ROFileLabel returns the specified SELinux readonly file label. +// +// Deprecated: this (apparently) has no users and will be removed from the +// future version of this package. Open a bug report if you use it. func ROFileLabel() string { return roFileLabel() } // KVMContainerLabels returns the default processLabel and mountLabel to be used // for kvm containers by the calling process. +// +// Deprecated: use [KVMContainerLabel] instead. func KVMContainerLabels() (string, string) { return kvmContainerLabels() } +// KVMContainerLabel returns the default process label to be used +// for KVM containers by the calling process. +func KVMContainerLabel() (string, error) { + return kvmContainerLabel() +} + // InitContainerLabels returns the default processLabel and file labels to be // used for containers running an init system like systemd by the calling process. +// +// Deprecated: use [InitContainerLabel] instead. func InitContainerLabels() (string, string) { return initContainerLabels() } +// InitContainerLabel returns the default process label to be used +// for containers running an init system like systemd by the calling process. +func InitContainerLabel() (string, error) { + return initContainerLabel() +} + // ContainerLabels returns an allocated processLabel and fileLabel to be used for // container labeling by the calling process. +// +// Deprecated: this (apparently) has no users and will be removed from the +// future version of this package. Open a bug report if you use it. func ContainerLabels() (processLabel string, fileLabel string) { return containerLabels() } @@ -305,11 +351,19 @@ func DisableSecOpt() []string { return []string{"disable"} } +// SEUserByName retrieves the SELinux username and security level for a given +// Linux username. The username and security level is based on the +// /etc/selinux/{SELINUXTYPE}/seusers file. +func SEUserByName(username string) (seUser string, level string, err error) { + return getSeUserByName(username) +} + // GetDefaultContextWithLevel gets a single context for the specified SELinux user // identity that is reachable from the specified scon context. The context is based // on the per-user /etc/selinux/{SELINUXTYPE}/contexts/users/ if it exists, // and falls back to the global /etc/selinux/{SELINUXTYPE}/contexts/default_contexts -// file. +// file and finally the global /etc/selinux/{SELINUXTYPE}/contexts/failsafe_context +// file if no match can be found anywhere else. func GetDefaultContextWithLevel(user, level, scon string) (string, error) { return getDefaultContextWithLevel(user, level, scon) } diff --git a/vendor/github.com/opencontainers/selinux/go-selinux/selinux_linux.go b/vendor/github.com/opencontainers/selinux/go-selinux/selinux_linux.go index 6d7f8e270bd73..2117155701f8c 100644 --- a/vendor/github.com/opencontainers/selinux/go-selinux/selinux_linux.go +++ b/vendor/github.com/opencontainers/selinux/go-selinux/selinux_linux.go @@ -3,16 +3,16 @@ package selinux import ( "bufio" "bytes" - "crypto/rand" - "encoding/binary" "errors" "fmt" "io" "io/fs" "math/big" + "math/rand/v2" "os" "os/user" "path/filepath" + "slices" "strconv" "strings" "sync" @@ -30,6 +30,7 @@ const ( selinuxDir = "/etc/selinux/" selinuxUsersDir = "contexts/users" defaultContexts = "contexts/default_contexts" + failsafeContext = "contexts/failsafe_context" selinuxConfig = selinuxDir + "config" selinuxfsMount = "/sys/fs/selinux" selinuxTypeTag = "SELINUXTYPE" @@ -38,11 +39,9 @@ const ( ) type selinuxState struct { - mcsList map[string]bool - selinuxfs string - selinuxfsOnce sync.Once - enabledSet bool - enabled bool + mcsList map[string]struct{} + enabledSet bool + enabled bool sync.Mutex } @@ -56,10 +55,19 @@ type mlsRange struct { high *level } +type openReaderCloser func() (io.ReadCloser, error) + +func createOpener(path string) openReaderCloser { + return func() (io.ReadCloser, error) { + return os.Open(path) + } +} + type defaultSECtx struct { - userRdr io.Reader + openUserRdr openReaderCloser verifier func(string) error - defaultRdr io.Reader + openDefaultRdr openReaderCloser + openFailsafeRdr openReaderCloser user, level, scon string } @@ -72,26 +80,15 @@ const ( var ( readOnlyFileLabel string - state = selinuxState{ - mcsList: make(map[string]bool), - } - - // for policyRoot() - policyRootOnce sync.Once - policyRootVal string - // for label() - loadLabelsOnce sync.Once - labels map[string]string + state = selinuxState{ + mcsList: make(map[string]struct{}), + } ) -func policyRoot() string { - policyRootOnce.Do(func() { - policyRootVal = filepath.Join(selinuxDir, readConfig(selinuxTypeTag)) - }) - - return policyRootVal -} +var policyRoot = sync.OnceValue(func() string { + return filepath.Join(selinuxDir, readConfig(selinuxTypeTag)) +}) func (s *selinuxState) setEnable(enabled bool) bool { s.Lock() @@ -148,7 +145,12 @@ func verifySELinuxfsMount(mnt string) bool { return true } -func findSELinuxfs() string { +// getSelinuxMountPoint returns the path to the mountpoint of an selinuxfs +// filesystem or an empty string if no mountpoint is found. Selinuxfs is +// a proc-like pseudo-filesystem that exposes the SELinux policy API to +// processes. The existence of an selinuxfs mount is used to determine +// whether SELinux is currently enabled or not. +var getSelinuxMountPoint = sync.OnceValue(func() string { // fast path: check the default mount first if verifySELinuxfsMount(selinuxfsMount) { return selinuxfsMount @@ -180,7 +182,7 @@ func findSELinuxfs() string { return mnt } } -} +}) // findSELinuxfsMount returns a next selinuxfs mount point found, // if there is one, or an empty string in case of EOF or error. @@ -203,23 +205,6 @@ func findSELinuxfsMount(s *bufio.Scanner) string { return "" } -func (s *selinuxState) getSELinuxfs() string { - s.selinuxfsOnce.Do(func() { - s.selinuxfs = findSELinuxfs() - }) - - return s.selinuxfs -} - -// getSelinuxMountPoint returns the path to the mountpoint of an selinuxfs -// filesystem or an empty string if no mountpoint is found. Selinuxfs is -// a proc-like pseudo-filesystem that exposes the SELinux policy API to -// processes. The existence of an selinuxfs mount is used to determine -// whether SELinux is currently enabled or not. -func getSelinuxMountPoint() string { - return state.getSELinuxfs() -} - // getEnabled returns whether SELinux is currently enabled. func getEnabled() bool { return state.getEnabled() @@ -244,12 +229,9 @@ func readConfig(target string) string { // Skip comments continue } - fields := bytes.SplitN(line, []byte{'='}, 2) - if len(fields) != 2 { - continue - } - if bytes.Equal(fields[0], []byte(target)) { - return string(bytes.Trim(fields[1], `"`)) + key, val, ok := bytes.Cut(line, []byte{'='}) + if ok && string(key) == target { + return string(bytes.Trim(val, `"`)) } } return "" @@ -530,17 +512,6 @@ func setFSCreateLabel(label string) error { return writeConThreadSelf("attr/fscreate", label) } -// fsCreateLabel returns the default label the kernel which the kernel is using -// for file system objects created by this task. "" indicates default. -func fsCreateLabel() (string, error) { - return readConThreadSelf("attr/fscreate") -} - -// currentLabel returns the SELinux label of the current process thread, or an error. -func currentLabel() (string, error) { - return readConThreadSelf("attr/current") -} - // pidLabel returns the SELinux label of the given pid, or an error. func pidLabel(pid int) (string, error) { it, err := openProcPid(pid, "attr/current", os.O_RDONLY|unix.O_CLOEXEC) @@ -551,12 +522,6 @@ func pidLabel(pid int) (string, error) { return readConFd(it) } -// ExecLabel returns the SELinux label that the kernel will use for any programs -// that are executed by the current process thread, or an error. -func execLabel() (string, error) { - return readConThreadSelf("exec") -} - // canonicalizeContext takes a context string and writes it to the kernel // the function then returns the context that the kernel will use. Use this // function to check if two contexts are equivalent @@ -581,13 +546,12 @@ func catsToBitset(cats string) (*big.Int, error) { catlist := strings.Split(cats, ",") for _, r := range catlist { - ranges := strings.SplitN(r, ".", 2) - if len(ranges) > 1 { - catstart, err := parseLevelItem(ranges[0], category) + if s, e, ok := strings.Cut(r, "."); ok { + catstart, err := parseLevelItem(s, category) if err != nil { return nil, err } - catend, err := parseLevelItem(ranges[1], category) + catend, err := parseLevelItem(e, category) if err != nil { return nil, err } @@ -595,7 +559,7 @@ func catsToBitset(cats string) (*big.Int, error) { bitset.SetBit(bitset, i, 1) } } else { - cat, err := parseLevelItem(ranges[0], category) + cat, err := parseLevelItem(r, category) if err != nil { return nil, err } @@ -623,14 +587,14 @@ func parseLevelItem(s string, sep levelItem) (int, error) { // parseLevel fills a level from a string that contains // a sensitivity and categories func (l *level) parseLevel(levelStr string) error { - lvl := strings.SplitN(levelStr, ":", 2) - sens, err := parseLevelItem(lvl[0], sensitivity) + s, c, ok := strings.Cut(levelStr, ":") + sens, err := parseLevelItem(s, sensitivity) if err != nil { return fmt.Errorf("failed to parse sensitivity: %w", err) } l.sens = sens - if len(lvl) > 1 { - cats, err := catsToBitset(lvl[1]) + if ok { + cats, err := catsToBitset(c) if err != nil { return fmt.Errorf("failed to parse categories: %w", err) } @@ -643,25 +607,19 @@ func (l *level) parseLevel(levelStr string) error { // rangeStrToMLSRange marshals a string representation of a range. func rangeStrToMLSRange(rangeStr string) (*mlsRange, error) { r := &mlsRange{} - l := strings.SplitN(rangeStr, "-", 2) - - switch len(l) { - // rangeStr that has a low and a high level, e.g. s4:c0.c1023-s6:c0.c1023 - case 2: + lo, hi, ok := strings.Cut(rangeStr, "-") + r.low = &level{} + if err := r.low.parseLevel(lo); err != nil { + return nil, fmt.Errorf("failed to parse low level %q: %w", lo, err) + } + if ok { + // rangeStr that has a low and a high level, e.g. s4:c0.c1023-s6:c0.c1023. r.high = &level{} - if err := r.high.parseLevel(l[1]); err != nil { - return nil, fmt.Errorf("failed to parse high level %q: %w", l[1], err) - } - fallthrough - // rangeStr that is single level, e.g. s6:c0,c3,c5,c30.c1023 - case 1: - r.low = &level{} - if err := r.low.parseLevel(l[0]); err != nil { - return nil, fmt.Errorf("failed to parse low level %q: %w", l[0], err) + if err := r.high.parseLevel(hi); err != nil { + return nil, fmt.Errorf("failed to parse high level %q: %w", hi, err) } - } - - if r.high == nil { + } else { + // rangeStr that is single level, e.g. s6:c0,c3,c5,c30.c1023. r.high = r.low } @@ -732,22 +690,6 @@ func (m mlsRange) String() string { return low + "-" + high } -// TODO: remove these in favor of built-in min/max -// once we stop supporting Go < 1.21. -func maxInt(a, b int) int { - if a > b { - return a - } - return b -} - -func minInt(a, b int) int { - if a < b { - return a - } - return b -} - // calculateGlbLub computes the glb (greatest lower bound) and lub (least upper bound) // of a source and target range. // The glblub is calculated as the greater of the low sensitivities and @@ -770,10 +712,10 @@ func calculateGlbLub(sourceRange, targetRange string) (string, error) { outrange := &mlsRange{low: &level{}, high: &level{}} /* take the greatest of the low */ - outrange.low.sens = maxInt(s.low.sens, t.low.sens) + outrange.low.sens = max(s.low.sens, t.low.sens) /* take the least of the high */ - outrange.high.sens = minInt(s.high.sens, t.high.sens) + outrange.high.sens = min(s.high.sens, t.high.sens) /* find the intersecting categories */ if s.low.cats != nil && t.low.cats != nil { @@ -807,10 +749,10 @@ func readWriteCon(fpath string, val string) (string, error) { } // peerLabel retrieves the label of the client on the other side of a socket -func peerLabel(fd uintptr) (string, error) { - l, err := unix.GetsockoptString(int(fd), unix.SOL_SOCKET, unix.SO_PEERSEC) +func peerLabel(fd int) (string, error) { + l, err := unix.GetsockoptString(fd, unix.SOL_SOCKET, unix.SO_PEERSEC) if err != nil { - return "", &os.PathError{Op: "getsockopt", Path: "fd " + strconv.Itoa(int(fd)), Err: err} + return "", &os.PathError{Op: "getsockopt", Path: "fd " + strconv.Itoa(fd), Err: err} } return l, nil } @@ -871,18 +813,34 @@ func newContext(label string) (Context, error) { // clearLabels clears all reserved labels func clearLabels() { state.Lock() - state.mcsList = make(map[string]bool) + state.mcsList = make(map[string]struct{}) state.Unlock() } -// reserveLabel reserves the MLS/MCS level component of the specified label -func reserveLabel(label string) { +// reserveLabel reserves the MLS/MCS level component of the specified label. +func reserveLabel(label string) error { + if len(label) != 0 { + con := strings.SplitN(label, ":", 4) + if len(con) > 3 { + return mcsAdd(con[3]) + } + } + + return nil +} + +func checkLabel(label string) error { if len(label) != 0 { con := strings.SplitN(label, ":", 4) if len(con) > 3 { - _ = mcsAdd(con[3]) + state.Lock() + defer state.Unlock() + if _, exist := state.mcsList[con[3]]; exist { + return ErrMCSAlreadyExists + } } } + return nil } func selinuxEnforcePath() string { @@ -938,10 +896,10 @@ func mcsAdd(mcs string) error { } state.Lock() defer state.Unlock() - if state.mcsList[mcs] { + if _, exist := state.mcsList[mcs]; exist { return ErrMCSAlreadyExists } - state.mcsList[mcs] = true + state.mcsList[mcs] = struct{}{} return nil } @@ -951,41 +909,21 @@ func mcsDelete(mcs string) { } state.Lock() defer state.Unlock() - state.mcsList[mcs] = false -} - -func intToMcs(id int, catRange uint32) string { - var ( - SETSIZE = int(catRange) - TIER = SETSIZE - ORD = id - ) - - if id < 1 || id > 523776 { - return "" - } - - for ORD > TIER { - ORD -= TIER - TIER-- - } - TIER = SETSIZE - TIER - ORD += TIER - return fmt.Sprintf("s0:c%d,c%d", TIER, ORD) + delete(state.mcsList, mcs) } func uniqMcs(catRange uint32) string { var ( - n uint32 c1, c2 uint32 mcs string ) for { - _ = binary.Read(rand.Reader, binary.LittleEndian, &n) - c1 = n % catRange - _ = binary.Read(rand.Reader, binary.LittleEndian, &n) - c2 = n % catRange + //#nosec G404 -- using slightly more predictable MCS labels won't affect security, so it's fine to use math/rand/v2 here. + { + c1 = rand.Uint32N(catRange) + c2 = rand.Uint32N(catRange) + } if c1 == c2 { continue } else if c1 > c2 { @@ -1023,11 +961,11 @@ func openContextFile() (*os.File, error) { return os.Open(filepath.Join(policyRoot(), "contexts", "lxc_contexts")) } -func loadLabels() { - labels = make(map[string]string) +var loadLabels = sync.OnceValue(func() map[string]string { + labels := make(map[string]string) in, err := openContextFile() if err != nil { - return + return labels } defer in.Close() @@ -1043,25 +981,21 @@ func loadLabels() { // Skip comments continue } - fields := bytes.SplitN(line, []byte{'='}, 2) - if len(fields) != 2 { - continue + if key, val, ok := bytes.Cut(line, []byte{'='}); ok { + key, val = bytes.TrimSpace(key), bytes.TrimSpace(val) + labels[string(key)] = string(bytes.Trim(val, `"`)) } - key, val := bytes.TrimSpace(fields[0]), bytes.TrimSpace(fields[1]) - labels[string(key)] = string(bytes.Trim(val, `"`)) } con, _ := NewContext(labels["file"]) con["level"] = fmt.Sprintf("s0:c%d,c%d", maxCategory-2, maxCategory-1) privContainerMountLabel = con.get() - reserveLabel(privContainerMountLabel) -} + _ = reserveLabel(privContainerMountLabel) + return labels +}) func label(key string) string { - loadLabelsOnce.Do(func() { - loadLabels() - }) - return labels[key] + return loadLabels()[key] } // kvmContainerLabels returns the default processLabel and mountLabel to be used @@ -1075,6 +1009,15 @@ func kvmContainerLabels() (string, string) { return addMcs(processLabel, label("file")) } +func kvmContainerLabel() (string, error) { + processLabel := label("kvm_process") + if processLabel == "" { + processLabel = label("process") + } + pLabel, _, err := addMcsProc(processLabel) + return pLabel, err +} + // initContainerLabels returns the default processLabel and file labels to be // used for containers running an init system like systemd by the calling process. func initContainerLabels() (string, string) { @@ -1086,6 +1029,16 @@ func initContainerLabels() (string, string) { return addMcs(processLabel, label("file")) } +func initContainerLabel() (string, error) { + processLabel := label("init_process") + if processLabel == "" { + processLabel = label("process") + } + + pLabel, _, err := addMcsProc(processLabel) + return pLabel, err +} + // containerLabels returns an allocated processLabel and fileLabel to be used for // container labeling by the calling process. func containerLabels() (processLabel string, fileLabel string) { @@ -1108,13 +1061,24 @@ func containerLabels() (processLabel string, fileLabel string) { return addMcs(processLabel, fileLabel) } -func addMcs(processLabel, fileLabel string) (string, string) { - scon, _ := NewContext(processLabel) +func addMcsProc(processLabel string) (string, string, error) { + var mcs string + scon, err := NewContext(processLabel) + if err != nil { + return "", "", err + } if scon["level"] != "" { - mcs := uniqMcs(CategoryRange) + mcs = uniqMcs(CategoryRange) scon["level"] = mcs processLabel = scon.Get() - scon, _ = NewContext(fileLabel) + } + return processLabel, mcs, nil +} + +func addMcs(processLabel, fileLabel string) (string, string) { + processLabel, mcs, _ := addMcsProc(processLabel) + if mcs != "" { + scon, _ := NewContext(fileLabel) scon["level"] = mcs fileLabel = scon.Get() } @@ -1281,6 +1245,111 @@ func dupSecOpt(src string) ([]string, error) { return dup, nil } +// checkGroup returns true if group's GID is in the list of GIDs gids. +func checkGroup(group string, gids []string, lookupGroup func(string) (*user.Group, error)) bool { + grp, err := lookupGroup(group) + if err != nil { + return false + } + + return slices.Contains(gids, grp.Gid) +} + +// getSeUserFromReader reads the seusers file: https://www.man7.org/linux/man-pages/man5/seusers.5.html +func getSeUserFromReader(username string, gids []string, r io.Reader, lookupGroup func(string) (*user.Group, error)) (seUser string, level string, err error) { + var defaultSeUser, defaultLevel string + var groupSeUser, groupLevel string + + lineNum := -1 + scanner := bufio.NewScanner(r) + for scanner.Scan() { + rawLine := scanner.Text() + lineNum++ + + // remove any trailing comments, then extra whitespace + line, _, _ := strings.Cut(rawLine, "#") + line = strings.TrimSpace(line) + if line == "" { + continue + } + + userField, rest, ok := strings.Cut(line, ":") + if !ok { + return "", "", fmt.Errorf("line %d: malformed line", lineNum) + } + if userField == "" { + return "", "", fmt.Errorf("line %d: user_id or group_id is empty", lineNum) + } + seUserField, rest, ok := strings.Cut(rest, ":") + if seUserField == "" { + return "", "", fmt.Errorf("line %d: seuser_id is empty", lineNum) + } + var levelField string + // level is optional + if ok { + levelField = rest + } + + // we found a match, return it + if userField == username { + return seUserField, levelField, nil + } + + // if the first field starts with '%' it's a group, check if + // the user is a member of that group and set the group + // SELinux user and level if so + if userField[0] == '%' && groupSeUser == "" { + if checkGroup(userField[1:], gids, lookupGroup) { + groupSeUser = seUserField + groupLevel = levelField + } + } else if userField == "__default__" && defaultSeUser == "" { + defaultSeUser = seUserField + defaultLevel = levelField + } + } + if err := scanner.Err(); err != nil { + return "", "", fmt.Errorf("failed to read seusers file: %w", err) + } + + if groupSeUser != "" { + return groupSeUser, groupLevel, nil + } + if defaultSeUser != "" { + return defaultSeUser, defaultLevel, nil + } + + return "", "", fmt.Errorf("could not find SELinux user for %q login", username) +} + +// getSeUserByName returns an SELinux user and MLS level that is +// mapped to a given Linux user. +func getSeUserByName(username string) (string, string, error) { + seUsersConf := filepath.Join(policyRoot(), "seusers") + confFile, err := os.Open(seUsersConf) + if err != nil { + return "", "", fmt.Errorf("failed to open seusers file: %w", err) + } + defer confFile.Close() + + usr, err := user.Lookup(username) + if err != nil { + return "", "", err + } + gids, err := usr.GroupIds() + if err != nil { + return "", "", err + } + gids = append([]string{usr.Gid}, gids...) + + seUser, level, err := getSeUserFromReader(username, gids, confFile, user.LookupGroup) + if err != nil { + return "", "", fmt.Errorf("failed to parse seusers file: %w", err) + } + + return seUser, level, nil +} + // findUserInContext scans the reader for a valid SELinux context // match that is verified with the verifier. Invalid contexts are // skipped. It returns a matched context or an empty string if no @@ -1338,6 +1407,33 @@ func findUserInContext(context Context, r io.Reader, verifier func(string) error return "", nil } +// getFailsafeContext returns the context in the failsafe_context file: +// https://www.man7.org/linux/man-pages/man5/failsafe_context.5.html +func getFailsafeContext(context Context, r io.Reader, verifier func(string) error) (string, error) { + conn := make([]byte, 256) + limReader := io.LimitReader(r, int64(len(conn))) + _, err := limReader.Read(conn) + if err != nil { + return "", fmt.Errorf("failed to read failsafe context: %w", err) + } + + conn = bytes.TrimSpace(conn) + toConns := strings.SplitN(string(conn), ":", 4) + if len(toConns) != 3 { + return "", nil + } + + context["role"] = toConns[0] + context["type"] = toConns[1] + + outConn := context.get() + if err := verifier(outConn); err != nil { + return "", err + } + + return outConn, nil +} + func getDefaultContextFromReaders(c *defaultSECtx) (string, error) { if c.verifier == nil { return "", ErrVerifierNil @@ -1352,18 +1448,45 @@ func getDefaultContextFromReaders(c *defaultSECtx) (string, error) { context["user"] = c.user context["level"] = c.level - conn, err := findUserInContext(context, c.userRdr, c.verifier) + userRdr, err := c.openUserRdr() if err != nil { - return "", err + return "", fmt.Errorf("failed to open user context file: %w", err) + } + defer userRdr.Close() + + conn, err := findUserInContext(context, userRdr, c.verifier) + if err != nil { + return "", fmt.Errorf("failed to read %q's user context file: %w", c.user, err) } if conn != "" { return conn, nil } - conn, err = findUserInContext(context, c.defaultRdr, c.verifier) + defaultRdr, err := c.openDefaultRdr() if err != nil { - return "", err + return "", fmt.Errorf("failed to open default context file: %w", err) + } + defer defaultRdr.Close() + + conn, err = findUserInContext(context, defaultRdr, c.verifier) + if err != nil { + return "", fmt.Errorf("failed to read default user context file: %w", err) + } + + if conn != "" { + return conn, nil + } + + failsafeRdr, err := c.openFailsafeRdr() + if err != nil { + return "", fmt.Errorf("failed to open failsafe context file: %w", err) + } + defer failsafeRdr.Close() + + conn, err = getFailsafeContext(context, failsafeRdr, c.verifier) + if err != nil { + return "", fmt.Errorf("failed to read failsafe_context: %w", err) } if conn != "" { @@ -1375,26 +1498,17 @@ func getDefaultContextFromReaders(c *defaultSECtx) (string, error) { func getDefaultContextWithLevel(user, level, scon string) (string, error) { userPath := filepath.Join(policyRoot(), selinuxUsersDir, user) - fu, err := os.Open(userPath) - if err != nil { - return "", err - } - defer fu.Close() - defaultPath := filepath.Join(policyRoot(), defaultContexts) - fd, err := os.Open(defaultPath) - if err != nil { - return "", err - } - defer fd.Close() + failsafePath := filepath.Join(policyRoot(), failsafeContext) c := defaultSECtx{ - user: user, - level: level, - scon: scon, - userRdr: fu, - defaultRdr: fd, - verifier: securityCheckContext, + user: user, + level: level, + scon: scon, + openUserRdr: createOpener(userPath), + openDefaultRdr: createOpener(defaultPath), + openFailsafeRdr: createOpener(failsafePath), + verifier: securityCheckContext, } return getDefaultContextFromReaders(&c) diff --git a/vendor/github.com/opencontainers/selinux/go-selinux/selinux_stub.go b/vendor/github.com/opencontainers/selinux/go-selinux/selinux_stub.go index 382244e5036b8..78a4e1fe3524a 100644 --- a/vendor/github.com/opencontainers/selinux/go-selinux/selinux_stub.go +++ b/vendor/github.com/opencontainers/selinux/go-selinux/selinux_stub.go @@ -1,5 +1,4 @@ //go:build !linux -// +build !linux package selinux @@ -41,22 +40,10 @@ func setFSCreateLabel(string) error { return nil } -func fsCreateLabel() (string, error) { - return "", nil -} - -func currentLabel() (string, error) { - return "", nil -} - func pidLabel(int) (string, error) { return "", nil } -func execLabel() (string, error) { - return "", nil -} - func canonicalizeContext(string) (string, error) { return "", nil } @@ -69,7 +56,7 @@ func calculateGlbLub(string, string) (string, error) { return "", nil } -func peerLabel(uintptr) (string, error) { +func peerLabel(int) (string, error) { return "", nil } @@ -92,7 +79,12 @@ func newContext(string) (Context, error) { func clearLabels() { } -func reserveLabel(string) { +func reserveLabel(string) error { + return nil +} + +func checkLabel(string) error { + return nil } func isMLSEnabled() bool { @@ -122,10 +114,18 @@ func kvmContainerLabels() (string, string) { return "", "" } +func kvmContainerLabel() (string, error) { + return "", nil +} + func initContainerLabels() (string, string) { return "", "" } +func initContainerLabel() (string, error) { + return "", nil +} + func containerLabels() (string, string) { return "", "" } @@ -146,6 +146,10 @@ func dupSecOpt(string) ([]string, error) { return nil, nil } +func getSeUserByName(string) (string, string, error) { + return "", "", nil +} + func getDefaultContextWithLevel(string, string, string) (string, error) { return "", nil } diff --git a/vendor/github.com/opencontainers/selinux/pkg/pwalkdir/pwalkdir.go b/vendor/github.com/opencontainers/selinux/pkg/pwalkdir/pwalkdir.go index 5d2d09a298509..d361dcb64ce2c 100644 --- a/vendor/github.com/opencontainers/selinux/pkg/pwalkdir/pwalkdir.go +++ b/vendor/github.com/opencontainers/selinux/pkg/pwalkdir/pwalkdir.go @@ -1,6 +1,3 @@ -//go:build go1.16 -// +build go1.16 - package pwalkdir import ( @@ -92,7 +89,7 @@ func WalkN(root string, walkFn fs.WalkDirFunc, num int) error { }() wg.Add(num) - for i := 0; i < num; i++ { + for range num { go func() { for file := range files { if e := walkFn(file.path, file.entry, nil); e != nil { diff --git a/vendor/modules.txt b/vendor/modules.txt index a9394ed664fa7..e94c62cc69b7e 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -460,8 +460,8 @@ github.com/opencontainers/runtime-spec/specs-go/features github.com/opencontainers/runtime-tools/generate github.com/opencontainers/runtime-tools/generate/seccomp github.com/opencontainers/runtime-tools/validate/capabilities -# github.com/opencontainers/selinux v1.13.1 -## explicit; go 1.19 +# github.com/opencontainers/selinux v1.14.1 +## explicit; go 1.22 github.com/opencontainers/selinux/go-selinux github.com/opencontainers/selinux/go-selinux/label github.com/opencontainers/selinux/pkg/pwalkdir From ba3a464b8dbbae3481b8f5b9a6e8b665845caf40 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 12 May 2026 18:45:47 -0700 Subject: [PATCH 3/5] bump oc/selinux to v1.15.1, use SetProcessKind Drop internal/cri/seutil/seutil.go in favor of direct call to newly introduced selinux.SetProcessKind. Signed-off-by: Kir Kolyshkin --- go.mod | 2 +- go.sum | 4 +- internal/cri/server/helpers_linux.go | 7 +-- .../cri/server/podsandbox/helpers_linux.go | 6 +-- internal/cri/seutil/seutil.go | 44 ----------------- .../selinux/go-selinux/label/label_linux.go | 11 +++-- .../selinux/go-selinux/selinux.go | 20 ++++++++ .../selinux/go-selinux/selinux_linux.go | 47 ++++++++++++++++++- .../selinux/go-selinux/selinux_stub.go | 4 ++ vendor/modules.txt | 2 +- 10 files changed, 89 insertions(+), 58 deletions(-) delete mode 100644 internal/cri/seutil/seutil.go diff --git a/go.mod b/go.mod index 18d960a9b198c..0887a798900a7 100644 --- a/go.mod +++ b/go.mod @@ -56,7 +56,7 @@ require ( github.com/opencontainers/image-spec v1.1.1 github.com/opencontainers/runtime-spec v1.3.0 github.com/opencontainers/runtime-tools v0.9.1-0.20251114084447-edf4cb3d2116 - github.com/opencontainers/selinux v1.14.1 + github.com/opencontainers/selinux v1.15.1 github.com/pelletier/go-toml/v2 v2.4.3 github.com/prometheus/client_golang v1.24.0 github.com/prometheus/client_model v0.6.2 diff --git a/go.sum b/go.sum index 7d2d0b775d1ef..8775b008be581 100644 --- a/go.sum +++ b/go.sum @@ -269,8 +269,8 @@ github.com/opencontainers/runtime-spec v1.3.0 h1:YZupQUdctfhpZy3TM39nN9Ika5CBWT5 github.com/opencontainers/runtime-spec v1.3.0/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0= github.com/opencontainers/runtime-tools v0.9.1-0.20251114084447-edf4cb3d2116 h1:tAKu3NkKWZYpqBSOJKwTxT1wIGueiF7gcmcNgr5pNTY= github.com/opencontainers/runtime-tools v0.9.1-0.20251114084447-edf4cb3d2116/go.mod h1:DKDEfzxvRkoQ6n9TGhxQgg2IM1lY4aM0eaQP4e3oElw= -github.com/opencontainers/selinux v1.14.1 h1:a7XlXV/nN/l5zFP1FWZYoExpClu1QOPMfWUV2CZ8kEQ= -github.com/opencontainers/selinux v1.14.1/go.mod h1:LenyElirjUHszfxrjuFqC85HIeXZKumHcKMQtnaDlQQ= +github.com/opencontainers/selinux v1.15.1 h1:ERxeh5caJvCzNAKdI8WQbJmB1LDTn4BuaAg8wihLBpA= +github.com/opencontainers/selinux v1.15.1/go.mod h1:LenyElirjUHszfxrjuFqC85HIeXZKumHcKMQtnaDlQQ= github.com/pelletier/go-toml/v2 v2.4.3 h1:GTRvJQutkOSftxIFD5xw9aepkYNuPWmVJpffdDPYVpY= github.com/pelletier/go-toml/v2 v2.4.3/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= github.com/petermattis/goid v0.0.0-20240813172612-4fcff4a6cae7 h1:Dx7Ovyv/SFnMFw3fD4oEoeorXc6saIiQ23LrGLth0Gw= diff --git a/internal/cri/server/helpers_linux.go b/internal/cri/server/helpers_linux.go index 383bbc30af2d6..d67059311c17e 100644 --- a/internal/cri/server/helpers_linux.go +++ b/internal/cri/server/helpers_linux.go @@ -36,9 +36,10 @@ import ( containerd "github.com/containerd/containerd/v2/client" "github.com/containerd/containerd/v2/core/mount" "github.com/containerd/containerd/v2/core/snapshots" - "github.com/containerd/containerd/v2/internal/cri/seutil" + "github.com/containerd/containerd/v2/pkg/apparmor" "github.com/containerd/containerd/v2/pkg/seccomp" + selinux "github.com/opencontainers/selinux/go-selinux" ) // apparmorEnabled returns true if apparmor is enabled, supported by the host, @@ -173,10 +174,10 @@ func isVMBasedRuntime(runtimeType string) bool { } func modifyProcessLabel(runtimeType string, spec *specs.Spec) error { - if !isVMBasedRuntime(runtimeType) { + if !selinux.GetEnabled() || !isVMBasedRuntime(runtimeType) { return nil } - l, err := seutil.ChangeToKVM(spec.Process.SelinuxLabel) + l, err := selinux.SetProcessKind(spec.Process.SelinuxLabel, selinux.ProcessKindKVM) if err != nil { return fmt.Errorf("failed to get selinux kvm label: %w", err) } diff --git a/internal/cri/server/podsandbox/helpers_linux.go b/internal/cri/server/podsandbox/helpers_linux.go index d97c59ae53004..55d9df24724ba 100644 --- a/internal/cri/server/podsandbox/helpers_linux.go +++ b/internal/cri/server/podsandbox/helpers_linux.go @@ -31,6 +31,7 @@ import ( "github.com/containerd/log" "github.com/moby/sys/mountinfo" runtimespec "github.com/opencontainers/runtime-spec/specs-go" + "github.com/opencontainers/selinux/go-selinux" "github.com/opencontainers/selinux/go-selinux/label" "golang.org/x/sys/unix" runtime "k8s.io/cri-api/pkg/apis/runtime/v1" @@ -38,7 +39,6 @@ import ( containerd "github.com/containerd/containerd/v2/client" "github.com/containerd/containerd/v2/core/mount" "github.com/containerd/containerd/v2/core/snapshots" - "github.com/containerd/containerd/v2/internal/cri/seutil" "github.com/containerd/containerd/v2/pkg/seccomp" "github.com/containerd/containerd/v2/pkg/sys" ) @@ -296,10 +296,10 @@ func isVMBasedRuntime(runtimeType string) bool { } func modifyProcessLabel(runtimeType string, spec *runtimespec.Spec) error { - if !isVMBasedRuntime(runtimeType) { + if !selinux.GetEnabled() || !isVMBasedRuntime(runtimeType) { return nil } - l, err := seutil.ChangeToKVM(spec.Process.SelinuxLabel) + l, err := selinux.SetProcessKind(spec.Process.SelinuxLabel, selinux.ProcessKindKVM) if err != nil { return fmt.Errorf("failed to get selinux kvm label: %w", err) } diff --git a/internal/cri/seutil/seutil.go b/internal/cri/seutil/seutil.go deleted file mode 100644 index 2cd1208943562..0000000000000 --- a/internal/cri/seutil/seutil.go +++ /dev/null @@ -1,44 +0,0 @@ -/* - Copyright The containerd Authors. - - 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 seutil - -import ( - "github.com/opencontainers/selinux/go-selinux" -) - -// ChangeToKVM process label -func ChangeToKVM(l string) (string, error) { - if l == "" || !selinux.GetEnabled() { - return "", nil - } - proc, err := selinux.KVMContainerLabel() - if err != nil { - return "", err - } - selinux.ReleaseLabel(proc) - - current, err := selinux.NewContext(l) - if err != nil { - return "", err - } - next, err := selinux.NewContext(proc) - if err != nil { - return "", err - } - current["type"] = next["type"] - return current.Get(), nil -} diff --git a/vendor/github.com/opencontainers/selinux/go-selinux/label/label_linux.go b/vendor/github.com/opencontainers/selinux/go-selinux/label/label_linux.go index 2145571780811..a89c6bda194c8 100644 --- a/vendor/github.com/opencontainers/selinux/go-selinux/label/label_linux.go +++ b/vendor/github.com/opencontainers/selinux/go-selinux/label/label_linux.go @@ -10,7 +10,6 @@ import ( // Valid Label Options var validOptions = map[string]bool{ - "disable": true, "type": true, "filetype": true, "user": true, @@ -35,9 +34,13 @@ func InitLabels(options []string) (plabel string, mlabel string, retErr error) { if !selinux.GetEnabled() { return "", "", nil } + if len(options) > 0 && options[0] == "disable" { + return "", selinux.PrivContainerMountLabel(), nil + } processLabel, mountLabel := selinux.ContainerLabels() //nolint:staticcheck // ContainerLabels will be moved to an internal package. - if processLabel == "" { - // processLabel is required; if empty, do nothing. + if processLabel == "" || len(options) == 0 { + // 1. processLabel is required; if empty, do nothing. + // 2. If there are no options to process, we're done. return processLabel, mountLabel, nil } defer func() { @@ -55,6 +58,8 @@ func InitLabels(options []string) (plabel string, mlabel string, retErr error) { return "", "", err } for _, opt := range options { + // For backward compatibility, process "disable" + // even if it's not the only option. if opt == "disable" { selinux.ReleaseLabel(mountLabel) return "", selinux.PrivContainerMountLabel(), nil diff --git a/vendor/github.com/opencontainers/selinux/go-selinux/selinux.go b/vendor/github.com/opencontainers/selinux/go-selinux/selinux.go index 1935bf69ee7c0..ad30aa960b09e 100644 --- a/vendor/github.com/opencontainers/selinux/go-selinux/selinux.go +++ b/vendor/github.com/opencontainers/selinux/go-selinux/selinux.go @@ -48,6 +48,21 @@ var ( privContainerMountLabel string ) +// ProcessKind selects which process domain [SetProcessKind] applies to a label. +type ProcessKind int + +const ( + ProcessKindRegular ProcessKind = 1 + ProcessKindInit ProcessKind = 2 + ProcessKindKVM ProcessKind = 3 +) + +// SetProcessKind returns label with its type component replaced by the one +// corresponding to kind. Other label components are kept intact. +func SetProcessKind(label string, kind ProcessKind) (string, error) { + return setProcessKind(label, kind) +} + // Context is a representation of the SELinux label broken into 4 parts type Context map[string]string @@ -231,6 +246,7 @@ func ReserveLabel(label string) { } // ReserveLabelV2 reserves the MLS/MCS level component of the specified label. +// Labels without MLS/MCS category component (":c") are ignored. // Returns an error if the label can't be reserved. // // Callers that are intentionally reusing an existing level/MCS (e.g. multiple @@ -292,6 +308,8 @@ func KVMContainerLabels() (string, string) { // KVMContainerLabel returns the default process label to be used // for KVM containers by the calling process. +// +// If you only need to change a type of existing label, use [SetProcessKind] instead. func KVMContainerLabel() (string, error) { return kvmContainerLabel() } @@ -306,6 +324,8 @@ func InitContainerLabels() (string, string) { // InitContainerLabel returns the default process label to be used // for containers running an init system like systemd by the calling process. +// +// If you only need to change a type of existing label, use [SetProcessKind] instead. func InitContainerLabel() (string, error) { return initContainerLabel() } diff --git a/vendor/github.com/opencontainers/selinux/go-selinux/selinux_linux.go b/vendor/github.com/opencontainers/selinux/go-selinux/selinux_linux.go index 2117155701f8c..6ee2814a31726 100644 --- a/vendor/github.com/opencontainers/selinux/go-selinux/selinux_linux.go +++ b/vendor/github.com/opencontainers/selinux/go-selinux/selinux_linux.go @@ -890,8 +890,10 @@ func defaultEnforceMode() int { return Disabled } +// mcsAdd reserves a level. If the argument is empty or does not contain +// MCS/MLS category component (no ":c"), it is ignored. func mcsAdd(mcs string) error { - if mcs == "" { + if !strings.Contains(mcs, ":c") { return nil } state.Lock() @@ -1513,3 +1515,46 @@ func getDefaultContextWithLevel(user, level, scon string) (string, error) { return getDefaultContextFromReaders(&c) } + +func (k ProcessKind) keys() (primary, fallback string, ok bool) { + switch k { + case ProcessKindRegular: + return "process", "", true + case ProcessKindInit: + return "init_process", "process", true + case ProcessKindKVM: + return "kvm_process", "process", true + } + return "", "", false +} + +func setProcessKind(cLabel string, k ProcessKind) (string, error) { + if cLabel == "" { + return "", nil + } + primary, fallback, ok := k.keys() + if !ok { + return "", fmt.Errorf("selinux.SetProcessKind: invalid ProcessKind %d", k) + } + + src := label(primary) + if src == "" && fallback != "" { + src = label(fallback) + } + if src == "" { + return cLabel, nil + } + + // Replace cLabel type with one from src. + srcCtx, err := newContext(src) + if err != nil { + return "", fmt.Errorf("selinux.SetProcessKind: invalid %s label %s: %w", primary, src, err) + } + dstCtx, err := newContext(cLabel) + if err != nil { + return "", fmt.Errorf("selinux.SetProcessKind: invalid label %s: %w", cLabel, err) + } + + dstCtx["type"] = srcCtx["type"] + return dstCtx.get(), nil +} diff --git a/vendor/github.com/opencontainers/selinux/go-selinux/selinux_stub.go b/vendor/github.com/opencontainers/selinux/go-selinux/selinux_stub.go index 78a4e1fe3524a..d01bf2615e8ff 100644 --- a/vendor/github.com/opencontainers/selinux/go-selinux/selinux_stub.go +++ b/vendor/github.com/opencontainers/selinux/go-selinux/selinux_stub.go @@ -157,3 +157,7 @@ func getDefaultContextWithLevel(string, string, string) (string, error) { func label(_ string) string { return "" } + +func setProcessKind(string, ProcessKind) (string, error) { + return "", nil +} diff --git a/vendor/modules.txt b/vendor/modules.txt index e94c62cc69b7e..5dab4ab88cf2f 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -460,7 +460,7 @@ github.com/opencontainers/runtime-spec/specs-go/features github.com/opencontainers/runtime-tools/generate github.com/opencontainers/runtime-tools/generate/seccomp github.com/opencontainers/runtime-tools/validate/capabilities -# github.com/opencontainers/selinux v1.14.1 +# github.com/opencontainers/selinux v1.15.1 ## explicit; go 1.22 github.com/opencontainers/selinux/go-selinux github.com/opencontainers/selinux/go-selinux/label From 71bc89b2885fcb5c85e73457056a42c57b6d6897 Mon Sep 17 00:00:00 2001 From: Damien Grisonnet Date: Mon, 3 Aug 2026 16:36:58 +0200 Subject: [PATCH 4/5] cri: fix container_start_time_seconds unit conversion The container_start_time_seconds metric was reporting nanoseconds instead of Unix seconds. The CRI container status stores StartedAt as nanoseconds (per the CRI API spec), but the metric name and help text indicate seconds. Convert by dividing by time.Second. Signed-off-by: Damien Grisonnet --- internal/cri/server/list_pod_sandbox_metrics_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/cri/server/list_pod_sandbox_metrics_linux.go b/internal/cri/server/list_pod_sandbox_metrics_linux.go index 83f0cefb68cb2..6762b6245c194 100644 --- a/internal/cri/server/list_pod_sandbox_metrics_linux.go +++ b/internal/cri/server/list_pod_sandbox_metrics_linux.go @@ -275,7 +275,7 @@ func (c *criService) collectContainerMetrics(ctx context.Context, container cont Timestamp: timestamp, MetricType: runtime.MetricType_GAUGE, LabelValues: containerLabels, - Value: &runtime.UInt64Value{Value: uint64(container.Status.Get().StartedAt)}, + Value: &runtime.UInt64Value{Value: uint64(container.Status.Get().StartedAt / int64(time.Second))}, }, }...) From a35da471f36265ff4e4c62c2a47aa230aba49bb3 Mon Sep 17 00:00:00 2001 From: Maksym Pavlenko Date: Mon, 3 Aug 2026 14:35:33 -0700 Subject: [PATCH 5/5] unpack: don't drop topHalf errors in parallel mode When a layer fails to prepare during parallel unpack we break out of the launch loop but never return the error, so unpack() can report success and label the image with a chainID that was never created. Keep the error and return it once the already queued layers have been drained, so those still commit as they would in sequential mode. Also end the layer's tracing span, which leaked on this path. Signed-off-by: Maksym Pavlenko --- core/unpack/unpacker.go | 19 ++++++--- core/unpack/unpacker_test.go | 77 ++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 6 deletions(-) diff --git a/core/unpack/unpacker.go b/core/unpack/unpacker.go index d0313e80c821e..bff72b3338cae 100644 --- a/core/unpack/unpacker.go +++ b/core/unpack/unpacker.go @@ -616,7 +616,10 @@ func (u *Unpacker) unpack( return err } - var statusChans []<-chan *unpackStatus + var ( + statusChans []<-chan *unpackStatus + topErr error + ) for i, desc := range layers { _, layerSpan := tracing.StartSpan(ctx, tracing.Name(unpackSpanPrefix, "unpackLayer")) @@ -628,13 +631,16 @@ func (u *Unpacker) unpack( ) statusCh, err := topHalf(i, desc, layerSpan, unpackLayerStart) if err != nil { - if parallel { - break - } else { - layerSpan.SetStatus(err) - layerSpan.End() + layerSpan.SetStatus(err) + layerSpan.End() + if !parallel { return err } + // Layers queued before the failure still need to be drained and + // committed (or aborted) below, so remember the error and join it + // after the drain instead of returning right away. + topErr = err + break } if statusCh == nil { // nothing to do, already exists @@ -658,6 +664,7 @@ func (u *Unpacker) unpack( errs = errors.Join(errs, err) } } + errs = errors.Join(errs, topErr) if errs != nil { return errs } diff --git a/core/unpack/unpacker_test.go b/core/unpack/unpacker_test.go index cce256a9b8446..adbe76a023663 100644 --- a/core/unpack/unpacker_test.go +++ b/core/unpack/unpacker_test.go @@ -19,6 +19,7 @@ package unpack import ( "context" "crypto/rand" + "errors" "fmt" "reflect" "testing" @@ -322,3 +323,79 @@ func TestUnpackStagedLayers(t *testing.T) { assert.Equal(t, chainIDs[1].String(), sn.commits[1].name) assert.Equal(t, chainIDs[0].String(), sn.commits[1].parent) } + +// failPrepareSnapshotter stages every layer like stagedSnapshotter, except the +// Prepare of the layer at index failAt, which fails with a non-AlreadyExists +// error. +type failPrepareSnapshotter struct { + stagedSnapshotter + failAt int + // prepareCalls counts every Prepare, including the failing one, which + // stagedSnapshotter only records on success. Prepare is called from the + // layer launch loop alone, so this needs no synchronization. + prepareCalls int +} + +var errPrepareFailed = errors.New("prepare failed") + +func (s *failPrepareSnapshotter) Prepare(ctx context.Context, key, parent string, opts ...snapshots.Opt) ([]mount.Mount, error) { + layer := s.prepareCalls + s.prepareCalls++ + if layer == s.failAt { + return nil, errPrepareFailed + } + return s.stagedSnapshotter.Prepare(ctx, key, parent, opts...) +} + +// TestUnpackParallelPrepareError verifies that a topHalf failure in parallel +// mode is reported back from unpack instead of being silently dropped, while +// the layers queued before the failure are still committed. +func TestUnpackParallelPrepareError(t *testing.T) { + ctx := context.Background() + + diffIDs := generateRandomDiffIDs(t, 3) + chainIDs := identity.ChainIDs(append([]digest.Digest{}, diffIDs...)) + layers := []ocispec.Descriptor{ + {MediaType: ocispec.MediaTypeImageLayerGzip, Digest: digest.FromString("layer-0"), Size: 1}, + {MediaType: ocispec.MediaTypeImageLayerGzip, Digest: digest.FromString("layer-1"), Size: 1}, + {MediaType: ocispec.MediaTypeImageLayerGzip, Digest: digest.FromString("layer-2"), Size: 1}, + } + + cs := imagetest.NewContentStore(ctx, t) + config := cs.JSONObject(ocispec.MediaTypeImageConfig, struct { + ocispec.Platform + RootFS ocispec.RootFS `json:"rootfs"` + }{ + Platform: ocispec.Platform{OS: "linux", Architecture: "amd64"}, + RootFS: ocispec.RootFS{Type: "layers", DiffIDs: diffIDs}, + }).Descriptor + + sn := &failPrepareSnapshotter{failAt: 1} + u, err := NewUnpacker(ctx, cs.Store, + WithUnpackLimiter(semaphore.NewWeighted(4)), + WithUnpackPlatform(Platform{ + Platform: platforms.All, + Snapshotter: sn, + Applier: failApplier{t}, + SnapshotterCapabilities: []string{snapshots.RebaseCap}, + }), + ) + require.NoError(t, err) + + fetch := images.HandlerFunc(func(_ context.Context, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) { + t.Errorf("fetch must not happen for a staged layer (%s)", desc.Digest) + return nil, nil + }) + + err = u.unpack(fetch, config, layers) + require.ErrorIs(t, err, errPrepareFailed) + + // The launch loop stops at the failing layer: the third one is never + // prepared, and only the first was prepared successfully. + assert.Equal(t, 2, sn.prepareCalls) + require.Len(t, sn.prepares, 1) + + // The layer prepared before the failure is still committed. + require.Len(t, sn.commits, 1) + assert.Equal(t, chainIDs[0].String(), sn.commits[0].name) +}