apparmor: align quoting with libapparmor by thaJeztah · Pull Request #44 · moby/profiles

thaJeztah · 2026-09-23T22:12:02Z

relates to contrib/apparmor: align whitespace parsing with libapparmor containerd/containerd#13278 (comment)
updates apparmor: align whitespace parsing with libapparmor #29

apparmor: splitCon: touch-up GoDoc

apparmor: align profile-name quoting with libapparmor

Commit 47d6e85 updated the template to
quote profile names. While the manpage (apparmor.d(5)) documents quoted
names, it is unclear how quoting must be handled:

PROFILE NAME = UNQUOTED PROFILE NAME | QUOTED PROFILE NAME

QUOTED PROFILE NAME = '"' UNQUOTED PROFILE NAME '"'

UNQUOTED PROFILE NAME = (must start with alphanumeric character (after
variable expansion), or '/' AARE have special meanings; see below. May
include VARIABLE. Rules with embedded spaces or tabs must be quoted.)

The lexer in parser/parser_lex.l further describes the syntax of quoted
identifiers:

ALLOWED_QUOTED_ID  [^\0"]|\\\"
QUOTED_ID          \"{ALLOWED_QUOTED_ID}*\"

However, quoting alone does not preserve a profile name literally.
processunquoted decodes recognized escape sequences and preserves
escaping for AARE special characters so that they remain literal when
processed by the pattern-matching backend. In particular, backslashes
must be escaped to prevent sequences such as \n from being decoded,
and AARE special characters such as *, ?, [], and {} must be
escaped to prevent them from acquiring pattern semantics. See
processunquoted and strn_escseq.

Update profile-name quoting to account for both AppArmor escape sequences
and AARE special characters, preserving the profile name literally. NUL
values are not handled, as they are not valid in quoted identifiers and
should not reach this code.

Also update profileData to provide accessors for its fields, allowing
profile names to be quoted by those methods instead of exposing raw
values directly to the template.

This also fixes handling of the unconfined daemon profile. The previous
quoting change rendered it as "unconfined", which changes its AppArmor
semantics from the special unconfined selector to a literal profile-name
expression. Keep this sentinel unquoted while continuing to quote actual
profile names.

Updates 47d6e85

Copilot

Copilot review overview

🟢 Approval recommended

Only a non-blocking GoDoc clarification remains.

Review effort: Balanced
Findings: None

Resolved since last review (1)

Iterate profile names by bytes to preserve invalid UTF-8

vvoland · 2026-09-24T10:30:24Z

I think profile declarations and peer patterns need different escaping here.
.Name is used for both, but AppArmor preserves AARE escapes and serializes the profile name as-is.

Opened thaJeztah#1 for consideration

thaJeztah · 2026-09-25T08:18:47Z

Making some extra changes inspired by your "let's also check what apparmor_parser says about this" - I'll give you a nudge when done

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Commit 47d6e85 updated the template to quote profile names. While the manpage ([apparmor.d(5)]) documents quoted names, it is unclear how quoting must be handled: PROFILE NAME = UNQUOTED PROFILE NAME | QUOTED PROFILE NAME QUOTED PROFILE NAME = '"' UNQUOTED PROFILE NAME '"' UNQUOTED PROFILE NAME = (must start with alphanumeric character (after variable expansion), or '/' AARE have special meanings; see below. May include VARIABLE. Rules with embedded spaces or tabs must be quoted.) The lexer in [parser/parser_lex.l] further describes the syntax of quoted identifiers: ALLOWED_QUOTED_ID [^\0"]|\\\" QUOTED_ID \"{ALLOWED_QUOTED_ID}*\" However, quoting alone does not preserve a profile name literally. [processunquoted] decodes recognized escape sequences and preserves escaping for AARE special characters so that they remain literal when processed by the pattern-matching backend. In particular, backslashes must be escaped to prevent sequences such as `\n` from being decoded, and AARE special characters such as `*`, `?`, `[]`, and `{}` must be escaped to prevent them from acquiring pattern semantics. See [processunquoted] and [strn_escseq]. Update profile-name quoting to account for both AppArmor escape sequences and AARE special characters, preserving the profile name literally. NUL values are not handled, as they are not valid in quoted identifiers and should not reach this code. Also update profileData to provide accessors for its fields, allowing profile names to be quoted by those methods instead of exposing raw values directly to the template. This also fixes handling of the `unconfined` daemon profile. The previous quoting change rendered it as `"unconfined"`, which changes its AppArmor semantics from the special unconfined selector to a literal profile-name expression. Keep this sentinel unquoted while continuing to quote actual profile names. Updates 47d6e85 [apparmor.d(5)]: https://manpages.ubuntu.com/manpages/xenial/man5/apparmor.d.5.html [parser/parser_lex.l]: https://gitlab.com/apparmor/apparmor/-/blob/v5.0.2/parser/parser_lex.l#L286-287 [processunquoted]: https://gitlab.com/apparmor/apparmor/-/blob/v5.0.2/parser/parser_misc.c#L468-507 [strn_escseq]: https://gitlab.com/apparmor/apparmor/-/blob/v5.0.2/parser/lib.c#L178-218 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah · 2026-09-25T12:36:39Z

+		{
+			name: "with-special-characters",
+			data: profileData{
+				name:          `foo"bar,*?[ab]{c,d}^\baz`,
+				daemonProfile: `daemon"bar,*?[ab]{c,d}^\baz`,
+			},
+		},


Added a test-case here as well, so that the before/after is more apparent. Test passes "before", but because it just takes the quoted profile as a name.

thaJeztah · 2026-09-25T12:37:06Z

 @{PROC}=/proc/

-profile "foo\"bar\,\*\?\[ab\]\{c\,d\}\^\\baz" flags=(attach_disconnected,mediate_deleted) {
+profile "foo\"bar,*?[ab]{c,d}^\baz" flags=(attach_disconnected,mediate_deleted) {


And this shows the diff clearly before/after 👍

thaJeztah · 2026-09-25T12:39:47Z

+	// Declarations retain AARE escapes, while peer patterns consume them.
+	for _, want := range []string{
+		`profile "foo\"bar,*?[ab]{c,d}^\baz" flags=(attach_disconnected,mediate_deleted) {`,
+		`  signal (receive) peer="daemon\,profile\\baz",`,
+		`  signal (send,receive) peer="foo\"bar\,\*\?\[ab\]\{c\,d\}\^\\baz",`,
+		`  ptrace (trace,tracedby,read,readby) peer="foo\"bar\,\*\?\[ab\]\{c\,d\}\^\\baz",`,
+	} {
+		if !strings.Contains(out.String(), "\n"+want+"\n") {
+			t.Errorf("generated profile is missing line %q", want)
+		}
+	}


Oh, perhaps this is redundant now, because I added it to the test-table 🤔

I removed this one 😅

(but probably we could move the test-table to a non-linux file, or just make it non-linux; most tests should be testable cross-platform)

AARE escaping changed the declared profile name. Keep declaration quoting separate from peer-pattern escaping. Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah requested a lite review from Copilot September 23, 2026 22:12

Copilot started reviewing on behalf of thaJeztah September 23, 2026 22:12 View session

thaJeztah force-pushed the apparmor_improve_whitespace_handling branch from e6c37cd to aed3cd0 Compare September 23, 2026 22:14

This comment was marked as resolved.

Sign in to view

thaJeztah requested a balanced review from Copilot September 23, 2026 22:18

Copilot started reviewing on behalf of thaJeztah September 23, 2026 22:18 View session

This comment was marked as resolved.

Sign in to view

thaJeztah force-pushed the apparmor_improve_whitespace_handling branch from aed3cd0 to d9e227e Compare September 23, 2026 22:54

thaJeztah mentioned this pull request Sep 23, 2026

contrib/apparmor: align whitespace parsing with libapparmor containerd/containerd#13278

Open

thaJeztah requested a balanced review from Copilot September 23, 2026 23:11

Copilot started reviewing on behalf of thaJeztah September 23, 2026 23:11 View session

This comment was marked as outdated.

Sign in to view

thaJeztah force-pushed the apparmor_improve_whitespace_handling branch 2 times, most recently from 7be3f28 to e30519d Compare September 23, 2026 23:24

thaJeztah requested a balanced review from Copilot September 23, 2026 23:27

Copilot started reviewing on behalf of thaJeztah September 23, 2026 23:27 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

thaJeztah requested a review from vvoland September 24, 2026 00:31

vvoland mentioned this pull request Sep 24, 2026

apparmor: Separate profile and peer escaping thaJeztah/profiles#1

Merged

thaJeztah force-pushed the apparmor_improve_whitespace_handling branch from 62df3d8 to afd0b8f Compare September 25, 2026 07:45

vvoland approved these changes Sep 25, 2026

View reviewed changes

thaJeztah added 2 commits September 25, 2026 14:35

apparmor: splitCon: touch-up GoDoc

aa254c7

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah force-pushed the apparmor_improve_whitespace_handling branch from afd0b8f to ad1e7de Compare September 25, 2026 12:35

thaJeztah commented Sep 25, 2026

View reviewed changes

apparmor: separate profile and peer escaping

f0494f1

AARE escaping changed the declared profile name. Keep declaration quoting separate from peer-pattern escaping. Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah force-pushed the apparmor_improve_whitespace_handling branch from ad1e7de to f0494f1 Compare September 25, 2026 12:42

vvoland approved these changes Sep 25, 2026

View reviewed changes

thaJeztah merged commit 85e237f into moby:main Sep 25, 2026
10 checks passed

thaJeztah deleted the apparmor_improve_whitespace_handling branch September 25, 2026 13:07

vvoland mentioned this pull request Sep 30, 2026

apparmor: Fix stale special-character profile expectation #47

Merged

Navigation Menu

apparmor: align quoting with libapparmor - #44

apparmor: align quoting with libapparmor#44
thaJeztah merged 3 commits into
moby:mainfrom
thaJeztah:apparmor_improve_whitespace_handling

thaJeztah commented Sep 23, 2026 •

edited

Loading

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Uh oh!

vvoland commented Sep 24, 2026 •

edited

Loading

Uh oh!

thaJeztah commented Sep 25, 2026

Uh oh!

thaJeztah Sep 25, 2026

Uh oh!

thaJeztah Sep 25, 2026

Uh oh!

thaJeztah Sep 25, 2026

Uh oh!

thaJeztah Sep 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

thaJeztah commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

apparmor: splitCon: touch-up GoDoc

apparmor: align profile-name quoting with libapparmor

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

vvoland commented Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Sep 25, 2026

Uh oh!

thaJeztah Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

thaJeztah Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

thaJeztah Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

thaJeztah Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thaJeztah commented Sep 23, 2026 •

edited

Loading

vvoland commented Sep 24, 2026 •

edited

Loading