Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WIP] der: replace typenum with const generics for BigUInt #325

Closed
wants to merge 1 commit into from
Closed

[WIP] der: replace typenum with const generics for BigUInt #325

wants to merge 1 commit into from

Conversation

tarcieri
Copy link
Member

@tarcieri tarcieri commented Mar 4, 2021

BigUInt was previously using typenum to define the size of an integer. This is necessary because it maps to a signed ASN.1 INTEGER and needs to add/remove a leading 0 when the first octet is >0x7f.

This is perhaps the one thing in RustCrypto right now where I feel that min_const_generics stabilization might actually help: it's a trivial usage, doesn't involve generic-array or associated constants of a trait definition, it's just using the type system to be generic around an integer value.

This eliminates typenum as a dependency and with it feature gating big-uint. Presently requires beta until 1.51 ships on March 25th.

@tarcieri tarcieri requested a review from newpavlov March 4, 2021 14:05
tarcieri
tarcieri commented Mar 4, 2021
View reviewed changes
der/src/asn1/big_uint.rs
Comment on lines -162 to -195
// Sizes supported by the current implementation (1 - 512 bytes)
impl_size!(
U1, U2, U3, U4, U5, U6, U7, U8, U9, U10, U11, U12, U13, U14, U15, U16, U17, U18, U19, U20, U21,
U22, U23, U24, U25, U26, U27, U28, U29, U30, U31, U32, U33, U34, U35, U36, U37, U38, U39, U40,
U41, U42, U43, U44, U45, U46, U47, U48, U49, U50, U51, U52, U53, U54, U55, U56, U57, U58, U59,
U60, U61, U62, U63, U64, U65, U66, U67, U68, U69, U70, U71, U72, U73, U74, U75, U76, U77, U78,
U79, U80, U81, U82, U83, U84, U85, U86, U87, U88, U89, U90, U91, U92, U93, U94, U95, U96, U97,
U98, U99, U100, U101, U102, U103, U104, U105, U106, U107, U108, U109, U110, U111, U112, U113,
U114, U115, U116, U117, U118, U119, U120, U121, U122, U123, U124, U125, U126, U127, U128, U129,
U130, U131, U132, U133, U134, U135, U136, U137, U138, U139, U140, U141, U142, U143, U144, U145,
U146, U147, U148, U149, U150, U151, U152, U153, U154, U155, U156, U157, U158, U159, U160, U161,
U162, U163, U164, U165, U166, U167, U168, U169, U170, U171, U172, U173, U174, U175, U176, U177,
U178, U179, U180, U181, U182, U183, U184, U185, U186, U187, U188, U189, U190, U191, U192, U193,
U194, U195, U196, U197, U198, U199, U200, U201, U202, U203, U204, U205, U206, U207, U208, U209,
U210, U211, U212, U213, U214, U215, U216, U217, U218, U219, U220, U221, U222, U223, U224, U225,
U226, U227, U228, U229, U230, U231, U232, U233, U234, U235, U236, U237, U238, U239, U240, U241,
U242, U243, U244, U245, U246, U247, U248, U249, U250, U251, U252, U253, U254, U255, U256, U257,
U258, U259, U260, U261, U262, U263, U264, U265, U266, U267, U268, U269, U270, U271, U272, U273,
U274, U275, U276, U277, U278, U279, U280, U281, U282, U283, U284, U285, U286, U287, U288, U289,
U290, U291, U292, U293, U294, U295, U296, U297, U298, U299, U300, U301, U302, U303, U304, U305,
U306, U307, U308, U309, U310, U311, U312, U313, U314, U315, U316, U317, U318, U319, U320, U321,
U322, U323, U324, U325, U326, U327, U328, U329, U330, U331, U332, U333, U334, U335, U336, U337,
U338, U339, U340, U341, U342, U343, U344, U345, U346, U347, U348, U349, U350, U351, U352, U353,
U354, U355, U356, U357, U358, U359, U360, U361, U362, U363, U364, U365, U366, U367, U368, U369,
U370, U371, U372, U373, U374, U375, U376, U377, U378, U379, U380, U381, U382, U383, U384, U385,
U386, U387, U388, U389, U390, U391, U392, U393, U394, U395, U396, U397, U398, U399, U400, U401,
U402, U403, U404, U405, U406, U407, U408, U409, U410, U411, U412, U413, U414, U415, U416, U417,
U418, U419, U420, U421, U422, U423, U424, U425, U426, U427, U428, U429, U430, U431, U432, U433,
U434, U435, U436, U437, U438, U439, U440, U441, U442, U443, U444, U445, U446, U447, U448, U449,
U450, U451, U452, U453, U454, U455, U456, U457, U458, U459, U460, U461, U462, U463, U464, U465,
U466, U467, U468, U469, U470, U471, U472, U473, U474, U475, U476, U477, U478, U479, U480, U481,
U482, U483, U484, U485, U486, U487, U488, U489, U490, U491, U492, U493, U494, U495, U496, U497,
U498, U499, U500, U501, U502, U503, U504, U505, U506, U507, U508, U509, U510, U511, U512
);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was really a roundabout way of disallowing U0. In hindsight I'm not sure it matters.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typenum has the NonZero marker trait for such use cases. :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh neat!

