[GIT] Digital signature library bugfix

[GIT] Digital signature library bugfix

[James Morris]

Please apply for v3.6.

The following changes since commit 0bd1189e239c76eb3a50e458548fbe7e4a5dfff1:
  Linus Torvalds (1):
        Merge branch 'for-3.6-fixes' of git://git.kernel.org/.../tj/wq

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git for-linus

Dmitry Kasatkin (1):
      digsig: add hash size comparision on signature verification

 lib/digsig.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

commit 83e7c8fb4347186f6723f4c7d176999becbb3830
Author: Dmitry Kasatkin <dmitry.kasatkin@intel.com>
Date:   Thu Sep 6 01:06:49 2012 +0300

    digsig: add hash size comparision on signature verification
    
    Commit b35e286a640f31d619a637332972498b51f3fd90 introduced the bug.
    
    When pkcs_1_v1_5_decode_emsa() returns without error and hash sizes do not
    match, hash comparision is not done and digsig_verify_rsa() returns no error.
    This is a bug and this patch fixes it.
    
    Cc: stable@vger.kernel.org
    Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@intel.com>
    Signed-off-by: James Morris <james.l.morris@oracle.com>

diff --git a/lib/digsig.c b/lib/digsig.c
index 286d558..77b1848 100644
--- a/lib/digsig.c
+++ b/lib/digsig.c
@@ -164,8 +164,12 @@ static int digsig_verify_rsa(struct key *key,
 
 	err = pkcs_1_v1_5_decode_emsa(out1, len, mblen, out2, &len);
 
-	if (!err && len == hlen)
-		err = memcmp(out2, h, hlen);
+	if (err || len != hlen) {
+		err = -EINVAL;
+		goto err;
+	}
+
+	err = memcmp(out2, h, hlen);
 
 err:
 	mpi_free(in);

Re: [GIT] Digital signature library bugfix

[Linus Torvalds]

On Wed, Sep 12, 2012 at 11:34 AM, James Morris <jmorris@namei.org> wrote:
>
> -       if (!err && len == hlen)
> -               err = memcmp(out2, h, hlen);
> +       if (err || len != hlen) {
> +               err = -EINVAL;
> +               goto err;
> +       }
> +
> +       err = memcmp(out2, h, hlen);
>
>  err:

Hmm. I'll pull, but this seems to drop the error return from
pkcs_1_v1_5_decode_emsa() and always replace it with -EINVAL.

Now, I didn't look, and maybe that's the only error that the decode
thing can return, but still, it looks bad.

Wouldn't it have been better to do instead

   if (err)
      goto err;
   err = -EINVAL;
   if (len != hlen)
      goto err;

and not overwrite the 'err' return with EINVAL?

             Linus

Re: [GIT] Digital signature library bugfix

[Kasatkin, Dmitry]

On Wed, Sep 12, 2012 at 8:38 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Wed, Sep 12, 2012 at 11:34 AM, James Morris <jmorris@namei.org> wrote:
>>
>> -       if (!err && len == hlen)
>> -               err = memcmp(out2, h, hlen);
>> +       if (err || len != hlen) {
>> +               err = -EINVAL;
>> +               goto err;
>> +       }
>> +
>> +       err = memcmp(out2, h, hlen);
>>
>>  err:
>
> Hmm. I'll pull, but this seems to drop the error return from
> pkcs_1_v1_5_decode_emsa() and always replace it with -EINVAL.
>
> Now, I didn't look, and maybe that's the only error that the decode
> thing can return, but still, it looks bad.
>
> Wouldn't it have been better to do instead
>
>    if (err)
>       goto err;
>    err = -EINVAL;
>    if (len != hlen)
>       goto err;
>
> and not overwrite the 'err' return with EINVAL?
>

Error code here just means success or failure.
I did not like it either, but that was just to fix a bug (cc:stable)
and I have one patch more to refactor that stuff for mainline kernel.

But I will re-send updated patch in a moment.

Thanks.

>              Linus
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT] Digital signature library bugfix

[Linus Torvalds]

On Wed, Sep 12, 2012 at 6:22 PM, Kasatkin, Dmitry
<dmitry.kasatkin@intel.com> wrote:
>
> But I will re-send updated patch in a moment.

Ok, I took that updated patch instead of the pull request, since I
liked that much more, and hadn't actually pushed out the pull yet.

           Linus

Re: [GIT] Digital signature library bugfix

[James Morris]

On Thu, 13 Sep 2012, Linus Torvalds wrote:

> On Wed, Sep 12, 2012 at 6:22 PM, Kasatkin, Dmitry
> <dmitry.kasatkin@intel.com> wrote:
> >
> > But I will re-send updated patch in a moment.
> 
> Ok, I took that updated patch instead of the pull request, since I
> liked that much more, and hadn't actually pushed out the pull yet.

Thanks.



-- 
James Morris
<jmorris@namei.org>

Re: [GIT] Digital signature library bugfix

[Kasatkin, Dmitry]

On Thu, Sep 13, 2012 at 7:53 AM, James Morris <jmorris@namei.org> wrote:
> On Thu, 13 Sep 2012, Linus Torvalds wrote:
>
>> On Wed, Sep 12, 2012 at 6:22 PM, Kasatkin, Dmitry
>> <dmitry.kasatkin@intel.com> wrote:
>> >
>> > But I will re-send updated patch in a moment.
>>
>> Ok, I took that updated patch instead of the pull request, since I
>> liked that much more, and hadn't actually pushed out the pull yet.
>
> Thanks.
>
>
>
> --
> James Morris
> <jmorris@namei.org>

Thanks.