Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190607002330.2999-1-prsriva02@gmail.com> <1560267426.4464.173.camel@linux.ibm.com>
In-Reply-To: <1560267426.4464.173.camel@linux.ibm.com>
From:   prakhar srivastava <prsriva02@gmail.com>
Date:   Tue, 11 Jun 2019 11:48:58 -0700
Message-ID: <CAEFn8q+RgSnJaWAJT+xN-CZWCsaOVqrPunT3icd_ua2bpkp_SQ@mail.gmail.com>
Subject: Re: [PATCH v7 0/3] add new ima hook ima_kexec_cmdline to measure
 kexec boot cmdline args
To:     Mimi Zohar <zohar@linux.ibm.com>
Cc:     linux-integrity <linux-integrity@vger.kernel.org>,
        linux-security-module@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Roberto Sassu <roberto.sassu@huawei.com>, vgoyal@redhat.com
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Jun 11, 2019 at 8:37 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> Hi Prakhar,
>
> The patch/patch set title in the Subject line should not explain "how"
> you add a new feature.  In this case an appropriate patch set title
> would be, "Add support for measuring the boot command line".
>  Similarly, the first patch in this patch set could be named "Define a
> new IMA hook to measure the boot command line arguments".
>
> On Thu, 2019-06-06 at 17:23 -0700, Prakhar Srivastava wrote:
> > The motive behind the patch series is to measure the boot cmdline args
> > used for soft reboot/kexec case.
>
> When mentoring, I suggest starting out with a simple status statement
> (eg. "The kexec boot command line arguments are not currently being
> measured."), followed by the problem statement in the first paragraph.
>
> >
> > For secure boot attestation, it is necessary to measure the kernel
>
> Secure boot enforces local file data integrity.  The term here should
> be "trusted boot attestation".
>
> > command line and the kernel version.
>
> The original version of this patch set included the kernel version.
> This version is just measuring the boot command line arguments.
>
Sorry missed it while updating the cover letter.
<snip>

> > The ima logs need to be carried over to the next kernel, which will be followed
> > up by other patchsets for x86_64 and arm64.
> >
> > The kexec cmdline hash
>
> ^stored in the "d-ng" field of the template data
>
I will add another template-name for ima-buf
> > can be verified using
>
> > sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements |
> >   grep  kexec-cmdline | cut -d' ' -f 6 | xxd -r -p | sha256sum
>
> Until per policy template field rule support is added, a template name
> needs to be defined.  Please define "ima-buf" as:
> {.name = "ima-buf", .fmt = "d-ng|n-ng|buf"}
>
> I'm still seeing some scripts/checkpatch "WARNING: line over 80
> characters".  scripts/Lindent should provide the correct way of
> formatting these lines.
>
> Some people feel that references to Lindent should be removed, but I
> tend to agree with the Documentation/hwmon/submitting-patches.rst
> comment pertaining to scripts/Lindent.
>
> "* Running your patch or driver file(s) through checkpatch does not
> mean its formatting is clean. If unsure about formatting in your new
> driver, run it through Lindent. Lindent is not perfect, and you may
> have to do some minor cleanup, but it is a good start."
>
I will double check fix the issues.
> Examples of where the line formatting is off is the call to
> ima_get_action() in process_buffer_measurement() and the call to
> process_buffer_measurement() in ima_kexec_cmdline().
>
Thanks,
Prakhar Srivastava
> thanks,
>
> Mimi
<snip>