On 01/18/16 at 10:49am, Peter Jones wrote:
> 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@xxxxxxxxxx>
> ---
> crypto/asymmetric_keys/pkcs7_parser.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
> index 758acab..8f3056c 100644
> --- 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 *context, size_t hdrlen,
> 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;
> }
> --
> 2.5.0
>
Tested-by: Dave Young <dyoung@xxxxxxxxxx>
Peter, thanks for the expanation. I was using such fix for several days but
not sure if it is right or not though it works for me.
Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