@tarcieri tarcieri force-pushed the der/const-generic-big-uint branch 4 times, most recently from 647e45f to 7779dae Compare March 4, 2021 14:16
@tarcieri
[WIP] der: replace typenum with const generics for BigUInt
aa40d4e 
BigUInt was previously using `typenum` to define the size of an integer.
This is necessary because it maps to a signed ASN.1 INTEGER and needs to
add/remove a leading 0 when the first octet is >0x7f.

This is perhaps the one thing in RustCrypto right now where I feel that
`min_const_generics` stabilization might actually help: it's a trivial
usage, doesn't involve `generic-array` or associated constants of a
trait definition, it's just using the type system to be generic around
an integer value.

This eliminates `typenum` as a dependency and with it feature gating
`big-uint`. Presently requires `beta` until 1.51 ships on March 25th.
@tarcieri tarcieri force-pushed the der/const-generic-big-uint branch from 7779dae to aa40d4e Compare March 4, 2021 14:21
@newpavlov
Copy link
Member

newpavlov commented Mar 5, 2021 •
edited
Loading

I am not sure it's worth to migrate only some crates while most of other crates will have to depend on typenum for the near future. Plus in future it could be worth for der to depend on the hypothetical bigint crate, which with high probability will use typenum. Nevertheless I will leave the final decision to you.

@tarcieri
Copy link
Member Author

tarcieri commented Mar 5, 2021

I would like to double check that it's possible to use a typenum constant as a const generic parameter, as that's the main use case for this right now (via the ecdsa crate).

If I can't get that to work, there's no point, but if it does work out I think it's worth a shot.

I'm also working on trying to implement ASN.1 SET OF and think that could greatly benefit from const generics as well.

@newpavlov
Copy link
Member

I would like to double check that it's possible to use a typenum constant as a const generic parameter

Hm, since USIZE is an associated constant of the Unsigned trait, you may encounter the same issue which does not allow us to use arrays dependent on a trait associated constant.

@tarcieri
Copy link
Member Author

tarcieri commented Mar 5, 2021

Alas, appears you're right. Guess we'll have to wait until const generics evolve a bit more.

Screen Shot 2021-03-05 at 7 46 23 AM

@tarcieri tarcieri closed this Mar 5, 2021
@tarcieri tarcieri deleted the der/const-generic-big-uint branch March 5, 2021 15:47
@tarcieri tarcieri mentioned this pull request Mar 25, 2021
Open
@tarcieri tarcieri mentioned this pull request Apr 26, 2021
Closed
tarcieri added a commit that referenced this pull request Jun 4, 2021
@tarcieri
der: const generic unsigned integer decoder with u32/u64/u128 support
0438c72 
This is another take on #325, but using const generics as a way to
provide a common implementation for Rust's unsigned integer primitives.

This commit replaces the existing DER decoder/encoder implementations
for integer primitives with a core const generic implementation, and
uses macros to write implementations of `TryFrom<Any>`, `Encodable`, and
the `Tagged` traits for each of the integer primitives.

This commit extends support to `u32`, `u64`, and `u128` using:

    impl_uint_encoding!(u8, u16, u32, u64, u128);
@tarcieri tarcieri mentioned this pull request Jun 4, 2021
Merged
tarcieri added a commit that referenced this pull request Jun 4, 2021
@tarcieri
der: const generic unsigned integer decoder with u32/u64/u128 support
39f33b4 
This is another take on #325, but using const generics as a way to
provide a common implementation for Rust's unsigned integer primitives.

This commit replaces the existing DER decoder/encoder implementations
for integer primitives with a core const generic implementation, and
uses macros to write implementations of `TryFrom<Any>`, `Encodable`, and
the `Tagged` traits for each of the integer primitives.

This commit extends support to `u32`, `u64`, and `u128` using:

    impl_uint_encoding!(u8, u16, u32, u64, u128);
tarcieri added a commit that referenced this pull request Jun 4, 2021
@tarcieri
der: const generic unsigned integer decoder with u32/u64/u128 support
b0a283d 
This is another take on #325, but using const generics as a way to
provide a common implementation for Rust's unsigned integer primitives.

This commit replaces the existing DER decoder/encoder implementations
for integer primitives with a core const generic implementation, and
uses macros to write implementations of `TryFrom<Any>`, `Encodable`, and
the `Tagged` traits for each of the integer primitives.

This commit extends support to `u32`, `u64`, and `u128` using:

    impl_uint_encoding!(u8, u16, u32, u64, u128);
tarcieri added a commit that referenced this pull request Jun 4, 2021
@tarcieri
der: const generic unsigned integer decoder with u32/u64/u128 support (…
9ce3d7a 
…#469)

This is another take on #325, but using const generics as a way to
provide a common implementation for Rust's unsigned integer primitives.

This commit replaces the existing DER decoder/encoder implementations
for integer primitives with a core const generic implementation, and
uses macros to write implementations of `TryFrom<Any>`, `Encodable`, and
the `Tagged` traits for each of the integer primitives.

This commit extends support to `u32`, `u64`, and `u128` using:

    impl_uint_encoding!(u8, u16, u32, u64, u128);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@newpavlov newpavlov Awaiting requested review from newpavlov

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tarcieri @newpavlov