Hello,

Am Donnerstag, 13. Februar 2014 schrieb Tomáš Bažant:
> i have a new version of our common work - it's both committed in svn
> and attached as an updated PDF for you convenience. it would be great
> if at least one of you can have a look if it's in a good shape - i
> believe it's for the last time in this documentation cycle, other
> topics for SLE 12 are round the corner...

You invented something totally new - a manual with a bugreport already 
included ;-)

I also found some other (less funny) errors, and some new errors you 
introduced in the last editing round.
(Just applying the patch will already fix some issues.)

On the positive side, the patches are getting shorter. The initial 
review had 1242 lines, for r12028 it had 367 lines and now we are at 284 
lines.

BTW: I did this review mostly based on the svn diff, and checked the PDF 
only in rare cases (like changes of bigger sections). In other words: 
You should [ask someone to] have a look at the PDF and check things like 
superfluous empty lines in <screen> blocks, strange linebreaks (there's 
still "Ap- \n pArmor") etc.

> btw thanks a lot for you previous comments

You are welcome ;-)

BTW: Did I mention that the work I do for the SLE manual is licensed 
under the wineware licence? [1]

If we ever meet at a conference [2], I'll bill you ;-))


Regards,

Christian Boltz

[1] similar to the beerware license, but s/beer/wine/ because I don't 
    like beer ;-)

[2] unfortunately I won't be able to visit oSC14 :-(

-- 
>Gibt es hier in dieser Liste eigentlich ausser mir noch jemanden ??
Nein, aber es laufen einige Robots, die Traffic vortäuschen. Ich bin
auch einer davon.
[Tobias Korb und Thorsten Haude in suse-programming]
Index: xml/apparmor_changehat.xml
===================================================================
--- xml/apparmor_changehat.xml	(Revision 12090)
+++ xml/apparmor_changehat.xml	(Arbeitskopie)
@@ -422,6 +422,9 @@
      Note the <literal>(complain)</literal> addition after the hat name - it
      tells &aa; to leave the adminer hat in complain mode. That is because
      we need to learn the hat profile by accessing Adminer later on.
+<!-- did you test this yourself? I remember a bug on older versions that caused some problems 
+	 when the main profile was enforced and only a hat in complain mode, but maybe this was 
+	 accidently ;-) fixed in the meantime -->
     </para>
    </step>
    <step>
@@ -432,7 +435,7 @@
    </step>
    <step>
     <para>
-     Check if the adminer hat really is in a complain mode.
+     Check if the adminer hat really is in complain mode.
     </para>
 <screen># aa-status 
 apparmor module is loaded.
Index: xml/apparmor_profiles.xml
===================================================================
--- xml/apparmor_profiles.xml	(Revision 12090)
+++ xml/apparmor_profiles.xml	(Arbeitskopie)
@@ -9,7 +9,7 @@
  <title>Profile Components and Syntax</title>
  <para>
   Building &aa; profiles to confine an application is very straightforward
-  and intuitive. aa; ships with several tools that assist in profile
+  and intuitive. &aa; ships with several tools that assist in profile
   creation. It does not require you to do any programming or script
   handling. The only task that is required of the administrator is to
   determine a policy of strictest access and execute permissions for each
@@ -487,13 +487,14 @@
    <literal>#</literal>.
   </para>
 
