der: Generic custom-class long > 30 tags PRIVATE, CONTEXT-SPECIFIC, APPLICATION #1545

dishmaker · 2024-10-06T20:01:38Z

Continuation of:
PR support private tags and tag numbers >30 that are stored in long form #1416
Issue der: Encoding higher tag number #1381
Issue der: implementing other tag classes than Context-Specific #877

In this PR:

Allow the use of tag numbers >30. In the der representation they are stored in the long form as multiple bytes.
Allow APPLCATION and PRIVATE tags to be annotated like context specific ones in the derive macros
Merge application, private, context-specific tags into one generic codebase CustomClass.
Split EXPLICIT and IMPLICIT encoding into CustomClassExplicit/Implicit
Add generic TAG: u16 to eg. ContextSpecificExplicit wrapper.
Fix the need of implementing both Decode and DecodeValue on CONTEXT-SPECIFIC tags.
- EXPLICIT tags only need to implement Decode<'a>.
- IMPLICIT tags only need to implement DecodeValue<'a> and Tagged.
- todo: maybe drop the requirement of Tagged too - it is needed for checking constructed bit
Add read_nested for IMPLICIT decoder (I had to fix x509-cert tests, negative ones)

In implicit Decode there was no read_nested before:

formats/der/src/asn1/context_specific.rs

Lines 66 to 67 in c501837

let header = Header::decode(reader)?;

let value = T::decode_value(reader, header)?;

Main breaking feature is here:

formats/der/src/asn1/context_specific.rs

Lines 9 to 23 in 756ef5f

    
           /// Context-specific class, EXPLICIT 
        
           pub type ContextSpecificExplicit<const TAG: u16, T> = 
        
               CustomClassExplicit<TAG, T, CLASS_CONTEXT_SPECIFIC>; 
        
           /// Context-specific class, IMPLICIT 
        
           pub type ContextSpecificImplicit<const TAG: u16, T> = 
        
               CustomClassImplicit<TAG, T, CLASS_CONTEXT_SPECIFIC>; 
        
           /// Context-specific class, reference, EXPLICIT 
        
           pub type ContextSpecificExplicitRef<'a, const TAG: u16, T> = 
        
               CustomClassExplicitRef<'a, TAG, T, CLASS_CONTEXT_SPECIFIC>; 
        
           /// Context-specific class, reference, IMPLICIT 
        
           pub type ContextSpecificImplicitRef<'a, const TAG: u16, T> = 
        
               CustomClassImplicitRef<'a, TAG, T, CLASS_CONTEXT_SPECIFIC>;

dishmaker · 2024-10-10T14:22:21Z

Sorry for not providing documentation on ContextSpecificExplicit and AnyCustomClassExplicit etc...
I'll try maybe next week.

brandonweeks · 2024-10-15T06:55:51Z

I've tested this PR with a prototype for parsing the Android key attestation X.509 extension that exclusively has tags >31. No issues discovered.

https://developer.android.com/privacy-and-security/security-key-attestation#key_attestation_ext_schema

dishmaker · 2024-12-02T11:00:55Z

[Spoiler] Wondering about constructed bit...

Currently, CONTEXT-SPECIFIC IMPLICIT forces the user to implement FixedTag (which implies Tagged). But the tag isn't actually used anywhere.

formats/der/src/asn1/custom_class.rs

Line 184 in 116ce15

let constructed = <T as FixedTag>::TAG.is_constructed();

I think it might be useful for some types to implement IsConstructed, but not FixedTag
(for example custom CHOICE would not be FixedTag).

But if FixedTag is available, then it will be auto-implemented.
Something like:

impl<T: FixedTag> IsConstructed for T {
    /// ASN.1 constructed bit, used for CONTEXT-SPECIFIC IMPLICIT encoding
    const CONSTRUCTED: bool = T::TAG.is_constructed();
}

…re stored in long form draft: CustomClass generic type feat: add decode_implicit and decode_explicit back fix: cargo test -p der now pass refactor: raname to Explicit struct refactor: TAG, T order docs: CustomClass feat: add ContextSpecificRef back (ContextSpecificExplicitRef) update der-derive to current der implementation feat: add tests for context-specific fix: dependency crates fixed for x509-cert cargo fmt fix: crates depending on context-specific and long TagNumber fix x509-cert

dishmaker · 2025-01-09T07:51:45Z

PR is ready for review 😃

tarcieri · 2025-01-21T21:15:07Z

@dishmaker as I noted in #1416, I would prefer a much more minimal PR which only changes the handling of the Tag type, and doesn't try to add higher-level APIs beyond that, which are really what's blocking getting this merged.

We can circle back on those types once the support is added in Tag, but putting it all in a single PR makes that PR large and also blocks getting Tag fixed on bikeshedding the design of the higher-level APIs, which could be additively included after a stable release.

dishmaker · 2025-01-23T15:27:53Z

Well, I agree the PR is huge. But these features are needed anyway - here's an independent impl of generic TAG: u8:
https://github.com/worldfnd/icao-9303/blob/main/src/asn1/application_tagged.rs#L8

tarcieri · 2025-01-23T21:18:28Z

@dishmaker I agree the higher-level functionality is eventually needed to make the changes useful, but because this PR is so large, I think it would be helpful for me as a reviewer to split it up into a set of breaking changes to the Tag type, and then a separate, purely additive PR which can add higher-level functionality.

That would allow more focus on both PRs, to ensure the breaking changes are being done correctly, and separately allowing more debate around the higher level API without blocking another release of the der crate, which it would be nice to do soon.

turbocool3r · 2025-02-08T19:44:08Z

Maybe there is a chance tag numbers can be made u32 instead of u16 if these changes are in the plans? This could allow parsing more exotic formats like Image4 files and their signatures used in firmwares for Apple devices.

tarcieri · 2025-02-08T20:18:47Z

@turbocool3r what are the specific requirements there?

turbocool3r · 2025-02-08T20:37:30Z

@tarcieri for this usecase 32-bit tag numbers should be enough.

This can be circumvented with some manual parsing though: https://gitlab.com/turbocooler/image4-rs/-/blob/main/lib/src/property/common.rs?ref_type=heads#L16.

tarcieri · 2025-02-08T20:59:21Z

Okay, so it needs PRIVATE class tags

turbocool3r · 2025-02-08T21:10:40Z

yep

tarcieri · 2025-02-24T09:37:02Z

@dishmaker can re-evaluate something like this after #1651 is merged

dishmaker marked this pull request as draft October 6, 2024 20:02

dishmaker force-pushed the custom_class_long_tags branch from 756ef5f to 41fa980 Compare October 9, 2024 13:53

dishmaker marked this pull request as ready for review October 9, 2024 15:29

dishmaker changed the title ~~WIP: Custom long > 30 tags PRIVATE, CONTEXT-SPECIFIC, APPLICATION~~ Generic custom-class long > 30 tags PRIVATE, CONTEXT-SPECIFIC, APPLICATION Oct 9, 2024

dishmaker changed the title ~~Generic custom-class long > 30 tags PRIVATE, CONTEXT-SPECIFIC, APPLICATION~~ der: Generic custom-class long > 30 tags PRIVATE, CONTEXT-SPECIFIC, APPLICATION Oct 9, 2024

dishmaker force-pushed the custom_class_long_tags branch from 02c666a to fd9b041 Compare October 10, 2024 06:53

Harris Kaufmann and others added 10 commits January 7, 2025 08:23

der: add tests for Application and Private tags

7833423

fix: clippy warnings

19e6db9

split x509-cert tests into functions

5c594a4

fix(x509-cert): test updated to read_nested in IMPLICIT

3d57777

fix(der_derive): tests with new implicit/explicit

0820d7e

der: add test for application tags

b507d5c

cargo fmt

01ea749

docs: add Tagged comments ISO/IEC 8825-1:2021 in CustomClasses

6590e41

docs + cleanup

6d607c0

dishmaker force-pushed the custom_class_long_tags branch from 21e723f to 6d607c0 Compare January 7, 2025 07:25

dishmaker closed this Feb 24, 2025

dishmaker mentioned this pull request Mar 15, 2025

der: add class bits consts for Application and Private tag support #1721

Merged

dishmaker mentioned this pull request Mar 28, 2025

der: drop Encode, Decode, Tagged traits requirements for IMPLICIT #1746

Open

	let header = Header::decode(reader)?;
	let value = T::decode_value(reader, header)?;

	/// Context-specific class, EXPLICIT
	pub type ContextSpecificExplicit<const TAG: u16, T> =
	CustomClassExplicit<TAG, T, CLASS_CONTEXT_SPECIFIC>;

	/// Context-specific class, IMPLICIT
	pub type ContextSpecificImplicit<const TAG: u16, T> =
	CustomClassImplicit<TAG, T, CLASS_CONTEXT_SPECIFIC>;

	/// Context-specific class, reference, EXPLICIT
	pub type ContextSpecificExplicitRef<'a, const TAG: u16, T> =
	CustomClassExplicitRef<'a, TAG, T, CLASS_CONTEXT_SPECIFIC>;

	/// Context-specific class, reference, IMPLICIT
	pub type ContextSpecificImplicitRef<'a, const TAG: u16, T> =
	CustomClassImplicitRef<'a, TAG, T, CLASS_CONTEXT_SPECIFIC>;

der: Generic custom-class long > 30 tags PRIVATE, CONTEXT-SPECIFIC, APPLICATION #1545

der: Generic custom-class long > 30 tags PRIVATE, CONTEXT-SPECIFIC, APPLICATION #1545

Uh oh!

Conversation

dishmaker commented Oct 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dishmaker commented Oct 10, 2024

Uh oh!

brandonweeks commented Oct 15, 2024

Uh oh!

dishmaker commented Dec 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dishmaker commented Jan 9, 2025

Uh oh!

tarcieri commented Jan 21, 2025

Uh oh!

dishmaker commented Jan 23, 2025

Uh oh!

tarcieri commented Jan 23, 2025

Uh oh!

turbocool3r commented Feb 8, 2025

Uh oh!

tarcieri commented Feb 8, 2025

Uh oh!

turbocool3r commented Feb 8, 2025

Uh oh!

tarcieri commented Feb 8, 2025

Uh oh!

turbocool3r commented Feb 8, 2025

Uh oh!

tarcieri commented Feb 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dishmaker commented Oct 6, 2024 •

edited

Loading

dishmaker commented Dec 2, 2024 •

edited

Loading