Re: [PATCH v5 10/21] appended signatures: Parse X.509 certificates

[Daniel Kiper]

On Mon, Jul 14, 2025 at 11:05:02PM +0530, Sudhakar Kuppusamy wrote:
> This code allows us to parse:
>
>  - X.509 certificates: at least enough to verify the signatures on the
>    PKCS#7 messages. We expect that the certificates embedded in GRUB will
>    be leaf certificates, not CA certificates. The parser enforces this.
>
>  - X.509 certificates support the Extended Key Usage extension and handle
>    it by verifying that the certificate has a Code Signing usage.
>
> Signed-off-by: Javier Martinez Canillas <javi...@redhat.com> # EKU support
> Reported-by: Michal Suchanek <msucha...@suse.com> # key usage issue
> Signed-off-by: Daniel Axtens <d...@axtens.net>
> Signed-off-by: Sudhakar Kuppusamy <sudha...@linux.ibm.com>
> Reviewed-by: Stefan Berger <stef...@linux.ibm.com>
> Reviewed-by: Avnish Chouhan <avn...@linux.ibm.com>
> Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>

[...]

> +static grub_err_t
> +grub_x509_read_subject_public_key (asn1_node asn, struct x509_certificate 
> *results)
> +{
> +  int result;
> +  grub_err_t err;
> +  const char *algo_name = 
> "tbsCertificate.subjectPublicKeyInfo.algorithm.algorithm";
> +  const char *params_name = 
> "tbsCertificate.subjectPublicKeyInfo.algorithm.parameters";
> +  const char *pk_name = 
> "tbsCertificate.subjectPublicKeyInfo.subjectPublicKey";
> +  char algo_oid[MAX_OID_LEN];
> +  int algo_size = sizeof (algo_oid);
> +  char params_value[2];
> +  int params_size = sizeof (params_value);
> +  grub_uint8_t *key_data = NULL;
> +  int key_size = 0;
> +  unsigned int key_type;
> +
> +  /* Algorithm: see notes for rsaEncryption_oid. */
> +  result = asn1_read_value (asn, algo_name, algo_oid, &algo_size);
> +  if (result != ASN1_SUCCESS)
> +    return grub_error (GRUB_ERR_BAD_FILE_TYPE, "error reading x509 public 
> key algorithm: %s",
> +                       asn1_strerror (result));
> +
> +  if (grub_strncmp (algo_oid, rsaEncryption_oid, sizeof (rsaEncryption_oid)) 
> != 0)
> +    return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
> +                       "unsupported x509 public key algorithm: %s", 
> algo_oid);
> +
> +  /*
> +   * RFC 3279 2.3.1
> +   * The rsaEncryption OID is intended to be used in the algorithm field
> +   * of a value of type AlgorithmIdentifier.  The parameters field MUST
> +   * have ASN.1 type NULL for this algorithm identifier.
> +   */
> +  result = asn1_read_value (asn, params_name, params_value, &params_size);
> +  if (result != ASN1_SUCCESS)
> +    return grub_error (GRUB_ERR_BAD_FILE_TYPE, "error reading x509 public 
> key parameters: %s",
> +                       asn1_strerror (result));
> +
> +  if (params_value[0] != ASN1_TAG_NULL)
> +    return grub_error (GRUB_ERR_BAD_FILE_TYPE,
> +                       "invalid x509 public key parameters: expected NULL.");

Error messages does not end with full stop. I saw it in a few places here.
Please fix it in all patches if needed. If you do that you can retain my RB...

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel