]> git.ipfire.org Git - thirdparty/ipxe.git/commitdiff
[crypto] Remove userptr_t from ASN.1 parsers
authorMichael Brown <mcb30@ipxe.org>
Mon, 21 Apr 2025 21:40:59 +0000 (22:40 +0100)
committerMichael Brown <mcb30@ipxe.org>
Mon, 21 Apr 2025 22:30:13 +0000 (23:30 +0100)
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>
src/crypto/asn1.c
src/drivers/net/iphone.c
src/image/der.c
src/image/efi_siglist.c
src/image/pem.c
src/include/ipxe/asn1.h
src/include/ipxe/der.h
src/include/ipxe/efi/efi_siglist.h
src/include/ipxe/pem.h
src/interface/efi/efi_cacert.c

index aa57c6a8bd8e68e58ca52b46e2ebea48a3e67ce2..7b84f6fc37f4a978eb47ce5baa68779dc6263f17 100644 (file)
@@ -88,7 +88,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
  *
  * @v cursor           ASN.1 object cursor
  * @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
  *
  * 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
  * invalidated.
  */
-static int asn1_start ( struct asn1_cursor *cursor, unsigned int type,
-                       size_t extra ) {
+static int asn1_start ( struct asn1_cursor *cursor, unsigned int type ) {
        unsigned int len_len;
        unsigned int len;
 
@@ -145,9 +143,9 @@ static int asn1_start ( struct asn1_cursor *cursor, unsigned int type,
                cursor->data++;
                cursor->len--;
        }
-       if ( ( cursor->len + extra ) < len ) {
+       if ( cursor->len < len ) {
                DBGC ( cursor, "ASN1 %p bad length %d (max %zd)\n",
-                      cursor, len, ( cursor->len + extra ) );
+                      cursor, len, cursor->len );
                asn1_invalidate_cursor ( cursor );
                return -EINVAL_ASN1_LEN;
        }
@@ -156,58 +154,36 @@ static int asn1_start ( struct asn1_cursor *cursor, unsigned int type,
 }
 
 /**
- * Enter ASN.1 partial object
+ * Enter ASN.1 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.
+ * The object cursor 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 asn1_enter ( struct asn1_cursor *cursor, unsigned int type ) {
        int len;
 
        /* Parse current object */
-       len = asn1_start ( cursor, type, *extra );
+       len = asn1_start ( cursor, type );
        if ( len < 0 ) {
                asn1_invalidate_cursor ( cursor );
                return len;
        }
 
-       /* Update cursor and additional length */
+       /* Update cursor */
        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
- *
- * @v cursor           ASN.1 object cursor
- * @v type             Expected type, or ASN1_ANY
- * @ret rc             Return status code
- *
- * The object cursor 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 ( struct asn1_cursor *cursor, unsigned int type ) {
-       static size_t no_extra = 0;
-
-       return asn1_enter_partial ( cursor, type, &no_extra );
-}
-
 /**
  * Skip ASN.1 object if present
  *
@@ -226,7 +202,7 @@ int asn1_skip_if_exists ( struct asn1_cursor *cursor, unsigned int type ) {
        int len;
 
        /* Parse current object */
-       len = asn1_start ( cursor, type, 0 );
+       len = asn1_start ( cursor, type );
        if ( len < 0 )
                return len;
 
@@ -281,7 +257,7 @@ int asn1_shrink ( struct asn1_cursor *cursor, unsigned int type ) {
 
        /* Find end of object */
        memcpy ( &temp, cursor, sizeof ( temp ) );
-       len = asn1_start ( &temp, type, 0 );
+       len = asn1_start ( &temp, type );
        if ( len < 0 ) {
                asn1_invalidate_cursor ( cursor );
                return len;
index 08459a6e239e2807f250125283c6c505b626f8ca..bcc9949fe48af6b021ffe0761ae20e0a96796f67 100644 (file)
@@ -1476,7 +1476,7 @@ static int ipair_rx_pubkey ( struct ipair *ipair, char *msg ) {
        }
 
        /* 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 ) {
                rc = next;
                DBGC ( ipair, "IPAIR %p invalid inner public key:\n%s\n",
index ac499233656b16f7d557c665e7ffa019262d3cf3..600e163c9d0c085ec4df64ed4dcbc8331e98c9f4 100644 (file)
@@ -28,7 +28,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 #include <assert.h>
 #include <ipxe/asn1.h>
 #include <ipxe/der.h>
-#include <ipxe/uaccess.h>
 #include <ipxe/image.h>
 
 /** @file
@@ -49,7 +48,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
  * The caller is responsible for eventually calling free() on the
  * 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 ) {
        size_t remaining;
        void *raw;
@@ -67,7 +66,7 @@ int der_asn1 ( userptr_t data, size_t len, size_t offset,
        /* Populate cursor and data buffer */
        (*cursor)->data = raw;
        (*cursor)->len = remaining;
-       copy_from_user ( raw, data, offset, remaining );
+       memcpy ( raw, ( data + offset ), remaining );
 
        /* Shrink 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 ) {
        struct asn1_cursor cursor;
-       uint8_t buf[8];
-       size_t extra;
        int rc;
 
-       /* Sanity check: no realistic DER image can be smaller than this */
-       if ( image->len < sizeof ( buf ) )
-               return -ENOEXEC;
-
-       /* Prepare partial cursor */
-       cursor.data = buf;
-       cursor.len = sizeof ( buf );
-       copy_from_user ( buf, image->data, 0, sizeof ( buf ) );
-       extra = ( image->len - sizeof ( buf ) );
+       /* Prepare cursor */
+       cursor.data = image->data;
+       cursor.len = image->len;
 
        /* Check that image begins with an ASN.1 sequence object */
-       if ( ( rc = asn1_enter_partial ( &cursor, ASN1_SEQUENCE,
-                                        &extra ) ) != 0 ) {
+       if ( ( rc = asn1_skip ( &cursor, ASN1_SEQUENCE ) ) != 0 ) {
                DBGC ( image, "DER %s is not valid ASN.1: %s\n",
                       image->name, strerror ( rc ) );
                return rc;
        }
 
        /* 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 );
                return -ENOEXEC;
        }
index 56c8493d61052ee6322ecb1dd80c50c351623a52..2bd273dbdebeb93321550eaf5d58334963cbf9e9 100644 (file)
@@ -49,8 +49,9 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
  * @v dhdr             Signature data header to fill in
  * @ret rc             Return status code
  */
-static int efisig_find ( userptr_t data, size_t len, size_t *start,
-                        EFI_SIGNATURE_LIST *lhdr, EFI_SIGNATURE_DATA *dhdr ) {
+static int efisig_find ( const void *data, size_t len, size_t *start,
+                        const EFI_SIGNATURE_LIST **lhdr,
+                        const EFI_SIGNATURE_DATA **dhdr ) {
        size_t offset;
        size_t remaining;
        size_t skip;
@@ -63,38 +64,38 @@ static int efisig_find ( userptr_t data, size_t len, size_t *start,
                /* Read list header */
                assert ( offset <= len );
                remaining = ( len - offset );
-               if ( remaining < sizeof ( *lhdr ) ) {
+               if ( remaining < sizeof ( **lhdr ) ) {
                        DBGC ( data, "EFISIG [%#zx,%#zx) truncated header "
                               "at +%#zx\n", *start, len, offset );
                        return -EINVAL;
                }
-               copy_from_user ( lhdr, data, offset, sizeof ( *lhdr ) );
+               *lhdr = ( data + offset );
 
                /* Get length of this signature list */
-               if ( remaining < lhdr->SignatureListSize ) {
+               if ( remaining < (*lhdr)->SignatureListSize ) {
                        DBGC ( data, "EFISIG [%#zx,%#zx) truncated list at "
                               "+%#zx\n", *start, len, offset );
                        return -EINVAL;
                }
-               remaining = lhdr->SignatureListSize;
+               remaining = (*lhdr)->SignatureListSize;
 
                /* Get length of each signature in list */
-               dlen = lhdr->SignatureSize;
-               if ( dlen < sizeof ( *dhdr ) ) {
+               dlen = (*lhdr)->SignatureSize;
+               if ( dlen < sizeof ( **dhdr ) ) {
                        DBGC ( data, "EFISIG [%#zx,%#zx) underlength "
                               "signatures at +%#zx\n", *start, len, offset );
                        return -EINVAL;
                }
 
                /* Strip list header (including variable portion) */
-               if ( ( remaining < sizeof ( *lhdr ) ) ||
-                    ( ( remaining - sizeof ( *lhdr ) ) <
-                      lhdr->SignatureHeaderSize ) ) {
+               if ( ( remaining < sizeof ( **lhdr ) ) ||
+                    ( ( remaining - sizeof ( **lhdr ) ) <
+                      (*lhdr)->SignatureHeaderSize ) ) {
                        DBGC ( data, "EFISIG [%#zx,%#zx) malformed header at "
                               "+%#zx\n", *start, len, offset );
                        return -EINVAL;
                }
-               skip = ( sizeof ( *lhdr ) + lhdr->SignatureHeaderSize );
+               skip = ( sizeof ( **lhdr ) + (*lhdr)->SignatureHeaderSize );
                offset += skip;
                remaining -= skip;
 
@@ -113,12 +114,12 @@ static int efisig_find ( userptr_t data, size_t len, size_t *start,
                                continue;
 
                        /* Read data header */
-                       copy_from_user ( dhdr, data, offset, sizeof ( *dhdr ));
+                       *dhdr = ( data + offset );
                        DBGC2 ( data, "EFISIG [%#zx,%#zx) %s ",
                                offset, ( offset + dlen ),
-                               efi_guid_ntoa ( &lhdr->SignatureType ) );
+                               efi_guid_ntoa ( &(*lhdr)->SignatureType ) );
                        DBGC2 ( data, "owner %s\n",
-                               efi_guid_ntoa ( &dhdr->SignatureOwner ) );
+                               efi_guid_ntoa ( &(*dhdr)->SignatureOwner ) );
                        *start = offset;
                        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
  * 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 ) {
-       EFI_SIGNATURE_LIST lhdr;
-       EFI_SIGNATURE_DATA dhdr;
-       int ( * asn1 ) ( userptr_t data, size_t len, size_t offset,
+       const EFI_SIGNATURE_LIST *lhdr;
+       const EFI_SIGNATURE_DATA *dhdr;
+       int ( * asn1 ) ( const void *data, size_t len, size_t offset,
                         struct asn1_cursor **cursor );
-       size_t skip = offsetof ( typeof ( dhdr ), SignatureData );
+       size_t skip = offsetof ( typeof ( *dhdr ), SignatureData );
        int next;
        int rc;
 
        /* Locate signature list entry */
        if ( ( rc = efisig_find ( data, len, &offset, &lhdr, &dhdr ) ) != 0 )
                goto err_entry;
-       len = ( offset + lhdr.SignatureSize );
+       len = ( offset + lhdr->SignatureSize );
 
        /* 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 );
        DBGC2 ( data, "EFISIG [%#zx,%#zx) extracting %s\n", offset, len,
                ( ( 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
  */
 static int efisig_image_probe ( struct image *image ) {
-       EFI_SIGNATURE_LIST lhdr;
-       EFI_SIGNATURE_DATA dhdr;
+       const EFI_SIGNATURE_LIST *lhdr;
+       const EFI_SIGNATURE_DATA *dhdr;
        size_t offset = 0;
        unsigned int count = 0;
        int rc;
@@ -205,7 +206,7 @@ static int efisig_image_probe ( struct image *image ) {
                }
 
                /* Skip this entry */
-               offset += lhdr.SignatureSize;
+               offset += lhdr->SignatureSize;
                count++;
 
                /* Check if we have reached end of the image */
index 2dcc36442355dfadeadb7faa24d4870977a815c8..caff822ad078ed6cf93fe913b109e8ef45aa1546 100644 (file)
@@ -28,7 +28,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 #include <assert.h>
 #include <ipxe/asn1.h>
 #include <ipxe/base64.h>
-#include <ipxe/uaccess.h>
 #include <ipxe/image.h>
 #include <ipxe/pem.h>
 
@@ -46,14 +45,14 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
  * @v offset           Starting offset
  * @ret next           Offset to next line
  */
-static size_t pem_next ( userptr_t data, size_t len, size_t offset ) {
-       off_t eol;
+static size_t pem_next ( const void *data, size_t len, size_t offset ) {
+       const void *sep;
 
        /* Find and skip next newline character, if any */
-       eol = memchr_user ( data, offset, '\n', ( len - offset ) );
-       if ( eol < 0 )
+       sep = memchr ( ( data + offset ), '\n', ( len - offset ) );
+       if ( ! sep )
                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
  * @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 ) {
-       char buf[ strlen ( marker ) ];
+       size_t marker_len = strlen ( marker );
 
        /* Sanity check */
        assert ( offset <= len );
@@ -76,10 +75,9 @@ static int pem_marker ( userptr_t data, size_t len, size_t offset,
        while ( offset < len ) {
 
                /* Check for marker */
-               if ( ( len - offset ) < sizeof ( buf ) )
+               if ( ( len - offset ) < marker_len )
                        break;
-               copy_from_user ( buf, data, offset, sizeof ( buf ) );
-               if ( memcmp ( buf, marker, sizeof ( buf ) ) == 0 )
+               if ( memcmp ( ( data + offset ), marker, marker_len ) == 0 )
                        return offset;
 
                /* 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
  * 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 ) {
        size_t encoded_len;
        size_t decoded_max_len;
@@ -140,7 +138,7 @@ int pem_asn1 ( userptr_t data, size_t len, size_t offset,
                rc = -ENOMEM;
                goto err_alloc_encoded;
        }
-       copy_from_user ( encoded, data, begin, encoded_len );
+       memcpy ( encoded, ( data + begin ), encoded_len );
        encoded[encoded_len] = '\0';
 
        /* Allocate cursor and data buffer */
index 8a7461cd36d974bd873cbd925808772841769080..9528c40f59a3b07c82de290114cb16305f50a91c 100644 (file)
@@ -481,8 +481,6 @@ asn1_built ( struct asn1_builder *builder ) {
        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_skip_if_exists ( struct asn1_cursor *cursor,
                                 unsigned int type );
index 983aeb23ca97d1f6bb4865a73590389a86205076..512bc0853fa56303a63b2f148266220542f46314 100644 (file)
@@ -13,7 +13,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 #include <ipxe/asn1.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 );
 
 extern struct image_type der_image_type __image_type ( PROBE_NORMAL );
index 177f28b00dd1f0409499b9d831658fcde3c8bb47..cbc835dc03f4e60fa23e50dbc18ba939a9d7349c 100644 (file)
 FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 
 #include <stdint.h>
-#include <ipxe/uaccess.h>
 #include <ipxe/asn1.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 );
 
 extern struct image_type efisig_image_type __image_type ( PROBE_NORMAL );
index d88ec5b6fa2001315c89486300c13a998dae4a77..d9ca017d553802fa9fba600a39f64a54d83ec297 100644 (file)
@@ -10,7 +10,6 @@
 FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 
 #include <stdint.h>
-#include <ipxe/uaccess.h>
 #include <ipxe/asn1.h>
 #include <ipxe/image.h>
 
@@ -20,7 +19,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 /** Post-encapsulation boundary marker */
 #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 );
 
 extern struct image_type pem_image_type __image_type ( PROBE_NORMAL );
index 5cc268b0e374548d181d733e18a57bdbf3b755f8..2b6c5c343d0b483944ae07c45fefab85f16002c8 100644 (file)
@@ -54,14 +54,14 @@ static struct x509_chain efi_cacerts = {
  * @v offset           Offset within data
  * @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 x509_certificate *cert;
        int next;
        int rc;
 
        /* Extract ASN.1 object */
-        next = efisig_asn1 ( virt_to_user ( data ), len, offset, &cursor );
+       next = efisig_asn1 ( data, len, offset, &cursor );
        if ( next < 0 ) {
                rc = next;
                DBGC ( &efi_cacerts, "EFICA could not parse at +%#zx: %s\n",