Fix for KamuSM root name constraint encoding

[cpu]

Description

The Kamu Sertifikasyon Merkezi (KamuSM) root certificate authority successfully petitioned Mozilla to expand the imposed name constraint placed on the TUBITAK Kamu SM SSL Kok Sertifikasi - Surum 1 root to allow issuing for all *.tr domains.

In #36 we tried to update the corresponding webpki-roots constraint, but arrived at the wrong encoding, making the constraint inoperative with webpki. This branch cleans up an unused name constraint constant and fixes the KamuSM root name constraint encoding.

Resolves #38

codegen: remove unused ANSSI name constraints.

The Agence Nationale de la Securite des Systemes d'Information (ANSSI) root is not present in the generated lib.rs, and so the associated name constraints are not used.

codegen: correct TUBITAK1 name constraints.

The content was misencoded, resulting in a permitted subtree base name that's an otherName type general name with a nonsensical value. These types of general name are not supported by webpki, making the name constraint functionally a no-op.

The updated encoding correctly specifies a permitted subtree with a dNSName type base general name with the value ".tr".

See #38 for a more detailed breakdown of the issue and fix.

Cargo: version 0.25.1 -> 0.25.2.

Increases the package version in preparation for a release.

[djc]

FWIW, I kept this around because it's still there in the upstream source in genname.c, so on the off chance that a certificate with the ANSSI_SUBJECT_DN ever makes it back in, this should apply to it.

Not sure that's a strong argument for keeping it, but it's why I kept it around in #34.

[djc]

Oh, good find -- thanks for fixing this up.

Probably good to add a version bump here, too?

Have you given any thought to better tests that would have caught this?

[cpu]

Probably good to add a version bump here, too?

Good call, I've added a commit for that.

Have you given any thought to better tests that would have caught this?

A little bit, it's a tricky situation. I think for each name constraint we want a negative test that the constraint correctly rejects a certificate that doesn't meet the name constraint requirements, but we can't source that for real trust anchors with real name constraints if the web PKI is working as intended.

The only two options I've been able to think of are:

1. writing test infrastructure to generate our own trust anchor with a name constraint matching the one under test, but with keypairs under our control, and then having it issue a positive and negative test case we can check with EndEntity::verify_for_usage.
2. exposing more of the webpki name validation API surface so we could instantiate GeneralName instances and call verify.rs's check_presented_id_conforms_to_constraints directly without needing to worry about the other aspects of validation that verify_for_usage requires (valid signatures, etc).

The latter feels unpleasant and counter to the goals of the webpki crate. The former is probably doable with some legwork? WDYT?

[djc]

Hmm, maybe (2) could still work if we move check_name_constraints() to become a method of Cert and rename SubjectCommonNameContents and variants a bit to clarify its purpose, that would be a decent entry point?

But (1) definitely feels like more of an integration test. Bit more work to get going but probably a better situation in the long term.

(And that would allow us to close #7, I think?)

[cpu]

But (1) definitely feels like more of an integration test. Bit more work to get going but probably a better situation in the long term.

I'll experiment with this a little bit (but it might be time consuming enough that we shouldn't block the fix release waiting on it).

[djc]

Yeah, feel free to just get this out -- we already have a tracking issue for the tests anyway!

[ctz]

Oops! Definitely my bad here.

I came up with a manual way to do a negative test: we can't change issued certs or find misissued certs, but we can change the constraints so the issued cert we do have becomes outside the constraint:

--- a/tests/codegen.rs
+++ b/tests/codegen.rs
@@ -201,3 +201,3 @@ const TUBITAK1_SUBJECT_DN: &[&[u8]] = &[
 const TUBITAK1_NAME_CONSTRAINTS: &[u8] =
-    &[0xA0, 0x07, 0x30, 0x05, 0x82, 0x03, 0x2E, 0x74, 0x72];
+    &[0xA0, 0x07, 0x30, 0x05, 0x82, 0x03, 0x2E, 0x6b, 0x72];

(this changes the constraint from turkey to korea)

after that:

running 1 test
test tubitak_name_constraint_works ... FAILED

failures:

---- tubitak_name_constraint_works stdout ----
thread 'tubitak_name_constraint_works' panicked at tests/verify.rs:31:6:
called `Result::unwrap()` on an `Err` value: NameConstraintViolation
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Which demonstrates the constraint now applies.

[djc]

Nice, I'm going to make sure this is published just to make sure we make this fix available for users.

[djc]

Published 0.25.2.

[cpu]

we can't change issued certs or find misissued certs, but we can change the constraints so the issued cert we do have becomes outside the constraint:

Good idea 👍

Nice, I'm going to make sure this is published just to make sure we make this fix available for users.

Thanks! I appreciate it.

[cpu]

But (1) definitely feels like more of an integration test. Bit more work to get going but probably a better situation in the long term.

I'll experiment with this a little bit (but it might be time consuming enough that we shouldn't block the fix release waiting on it).

Here's something like what I was thinking: #40