-<screen>include "/etc/apparmor.d/foo"</screen>
-
+<screen>include "/etc/apparmor.d/abstractions/foo"</screen>
+<!-- (abstractions/ is where most includes live, and /etc/apparmor.d/ itsself usually contains full profiles, not something you want to #include -->
   <para>
    is the same as using
   </para>
 
-<screen>#include "/etc/apparmor.d/foo"</screen>
+<screen>#include "/etc/apparmor.d/abstractions/foo"</screen>
+<!-- (abstractions/ is where most includes live, and /etc/apparmor.d/ itsself usually contains full profiles, not something you want to #include -->
 
   <note>
    <title>No Trailing ','</title>
@@ -509,17 +510,20 @@
    to do an absolute or relative path lookup.
   </para>
 
-<screen>include "/etc/apparmor.d/foo" # absolute path
-include "foo"                 # relative path to the directory of current file</screen>
+<screen>include "/etc/apparmor.d/abstractions/foo" # absolute path
+include "abstractions/foo"                 # relative path to the directory of current file</screen>
+<!-- (abstractions/ is where most includes live, and /etc/apparmor.d/ itsself usually contains full profiles, not something you want to #include -->
 
   <para>
    Please note that when using relative path includes, once the file is
    included, it is considered the new current file for its includes. For
    example, suppose you are in the <filename>/etc/apparmor.d/foo</filename>
+   <!-- (you might want to use something else instead of foo to avoid confusion) -->
    file, then
   </para>
 
 <screen>include "abstractions/bar"</screen>
+<!-- (you might want to use abstractions/foo for consistency) -->
 
   <para>
    includes the file <filename>/etc/apparmor.d/abstractions/bar</filename>.
@@ -1004,6 +1008,12 @@
 @{HOME}=@{HOMEDIRS}/*/ /root/
 /path/to/profiled/binary {
   @{HOMEDIRS}+=/srv/nfs/home/ /mnt/home/
+<!-- AFAIK variable definitions/changes must be outside the {...} profile block (However I never tested inside a profile) 
+
+The usual use case is:
+- variable definition in tunables/foo
+- additions in tunable/morefoo or in the profile (above the profile itsself)
+-->
   [...]
 }</screen>
    <note>
@@ -1566,8 +1576,14 @@
    <para>
     Use the lower-cased versions of exec modes&mdash; <literal>px</literal>,
     <literal>cx</literal>, <literal>ux</literal>&mdash;only in very special
-    cases. They enable the designated child processes to be run without any
-    &aa; protection. They do not scrub the environment of variables such as
+    cases. 
+<!--
+	They enable the designated child processes to be run without any
+    &aa; protection. 
+
+This sentense sound a bit too terrible (with the obvious exception of ux - maybe just remove it?
+-->	
+	They do not scrub the environment of variables such as
     <envar>LD_PRELOAD</envar>. As a result, the calling domain may have an
     undue amount of influence over the called resource. Use these modes only
     if the child absolutely <emphasis>must</emphasis> be run unconfined and
@@ -1720,15 +1736,17 @@
    <para>
     The same applies if you add the unconfined <literal>ux</literal> mode,
     where the resulting modes are <literal>cux</literal>,
-    <literal>Cux</literal>, <literal>pux</literal> and
-    <literal>Pux</literal>. These modes allow falling back to
+    <literal>CUx</literal>, <literal>pux</literal> and
+<!-- don't mix uppercase and lowercase - it will probably result in a parser failure 
+	 (the obvious exception is "i", which is only available in lowercase) -->
+    <literal>PUx</literal>. These modes allow falling back to
     <quote>unconfined</quote> when the specified profile is not found.
    </para>
-<screen>/path Pux -> profile_name,</screen>
+<screen>/path PUx -> profile_name,</screen>
    <para>
     or
    </para>
-<screen>Pux /path -> profile_name,</screen>
+<screen>PUx /path -> profile_name,</screen>
    <para>
     where <literal>-> profile_name</literal> is optional.
    </para>
Index: xml/apparmor_profiles_man.xml
===================================================================
--- xml/apparmor_profiles_man.xml	(Revision 12090)
+++ xml/apparmor_profiles_man.xml	(Arbeitskopie)
@@ -27,7 +27,7 @@
   <title>Checking the &aa; Status</title>
 
   <para>
-   The &aa; can be in any one of three states:
+   &aa; can be in any one of three states:
   </para>
 
   <variablelist>
@@ -85,6 +85,9 @@
       state. If &aa; is already running, <option>start</option> reports a
       warning and takes no action.
      </para>
+
+<!-- A note that running processes need to be restarted to apply the profiles would be nice. -->
+
     </listitem>
    </varlistentry>
    <varlistentry>
@@ -543,7 +546,7 @@
       Re-scan all profiles.
      </para>
      <para>
-      To have &aa; r all of the profiles and change the enforcement mode in
+      To have &aa; rescan all of the profiles and change the enforcement mode in
       the kernel, enter <command>rcapparmor reload</command>.
      </para>
     </step>
@@ -869,6 +872,7 @@
 	  os="slert">&slert;</phrase>,
          the default full path is
          <phrase><filename>/usr/sbin/httpd2-prefork</filename></phrase>.
+<!-- results in funny linebreak  ("/ \n usr/...")-->
         </para>
        </listitem>
        <listitem>
@@ -1206,7 +1210,7 @@
           <term>Deny</term>
           <listitem>
            <para>
-            Add a <literal>deny</literal> rule to the profile, and
+            Adds a <literal>deny</literal> rule to the profile, and
             permanently prevents the program from accessing the specified
             directory path entries. &aa; then continues to the next event.
            </para>
@@ -1236,6 +1240,8 @@
         <para>
          <xref linkend="ex.apparmor.commandline.profiling.summary.genprof.perms"/>
          shows &aa; suggest allowing a globbing pattern
+<!-- funny linebreak: "Ap- \n pArmor 
+	 (should be fixed on the global level, not (only) here -->
          <filename>/var/run/nscd/*</filename> for reading, then using an
          abstraction to cover common Apache related access rules.
         </para>
@@ -1245,12 +1251,16 @@
 Path:     /var/run/nscd/dbSz9CTr
 Mode:     r
 Severity: unexpected rank input: var/run/nscd/dbSz9CTr
-
+<!--      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+		  oops - that's the result of a disconnected path (and a small bug in logprof)
+		  Please replace it with "3"
+		  (just for the record: that's a case where flags=(attach_disconnected) would be helpful)
+		  -->
   1 - /var/run/nscd/dbSz9CTr 
  [2 - /var/run/nscd/*]
 
 (A)llow / [(D)eny] / (G)lob / Glob w/(E)xt / (N)ew / Abo(r)t / (F)inish / (O)pts
-Adding var/run/nscd/* r to profile.
+Adding /var/run/nscd/* r to profile.
 
 Profile:  /usr/sbin/httpd2-prefork
 Path:     /proc/11769/attr/current
@@ -1515,7 +1525,7 @@
     </para>
     <para>
      When <command>aa-logprof</command> exits, profiles are updated with the
-     changes. If the &aa; is active, the updated profiles are reloaded and,
+     changes. If &aa; is active, the updated profiles are reloaded and,
      if any processes that generated security events are still running in
      the null-XXX, those processes are set to run under their proper
      profiles.
@@ -1942,11 +1952,13 @@
 syntax on
 set modeline
 set modelines=5
+<!-- I'm afraid the security team won't like this. (There was a security update to explicitely disable modelines some years ago!)
+	 Additionally, you miss a note that the profiles need to contain "# vim:ft=apparmor" to get this working -->
 </screen>
     <tip>
      <para>
       <command>vim</command> comes with &aa; highlighting automatically
-      enabled for files in /etc/apparmor.d/ so modlines might not be needed.
+      enabled for files in /etc/apparmor.d/ so modelines might not be needed.
      </para>
     </tip>
     <para>
@@ -1961,6 +1973,9 @@
         <literal>#include</literal> lines that pull in other &aa; rules I'm
         quite sure #include is no longer blue... and comments that begin
         with <literal>#</literal>
+<!-- looks like a comment made it into the user-visible text... 
+	 (and yes, #include has a color different from comments)
+	 -->
        </para>
       </listitem>
      </varlistentry>
@@ -2021,6 +2036,8 @@
       attacker to send you a file to open with vim that might do something
       unsafe. This paragraph should be next to the paragraph about
       modelines, not here
+<!-- What about _acting_ as described in the last two lines instead of including them in the paragraph?
+	 OTOH, I like the idea to ship the manual with the first bugreport already included ;-) -->
      </para>
     </note>
     <para>
@@ -2027,6 +2044,7 @@
      Use the <systemitem>apparmor.vim</systemitem> and
      <systemitem>vim</systemitem> man pages and the <option>:help</option>
      syntax from within the vim editor for further vim help about syntax
+<!-- that should be ... <option>:help syntax</option> from within... -->
      highlighting. The &aa; syntax is stored in
      <filename>/usr/share/vim/current/syntax/apparmor.vim.</filename>
     </para>
Index: xml/apparmor_start.xml
===================================================================
--- xml/apparmor_start.xml	(Revision 12090)
+++ xml/apparmor_start.xml	(Arbeitskopie)
@@ -199,7 +199,7 @@
   <procedure>
    <step>
     <para>
-     Start &yast; select <guimenu>&aa; Configuration</guimenu>, and click
+     Start &yast;, select <guimenu>&aa; Configuration</guimenu>, and click
      <guimenu>Settings</guimenu> in the main window.
     </para>
    </step>
Index: xml/security_docupdates.xml
===================================================================
--- xml/security_docupdates.xml	(Revision 12090)
+++ xml/security_docupdates.xml	(Arbeitskopie)
@@ -127,7 +127,8 @@
      <itemizedlist>
       <listitem>
        <para>
-        New <literal>Pux/pux</literal> and <literal>Cux/cux</literal>
+        New <literal>PUx/pux</literal> and <literal>CUx/cux</literal>
+<!-- Pux / Cux (mixed upper-/lowercase) will fail parsing -->
         profile transitions added in
         <xref linkend="sec.apparmor.profiles.exec.fallback"/>.
        </para>
-- 
AppArmor mailing list
[email protected]
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to