Bug 93764

On 23/2/06 19:16, "Daniel Gerzo" <danger@rulez.sk> wrote:

Daniel,

Welcome to your first proper review by me.  We hope that you don't squirm
too much. :)

> @@ -2283,7 +2283,50 @@
>  
>        <para>Set the script to run to activate your rules:</para>
>  
> -      <programlisting>firewall_script="/etc/ipfw.rules"</programlisting>
> +      <programlisting>firewall_script="/etc/rc.firewall"</programlisting>

That's the default, so perhaps we can get away with not specifying this all.

> +
> +      <para>Set the type of firewall.  This enables a simple pre-set
> + ruleset for <application>IPFW</application>:</para>
> +      
> +      <programlisting>firewall_type="open"</programlisting>
> +
> +      <para>Available values for this setting are:</para>

This is the handbook.  We can get away with an actual paragraph explaining
this pre-set ruleset thing.  I think that the above is too brief and
presupposes that I know what you're saying already.


> +      <itemizedlist>
> + <listitem>
> +   <para><literal>open</literal> &mdash; allow anyone in.</para>
> + </listitem>

That text isn't good.  "Pass all traffic" perhaps?

> + <listitem>
> +   <para><literal>client</literal> &mdash; will protect only this
> +     machine.</para>
> + </listitem>
> + <listitem>
> +   <para><literal>simple</literal> &mdash; protect the whole
> +     network.</para>

Do what now?  So I can run this on a FreeBSD machine and do without a
firewall?  Sweet!  Seriously though, "protect the whole network" is
misleading in the absence of context.

> + </listitem>
> + <listitem>
> +   <para><literal>closed</literal> &mdash; entirely disables IP
> +     services except via lo0 interface.</para>

s/services/traffic/, s/via/via the/

> + </listitem>
> + <listitem>
> +   <para><literal>UNKNOWN</literal> &mdash; disables the loading
> +     of firewall rules.</para>
> + </listitem>
> + <listitem>
> +   <para><replaceable>filename</replaceable> &mdash; will load the rules
> +     in the given filename (full path required).</para>

s/full/absolute/

> + </listitem>
> +      </itemizedlist>
> +
> +      <note>
> + <para>If <literal>firewall_type</literal> is set either to
> +   <literal>client</literal> or <literal>simple</literal>, the
> +   default rules found in <filename>/etc/rc.firewall</filename>
> +   should be reviewed to fit to the setup of the given machine.

Try to avoid the word "setup" if you can.  Try one of "purpose",
"configuration", "services", "function", etc.

> +   Also note, that the examples used in this chapter expect that

That comma doesn't belong there.

> +   the <literal>firewall_type</literal> is set to
> +   <filename>/etc/ipfw.rules</filename>.</para>
> +      </note>

Ceri
-- 
That must be wonderful!  I don't understand it at all.
                                      -- Moliere

Comment 2 jcamou 2006-03-03 14:30:50 UTC

Responsible Changed
From-To: freebsd-doc->jcamou

Grab pr.

Comment 3 jcamou 2006-08-21 14:37:51 UTC

Responsible Changed
From-To: jcamou->danger

Over to submitter now that he's got a commit bit.

Comment 4 Daniel Gerzo 2006-08-26 00:55:03 UTC

State Changed
From-To: open->closed

I commited a slightly modified diff into our tree.