lkml.org 
[lkml]   [2016]   [Oct]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.4 087/112] PKCS#7: Dont require SpcSpOpusInfo in Authenticode pkcs7 signatures
    Date
    4.4-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Peter Jones <pjones@redhat.com>

    commit 7ee7014d0eb6bcac679c0bd5fe9ce65bc4325648 upstream.

    Dave Young reported:
    > Hi,
    >
    > I saw the warning "Missing required AuthAttr" when testing kexec,
    > known issue? Idea about how to fix it?
    >
    > The kernel is latest linus tree plus sevral patches from Toshi to
    > cleanup io resource structure.
    >
    > in function pkcs7_sig_note_set_of_authattrs():
    > if (!test_bit(sinfo_has_content_type, &sinfo->aa_set) ||
    > !test_bit(sinfo_has_message_digest, &sinfo->aa_set) ||
    > (ctx->msg->data_type == OID_msIndirectData &&
    > !test_bit(sinfo_has_ms_opus_info, &sinfo->aa_set))) {
    > pr_warn("Missing required AuthAttr\n");
    > return -EBADMSG;
    > }
    >
    > The third condition below is true:
    > (ctx->msg->data_type == OID_msIndirectData &&
    > !test_bit(sinfo_has_ms_opus_info, &sinfo->aa_set))
    >
    > I signed the kernel with redhat test key like below:
    > pesign -c 'Red Hat Test Certificate' -i arch/x86/boot/bzImage -o /boot/vmlinuz-4.4.0-rc8+ -s --force

    And right he is! The Authenticode specification is a paragon amongst
    technical documents, and has this pearl of wisdom to offer:

    ---------------------------------
    Authenticode-Specific SignerInfo UnauthenticatedAttributes Structures

    The following Authenticode-specific data structures are present in
    SignerInfo authenticated attributes.

    SpcSpOpusInfo
    SpcSpOpusInfo is identified by SPC_SP_OPUS_INFO_OBJID
    (1.3.6.1.4.1.311.2.1.12) and is defined as follows:
    SpcSpOpusInfo ::= SEQUENCE {
    programName [0] EXPLICIT SpcString OPTIONAL,
    moreInfo [1] EXPLICIT SpcLink OPTIONAL,
    } --#public--

    SpcSpOpusInfo has two fields:
    programName
    This field contains the program description:
    If publisher chooses not to specify a description, the SpcString
    structure contains a zero-length program name.
    If the publisher chooses to specify a
    description, the SpcString structure contains a Unicode string.
    moreInfo
    This field is set to an SPCLink structure that contains a URL for
    a Web site with more information about the signer. The URL is an
    ASCII string.
    ---------------------------------

    Which is to say that this is an optional *unauthenticated* field which
    may be present in the Authenticated Attribute list. This is not how
    pkcs7 is supposed to work, so when David implemented this, he didn't
    appreciate the subtlety the original spec author was working with, and
    missed the part of the sublime prose that says this Authenticated
    Attribute is an Unauthenticated Attribute. As a result, the code in
    question simply takes as given that the Authenticated Attributes should
    be authenticated.

    But this one should not, individually. Because it says it's not
    authenticated.

    It still has to hash right so the TBS digest is correct. So it is both
    authenticated and unauthenticated, all at once. Truly, a wonder of
    technical accomplishment.

    Additionally, pesign's implementation has always attempted to be
    compatible with the signatures emitted from contemporary versions of
    Microsoft's signtool.exe. During the initial implementation, Microsoft
    signatures always produced the same values for SpcSpOpusInfo -
    {U"Microsoft Windows", "http://www.microsoft.com"} - without regard to
    who the signer was.

    Sometime between Windows 8 and Windows 8.1 they stopped including the
    field in their signatures altogether, and as such pesign stopped
    producing them in commits c0c4da6 and d79cb0c, sometime around June of
    2012. The theory here is that anything that breaks with
    pesign signatures would also be breaking with signtool.exe sigs as well,
    and that'll be a more noticed problem for firmwares parsing it, so it'll
    get fixed. The fact that we've done exactly this bug in Linux code is
    first class, grade A irony.

    So anyway, we should not be checking this field for presence or any
    particular value: if the field exists, it should be at the right place,
    but aside from that, as long as the hash matches the field is good.

    Signed-off-by: Peter Jones <pjones@redhat.com>
    Tested-by: Dave Young <dyoung@redhat.com>
    Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
    Signed-off-by: Juerg Haefliger <juerg.haefliger@hpe.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    crypto/asymmetric_keys/pkcs7_parser.c | 4 +---
    1 file changed, 1 insertion(+), 3 deletions(-)

    --- a/crypto/asymmetric_keys/pkcs7_parser.c
    +++ b/crypto/asymmetric_keys/pkcs7_parser.c
    @@ -547,9 +547,7 @@ int pkcs7_sig_note_set_of_authattrs(void
    struct pkcs7_signed_info *sinfo = ctx->sinfo;

    if (!test_bit(sinfo_has_content_type, &sinfo->aa_set) ||
    - !test_bit(sinfo_has_message_digest, &sinfo->aa_set) ||
    - (ctx->msg->data_type == OID_msIndirectData &&
    - !test_bit(sinfo_has_ms_opus_info, &sinfo->aa_set))) {
    + !test_bit(sinfo_has_message_digest, &sinfo->aa_set)) {
    pr_warn("Missing required AuthAttr\n");
    return -EBADMSG;
    }

    \
     
     \ /
      Last update: 2016-10-26 14:50    [W:4.872 / U:1.260 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site