[crypto] Remove userptr_t from ASN.1 parsers

Simplify the ASN.1 code by assuming that all objects are fully
accessible via pointer dereferences.  This allows the concept of
"additional data beyond the end of the cursor" to be removed, and
simplifies parsing of all ASN.1 image formats.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
This commit is contained in:
Michael Brown
2025-04-21 22:40:59 +01:00
parent 04d0b2fdf9
commit 3f8937d2f3
10 changed files with 70 additions and 109 deletions

View File

@@ -88,7 +88,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
* *
* @v cursor ASN.1 object cursor * @v cursor ASN.1 object cursor
* @v type Expected type, or ASN1_ANY * @v type Expected type, or ASN1_ANY
* @v extra Additional length not present within partial cursor
* @ret len Length of object body, or negative error * @ret len Length of object body, or negative error
* *
* The object cursor will be updated to point to the start of the * The object cursor will be updated to point to the start of the
@@ -100,8 +99,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
* modified. If any other error occurs, the object cursor will be * modified. If any other error occurs, the object cursor will be
* invalidated. * invalidated.
*/ */
static int asn1_start ( struct asn1_cursor *cursor, unsigned int type, static int asn1_start ( struct asn1_cursor *cursor, unsigned int type ) {
size_t extra ) {
unsigned int len_len; unsigned int len_len;
unsigned int len; unsigned int len;
@@ -145,9 +143,9 @@ static int asn1_start ( struct asn1_cursor *cursor, unsigned int type,
cursor->data++; cursor->data++;
cursor->len--; cursor->len--;
} }
if ( ( cursor->len + extra ) < len ) { if ( cursor->len < len ) {
DBGC ( cursor, "ASN1 %p bad length %d (max %zd)\n", DBGC ( cursor, "ASN1 %p bad length %d (max %zd)\n",
cursor, len, ( cursor->len + extra ) ); cursor, len, cursor->len );
asn1_invalidate_cursor ( cursor ); asn1_invalidate_cursor ( cursor );
return -EINVAL_ASN1_LEN; return -EINVAL_ASN1_LEN;
} }
@@ -155,41 +153,6 @@ static int asn1_start ( struct asn1_cursor *cursor, unsigned int type,
return len; return len;
} }
/**
* Enter ASN.1 partial object
*
* @v cursor ASN.1 object cursor
* @v type Expected type, or ASN1_ANY
* @v extra Additional length beyond partial object
* @ret rc Return status code
*
* The object cursor and additional length will be updated to point to
* the body of the current ASN.1 object.
*
* If any error occurs, the object cursor will be invalidated.
*/
int asn1_enter_partial ( struct asn1_cursor *cursor, unsigned int type,
size_t *extra ) {
int len;
/* Parse current object */
len = asn1_start ( cursor, type, *extra );
if ( len < 0 ) {
asn1_invalidate_cursor ( cursor );
return len;
}
/* Update cursor and additional length */
if ( ( ( size_t ) len ) <= cursor->len )
cursor->len = len;
assert ( ( len - cursor->len ) <= *extra );
*extra = ( len - cursor->len );
DBGC ( cursor, "ASN1 %p entered object type %02x (len %x)\n",
cursor, type, len );
return 0;
}
/** /**
* Enter ASN.1 object * Enter ASN.1 object
* *
@@ -203,9 +166,22 @@ int asn1_enter_partial ( struct asn1_cursor *cursor, unsigned int type,
* If any error occurs, the object cursor will be invalidated. * If any error occurs, the object cursor will be invalidated.
*/ */
int asn1_enter ( struct asn1_cursor *cursor, unsigned int type ) { int asn1_enter ( struct asn1_cursor *cursor, unsigned int type ) {
static size_t no_extra = 0; int len;
return asn1_enter_partial ( cursor, type, &no_extra ); /* Parse current object */
len = asn1_start ( cursor, type );
if ( len < 0 ) {
asn1_invalidate_cursor ( cursor );
return len;
}
/* Update cursor */
if ( ( ( size_t ) len ) <= cursor->len )
cursor->len = len;
DBGC ( cursor, "ASN1 %p entered object type %02x (len %x)\n",
cursor, type, len );
return 0;
} }
/** /**
@@ -226,7 +202,7 @@ int asn1_skip_if_exists ( struct asn1_cursor *cursor, unsigned int type ) {
int len; int len;
/* Parse current object */ /* Parse current object */
len = asn1_start ( cursor, type, 0 ); len = asn1_start ( cursor, type );
if ( len < 0 ) if ( len < 0 )
return len; return len;
@@ -281,7 +257,7 @@ int asn1_shrink ( struct asn1_cursor *cursor, unsigned int type ) {
/* Find end of object */ /* Find end of object */
memcpy ( &temp, cursor, sizeof ( temp ) ); memcpy ( &temp, cursor, sizeof ( temp ) );
len = asn1_start ( &temp, type, 0 ); len = asn1_start ( &temp, type );
if ( len < 0 ) { if ( len < 0 ) {
asn1_invalidate_cursor ( cursor ); asn1_invalidate_cursor ( cursor );
return len; return len;

View File

@@ -1476,7 +1476,7 @@ static int ipair_rx_pubkey ( struct ipair *ipair, char *msg ) {
} }
/* Decode inner layer of Base64 */ /* Decode inner layer of Base64 */
next = pem_asn1 ( virt_to_user ( decoded ), len, 0, &key ); next = pem_asn1 ( decoded, len, 0, &key );
if ( next < 0 ) { if ( next < 0 ) {
rc = next; rc = next;
DBGC ( ipair, "IPAIR %p invalid inner public key:\n%s\n", DBGC ( ipair, "IPAIR %p invalid inner public key:\n%s\n",

View File

@@ -28,7 +28,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
#include <assert.h> #include <assert.h>
#include <ipxe/asn1.h> #include <ipxe/asn1.h>
#include <ipxe/der.h> #include <ipxe/der.h>
#include <ipxe/uaccess.h>
#include <ipxe/image.h> #include <ipxe/image.h>
/** @file /** @file
@@ -49,7 +48,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
* The caller is responsible for eventually calling free() on the * The caller is responsible for eventually calling free() on the
* allocated ASN.1 cursor. * allocated ASN.1 cursor.
*/ */
int der_asn1 ( userptr_t data, size_t len, size_t offset, int der_asn1 ( const void *data, size_t len, size_t offset,
struct asn1_cursor **cursor ) { struct asn1_cursor **cursor ) {
size_t remaining; size_t remaining;
void *raw; void *raw;
@@ -67,7 +66,7 @@ int der_asn1 ( userptr_t data, size_t len, size_t offset,
/* Populate cursor and data buffer */ /* Populate cursor and data buffer */
(*cursor)->data = raw; (*cursor)->data = raw;
(*cursor)->len = remaining; (*cursor)->len = remaining;
copy_from_user ( raw, data, offset, remaining ); memcpy ( raw, ( data + offset ), remaining );
/* Shrink cursor */ /* Shrink cursor */
asn1_shrink_any ( *cursor ); asn1_shrink_any ( *cursor );
@@ -83,30 +82,21 @@ int der_asn1 ( userptr_t data, size_t len, size_t offset,
*/ */
static int der_image_probe ( struct image *image ) { static int der_image_probe ( struct image *image ) {
struct asn1_cursor cursor; struct asn1_cursor cursor;
uint8_t buf[8];
size_t extra;
int rc; int rc;
/* Sanity check: no realistic DER image can be smaller than this */ /* Prepare cursor */
if ( image->len < sizeof ( buf ) ) cursor.data = image->data;
return -ENOEXEC; cursor.len = image->len;
/* Prepare partial cursor */
cursor.data = buf;
cursor.len = sizeof ( buf );
copy_from_user ( buf, image->data, 0, sizeof ( buf ) );
extra = ( image->len - sizeof ( buf ) );
/* Check that image begins with an ASN.1 sequence object */ /* Check that image begins with an ASN.1 sequence object */
if ( ( rc = asn1_enter_partial ( &cursor, ASN1_SEQUENCE, if ( ( rc = asn1_skip ( &cursor, ASN1_SEQUENCE ) ) != 0 ) {
&extra ) ) != 0 ) {
DBGC ( image, "DER %s is not valid ASN.1: %s\n", DBGC ( image, "DER %s is not valid ASN.1: %s\n",
image->name, strerror ( rc ) ); image->name, strerror ( rc ) );
return rc; return rc;
} }
/* Check that image comprises a single well-formed ASN.1 object */ /* Check that image comprises a single well-formed ASN.1 object */
if ( extra != ( image->len - sizeof ( buf ) ) ) { if ( cursor.len ) {
DBGC ( image, "DER %s is not single ASN.1\n", image->name ); DBGC ( image, "DER %s is not single ASN.1\n", image->name );
return -ENOEXEC; return -ENOEXEC;
} }

View File

@@ -49,8 +49,9 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
* @v dhdr Signature data header to fill in * @v dhdr Signature data header to fill in
* @ret rc Return status code * @ret rc Return status code
*/ */
static int efisig_find ( userptr_t data, size_t len, size_t *start, static int efisig_find ( const void *data, size_t len, size_t *start,
EFI_SIGNATURE_LIST *lhdr, EFI_SIGNATURE_DATA *dhdr ) { const EFI_SIGNATURE_LIST **lhdr,
const EFI_SIGNATURE_DATA **dhdr ) {
size_t offset; size_t offset;
size_t remaining; size_t remaining;
size_t skip; size_t skip;
@@ -63,38 +64,38 @@ static int efisig_find ( userptr_t data, size_t len, size_t *start,
/* Read list header */ /* Read list header */
assert ( offset <= len ); assert ( offset <= len );
remaining = ( len - offset ); remaining = ( len - offset );
if ( remaining < sizeof ( *lhdr ) ) { if ( remaining < sizeof ( **lhdr ) ) {
DBGC ( data, "EFISIG [%#zx,%#zx) truncated header " DBGC ( data, "EFISIG [%#zx,%#zx) truncated header "
"at +%#zx\n", *start, len, offset ); "at +%#zx\n", *start, len, offset );
return -EINVAL; return -EINVAL;
} }
copy_from_user ( lhdr, data, offset, sizeof ( *lhdr ) ); *lhdr = ( data + offset );
/* Get length of this signature list */ /* Get length of this signature list */
if ( remaining < lhdr->SignatureListSize ) { if ( remaining < (*lhdr)->SignatureListSize ) {
DBGC ( data, "EFISIG [%#zx,%#zx) truncated list at " DBGC ( data, "EFISIG [%#zx,%#zx) truncated list at "
"+%#zx\n", *start, len, offset ); "+%#zx\n", *start, len, offset );
return -EINVAL; return -EINVAL;
} }
remaining = lhdr->SignatureListSize; remaining = (*lhdr)->SignatureListSize;
/* Get length of each signature in list */ /* Get length of each signature in list */
dlen = lhdr->SignatureSize; dlen = (*lhdr)->SignatureSize;
if ( dlen < sizeof ( *dhdr ) ) { if ( dlen < sizeof ( **dhdr ) ) {
DBGC ( data, "EFISIG [%#zx,%#zx) underlength " DBGC ( data, "EFISIG [%#zx,%#zx) underlength "
"signatures at +%#zx\n", *start, len, offset ); "signatures at +%#zx\n", *start, len, offset );
return -EINVAL; return -EINVAL;
} }
/* Strip list header (including variable portion) */ /* Strip list header (including variable portion) */
if ( ( remaining < sizeof ( *lhdr ) ) || if ( ( remaining < sizeof ( **lhdr ) ) ||
( ( remaining - sizeof ( *lhdr ) ) < ( ( remaining - sizeof ( **lhdr ) ) <
lhdr->SignatureHeaderSize ) ) { (*lhdr)->SignatureHeaderSize ) ) {
DBGC ( data, "EFISIG [%#zx,%#zx) malformed header at " DBGC ( data, "EFISIG [%#zx,%#zx) malformed header at "
"+%#zx\n", *start, len, offset ); "+%#zx\n", *start, len, offset );
return -EINVAL; return -EINVAL;
} }
skip = ( sizeof ( *lhdr ) + lhdr->SignatureHeaderSize ); skip = ( sizeof ( **lhdr ) + (*lhdr)->SignatureHeaderSize );
offset += skip; offset += skip;
remaining -= skip; remaining -= skip;
@@ -113,12 +114,12 @@ static int efisig_find ( userptr_t data, size_t len, size_t *start,
continue; continue;
/* Read data header */ /* Read data header */
copy_from_user ( dhdr, data, offset, sizeof ( *dhdr )); *dhdr = ( data + offset );
DBGC2 ( data, "EFISIG [%#zx,%#zx) %s ", DBGC2 ( data, "EFISIG [%#zx,%#zx) %s ",
offset, ( offset + dlen ), offset, ( offset + dlen ),
efi_guid_ntoa ( &lhdr->SignatureType ) ); efi_guid_ntoa ( &(*lhdr)->SignatureType ) );
DBGC2 ( data, "owner %s\n", DBGC2 ( data, "owner %s\n",
efi_guid_ntoa ( &dhdr->SignatureOwner ) ); efi_guid_ntoa ( &(*dhdr)->SignatureOwner ) );
*start = offset; *start = offset;
return 0; return 0;
} }
@@ -137,23 +138,23 @@ static int efisig_find ( userptr_t data, size_t len, size_t *start,
* The caller is responsible for eventually calling free() on the * The caller is responsible for eventually calling free() on the
* allocated ASN.1 cursor. * allocated ASN.1 cursor.
*/ */
int efisig_asn1 ( userptr_t data, size_t len, size_t offset, int efisig_asn1 ( const void *data, size_t len, size_t offset,
struct asn1_cursor **cursor ) { struct asn1_cursor **cursor ) {
EFI_SIGNATURE_LIST lhdr; const EFI_SIGNATURE_LIST *lhdr;
EFI_SIGNATURE_DATA dhdr; const EFI_SIGNATURE_DATA *dhdr;
int ( * asn1 ) ( userptr_t data, size_t len, size_t offset, int ( * asn1 ) ( const void *data, size_t len, size_t offset,
struct asn1_cursor **cursor ); struct asn1_cursor **cursor );
size_t skip = offsetof ( typeof ( dhdr ), SignatureData ); size_t skip = offsetof ( typeof ( *dhdr ), SignatureData );
int next; int next;
int rc; int rc;
/* Locate signature list entry */ /* Locate signature list entry */
if ( ( rc = efisig_find ( data, len, &offset, &lhdr, &dhdr ) ) != 0 ) if ( ( rc = efisig_find ( data, len, &offset, &lhdr, &dhdr ) ) != 0 )
goto err_entry; goto err_entry;
len = ( offset + lhdr.SignatureSize ); len = ( offset + lhdr->SignatureSize );
/* Parse as PEM or DER based on first character */ /* Parse as PEM or DER based on first character */
asn1 = ( ( dhdr.SignatureData[0] == ASN1_SEQUENCE ) ? asn1 = ( ( dhdr->SignatureData[0] == ASN1_SEQUENCE ) ?
der_asn1 : pem_asn1 ); der_asn1 : pem_asn1 );
DBGC2 ( data, "EFISIG [%#zx,%#zx) extracting %s\n", offset, len, DBGC2 ( data, "EFISIG [%#zx,%#zx) extracting %s\n", offset, len,
( ( asn1 == der_asn1 ) ? "DER" : "PEM" ) ); ( ( asn1 == der_asn1 ) ? "DER" : "PEM" ) );
@@ -189,8 +190,8 @@ int efisig_asn1 ( userptr_t data, size_t len, size_t offset,
* @ret rc Return status code * @ret rc Return status code
*/ */
static int efisig_image_probe ( struct image *image ) { static int efisig_image_probe ( struct image *image ) {
EFI_SIGNATURE_LIST lhdr; const EFI_SIGNATURE_LIST *lhdr;
EFI_SIGNATURE_DATA dhdr; const EFI_SIGNATURE_DATA *dhdr;
size_t offset = 0; size_t offset = 0;
unsigned int count = 0; unsigned int count = 0;
int rc; int rc;
@@ -205,7 +206,7 @@ static int efisig_image_probe ( struct image *image ) {
} }
/* Skip this entry */ /* Skip this entry */
offset += lhdr.SignatureSize; offset += lhdr->SignatureSize;
count++; count++;
/* Check if we have reached end of the image */ /* Check if we have reached end of the image */

View File

@@ -28,7 +28,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
#include <assert.h> #include <assert.h>
#include <ipxe/asn1.h> #include <ipxe/asn1.h>
#include <ipxe/base64.h> #include <ipxe/base64.h>
#include <ipxe/uaccess.h>
#include <ipxe/image.h> #include <ipxe/image.h>
#include <ipxe/pem.h> #include <ipxe/pem.h>
@@ -46,14 +45,14 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
* @v offset Starting offset * @v offset Starting offset
* @ret next Offset to next line * @ret next Offset to next line
*/ */
static size_t pem_next ( userptr_t data, size_t len, size_t offset ) { static size_t pem_next ( const void *data, size_t len, size_t offset ) {
off_t eol; const void *sep;
/* Find and skip next newline character, if any */ /* Find and skip next newline character, if any */
eol = memchr_user ( data, offset, '\n', ( len - offset ) ); sep = memchr ( ( data + offset ), '\n', ( len - offset ) );
if ( eol < 0 ) if ( ! sep )
return len; return len;
return ( eol + 1 ); return ( ( sep - data ) + 1 );
} }
/** /**
@@ -65,9 +64,9 @@ static size_t pem_next ( userptr_t data, size_t len, size_t offset ) {
* @v marker Boundary marker * @v marker Boundary marker
* @ret offset Offset to boundary marker line, or negative error * @ret offset Offset to boundary marker line, or negative error
*/ */
static int pem_marker ( userptr_t data, size_t len, size_t offset, static int pem_marker ( const void *data, size_t len, size_t offset,
const char *marker ) { const char *marker ) {
char buf[ strlen ( marker ) ]; size_t marker_len = strlen ( marker );
/* Sanity check */ /* Sanity check */
assert ( offset <= len ); assert ( offset <= len );
@@ -76,10 +75,9 @@ static int pem_marker ( userptr_t data, size_t len, size_t offset,
while ( offset < len ) { while ( offset < len ) {
/* Check for marker */ /* Check for marker */
if ( ( len - offset ) < sizeof ( buf ) ) if ( ( len - offset ) < marker_len )
break; break;
copy_from_user ( buf, data, offset, sizeof ( buf ) ); if ( memcmp ( ( data + offset ), marker, marker_len ) == 0 )
if ( memcmp ( buf, marker, sizeof ( buf ) ) == 0 )
return offset; return offset;
/* Move to next line */ /* Move to next line */
@@ -102,7 +100,7 @@ static int pem_marker ( userptr_t data, size_t len, size_t offset,
* The caller is responsible for eventually calling free() on the * The caller is responsible for eventually calling free() on the
* allocated ASN.1 cursor. * allocated ASN.1 cursor.
*/ */
int pem_asn1 ( userptr_t data, size_t len, size_t offset, int pem_asn1 ( const void *data, size_t len, size_t offset,
struct asn1_cursor **cursor ) { struct asn1_cursor **cursor ) {
size_t encoded_len; size_t encoded_len;
size_t decoded_max_len; size_t decoded_max_len;
@@ -140,7 +138,7 @@ int pem_asn1 ( userptr_t data, size_t len, size_t offset,
rc = -ENOMEM; rc = -ENOMEM;
goto err_alloc_encoded; goto err_alloc_encoded;
} }
copy_from_user ( encoded, data, begin, encoded_len ); memcpy ( encoded, ( data + begin ), encoded_len );
encoded[encoded_len] = '\0'; encoded[encoded_len] = '\0';
/* Allocate cursor and data buffer */ /* Allocate cursor and data buffer */

View File

@@ -481,8 +481,6 @@ asn1_built ( struct asn1_builder *builder ) {
return &u->cursor; return &u->cursor;
} }
extern int asn1_enter_partial ( struct asn1_cursor *cursor, unsigned int type,
size_t *extra );
extern int asn1_enter ( struct asn1_cursor *cursor, unsigned int type ); extern int asn1_enter ( struct asn1_cursor *cursor, unsigned int type );
extern int asn1_skip_if_exists ( struct asn1_cursor *cursor, extern int asn1_skip_if_exists ( struct asn1_cursor *cursor,
unsigned int type ); unsigned int type );

View File

@@ -13,7 +13,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
#include <ipxe/asn1.h> #include <ipxe/asn1.h>
#include <ipxe/image.h> #include <ipxe/image.h>
extern int der_asn1 ( userptr_t data, size_t len, size_t offset, extern int der_asn1 ( const void *data, size_t len, size_t offset,
struct asn1_cursor **cursor ); struct asn1_cursor **cursor );
extern struct image_type der_image_type __image_type ( PROBE_NORMAL ); extern struct image_type der_image_type __image_type ( PROBE_NORMAL );

View File

@@ -10,11 +10,10 @@
FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
#include <stdint.h> #include <stdint.h>
#include <ipxe/uaccess.h>
#include <ipxe/asn1.h> #include <ipxe/asn1.h>
#include <ipxe/image.h> #include <ipxe/image.h>
extern int efisig_asn1 ( userptr_t data, size_t len, size_t offset, extern int efisig_asn1 ( const void *data, size_t len, size_t offset,
struct asn1_cursor **cursor ); struct asn1_cursor **cursor );
extern struct image_type efisig_image_type __image_type ( PROBE_NORMAL ); extern struct image_type efisig_image_type __image_type ( PROBE_NORMAL );

View File

@@ -10,7 +10,6 @@
FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
#include <stdint.h> #include <stdint.h>
#include <ipxe/uaccess.h>
#include <ipxe/asn1.h> #include <ipxe/asn1.h>
#include <ipxe/image.h> #include <ipxe/image.h>
@@ -20,7 +19,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
/** Post-encapsulation boundary marker */ /** Post-encapsulation boundary marker */
#define PEM_END "-----END" #define PEM_END "-----END"
extern int pem_asn1 ( userptr_t data, size_t len, size_t offset, extern int pem_asn1 ( const void *data, size_t len, size_t offset,
struct asn1_cursor **cursor ); struct asn1_cursor **cursor );
extern struct image_type pem_image_type __image_type ( PROBE_NORMAL ); extern struct image_type pem_image_type __image_type ( PROBE_NORMAL );

View File

@@ -54,14 +54,14 @@ static struct x509_chain efi_cacerts = {
* @v offset Offset within data * @v offset Offset within data
* @v next Next offset, or negative error * @v next Next offset, or negative error
*/ */
static int efi_cacert ( void *data, size_t len, size_t offset ) { static int efi_cacert ( const void *data, size_t len, size_t offset ) {
struct asn1_cursor *cursor; struct asn1_cursor *cursor;
struct x509_certificate *cert; struct x509_certificate *cert;
int next; int next;
int rc; int rc;
/* Extract ASN.1 object */ /* Extract ASN.1 object */
next = efisig_asn1 ( virt_to_user ( data ), len, offset, &cursor ); next = efisig_asn1 ( data, len, offset, &cursor );
if ( next < 0 ) { if ( next < 0 ) {
rc = next; rc = next;
DBGC ( &efi_cacerts, "EFICA could not parse at +%#zx: %s\n", DBGC ( &efi_cacerts, "EFICA could not parse at +%#zx: %s\n",