Skip to content

Conversation

@ChristopherRabotin
Copy link
Contributor

@ChristopherRabotin ChristopherRabotin commented Jan 19, 2022

Covers sections 8.5 and 11.3 of ITU-T X.690 (02/2021)

Caveats:
1. Interpretation of section 11.3.1 to be further discussed
2. Currently, all encoding is binary (no ISO 6093 NR3)

Testing of arbitrarily large and small f64s shows that DER typically
requires 12 bytes to encode these 8-byte IEEE-754 values. This can be
explained because of the extra two bytes for the tag and length, and the
fact that the exponent is encoded on 1 to 2 bytes (only 11 bits in
IEEE-754), and the mantissa may require up to 8 bytes itself, hence 12
bytes total. Of note that, depending on Caveat 1 above, this information
may be wrong.

Closes #304

Signed-off-by: Christopher Rabotin <[email protected]>
Signed-off-by: Christopher Rabotin <[email protected]>
Signed-off-by: Christopher Rabotin <[email protected]>
Signed-off-by: Christopher Rabotin <[email protected]>
Covers sections 8.5 and 11.3 of ITU-T X.690 (02/2021)

Caveats:
	1. Interpretation of section 11.3.1 to be further discussed
	2. Currently, all encoding is binary (no ISO 6093 NR3)

Testing of arbitrarily large and small f64s shows that DER typically
requires 12 bytes to encode these 8-byte IEEE-754 values. This can be
explained because of the extra two bytes for the tag and length, and the
fact that the exponent is encoded on 1 to 2 bytes (only 11 bits in
IEEE-754), and the mantissa may require up to 8 bytes itself, hence 12
bytes total. Of note that, depending on Caveat 1 above, this information
may be wrong.
Signed-off-by: Christopher Rabotin <[email protected]>
@tarcieri
Copy link
Member

If you can find another implementation to do interop testing with, that'd be great

ChristopherRabotin and others added 2 commits January 30, 2022 22:13
Co-authored-by: Tony Arcieri <[email protected]>
Co-authored-by: Tony Arcieri <[email protected]>
@ChristopherRabotin
Copy link
Contributor Author

Thanks for the review. I'll add a demo encoding from the ASN1 playground

@ChristopherRabotin
Copy link
Contributor Author

As a quick update, I'm working on adding a bunch of validation test cases. It's gonna take some time as it seems like some of my encoding does not match the ASN1 playground so I'm going through the specs again to better understand what I didn't implement correctly. I'll keep the branch up-to-date with master.

ChristopherRabotin and others added 6 commits February 6, 2022 20:53
Signed-off-by: Christopher Rabotin <[email protected]>
These were commented because the ASN1 playground encodes in NR3
(base 10) but this impl only encodes in base 2, so the reciprocity tests
would never have worked. Instead, all other cases were tested for
decoding in the ASN1 playground.

Signed-off-by: Christopher Rabotin <[email protected]>
@ChristopherRabotin
Copy link
Contributor Author

After a two months of working on other stuff, here's the updated PR with validation cases. Let me know if you have any questions.

I'm not sure why clippy is failing in the Github actions, it works locally with clippy 0.1.59 (9d1b210 2022-02-23).

Cheers

Signed-off-by: Christopher Rabotin <[email protected]>
Signed-off-by: Christopher Rabotin <[email protected]>
@ChristopherRabotin
Copy link
Contributor Author

Not sure what this CI build failed as running cargo test --no-default-features works locally and both the val and val2 variables are in fact defined.

@tarcieri
Copy link
Member

It's failing here:

https://github.com/RustCrypto/formats/runs/5669154521?check_suite_focus=true

error: there is no argument named `val`
   --> der/src/asn1/real.rs:827:30
    |
827 |                 "fail - want {val}\tgot {val2}"
    |                              ^^^^^

error: there is no argument named `val2`
   --> der/src/asn1/real.rs:827:[41](https://github.com/RustCrypto/formats/runs/5669154521?check_suite_focus=true#step:7:41)
    |
827 |                 "fail - want {val}\tgot {val2}"
    |

I think the issue is that this is using the new captured identifiers in format strings feature which was introduced in Rust 1.58, and the MSRV of the crate is 1.57, which is what the test is failing on.

Signed-off-by: Christopher Rabotin <[email protected]>
@ChristopherRabotin
Copy link
Contributor Author

Fixed, thanks!

Co-authored-by: Tony Arcieri <[email protected]>
@ChristopherRabotin
Copy link
Contributor Author

Thanks for the suggestion, that fixed all of the tests.

Signed-off-by: Christopher Rabotin <[email protected]>
@tarcieri tarcieri merged commit f13d734 into RustCrypto:master Mar 27, 2022
@tarcieri
Copy link
Member

Thank you!

@tarcieri tarcieri mentioned this pull request May 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

der: support for the REAL type

2 participants