Sorry for the late review, i was sick during a long period in November. The PR looks good except for the fact that it introduces a dependency. Could you use write_bitvec_bytes from yasna instead?

Hey, no problem. Hope you're feeling better now.

I've removed the bigint dependency in the latest version of the PR.

So, the tests are failing because x509_parser expects the serial number to be tagged as INTEGER, which write_biguint does, but write_bitvec_bytes uses the BITSTRING tag, causing x509_parser::parse_x509_certificate() to fail.

Not sure how to solve this without introducing the bigint dependency. Any suggestions?

Wait, CI is giving an error:

 thread 'tests::test_with_key_usages' panicked at 'called `Result::unwrap()` on an `Err` value: Error(InvalidSerial)', src/lib.rs:2432:70

Also, maybe it would be good to have a test with a long 20 byte serial in the test suite. E.g. you create a cert, then read it back and ensure the serial number still has 20 bytes (and matches the initial array with 20 bytes).

Yes, please see the comment above about the failed tests.

Oh good point, we can't use write_bitvec_bytes. I guess we need a write_biguint_bytes function in yasna then... This is the PR that added it: qnighy/yasna.rs#44

@andrenth can you check if your PR will work with qnighy/yasna.rs#66 applied?

Yeah, it does work after replacing the two calls of write_bitvec_bytes with write_bigint_bytes:

-                               writer.next().write_bitvec_bytes(serial.as_ref(), serial.len() * 8);
+                               writer.next().write_bigint_bytes(serial.as_ref(), true);

@andrenth I have released a version of yasna with that function added. You can increase the requirement in Cargo.toml now and update your PR to use it.

@andrenth friendly ping

Okay, merging anyways, I will fix it once it's merged.