Auto-detect key_identifier_method / add SHA1 support.

[brocaar]

If using from_ca_cert_der/_pem, the key_identifier_method would always be set to Sha256, which is not always true. If using OpenSSL for example SHA1 would be used.

If the provided CA certificate contains a SubjectKeyIdentifier extension, then this change tries to detect the correct method.

As well, this adds support for the SHA1 method as this is (still) being used by for example OpenSSL and cfssl.

Fixes #195.

[cpu]

Thanks for the PR! I will try to give this a proper review in the coming days. In the meantime, I have one comment about the approach. It differs slightly from what I had in mind.

[cpu]

I think rather than trying to detect the key ID method by trying a series of digest algorithms and detecting a match from_ca_cert_der should always pull in the exact subject key ID that was found in the cert and carry it forward. It seems plausible to me based on RFC 5280 that there are SKIs in the wild that don't use any of these KeyIdMethods.

[brocaar]

I was thinking about that too... But on the other hand it is also weird when the certificate file is read into params, and then the key_identifier_method (which is a public field) doesn't match the reality because it defaults to Sha256....

Do I understand your feedback correctly that you had something like this in mind:

	/// Calculates a subject key identifier for the certificate subject's public key.
	/// This key identifier is used in the SubjectKeyIdentifier X.509v3 extension.
	fn key_identifier<K: PublicKeyData>(&self, pub_key: &K) -> Vec<u8> {
+		if let Some(v) = self.key_identifier_digest {
+			return v.clone();
+		}

		// Decide which method from RFC 7093 to use
		let digest_method = match self.key_identifier_method {
			KeyIdMethod::Sha256 => &digest::SHA256,
			KeyIdMethod::Sha384 => &digest::SHA384,
			KeyIdMethod::Sha512 => &digest::SHA512,
			KeyIdMethod::Sha1LegacyOnly => &digest::SHA1_FOR_LEGACY_USE_ONLY,
		};
		let digest = digest::digest(digest_method, pub_key.raw_bytes());
		let truncated_digest = &digest.as_ref()[0..20];
		truncated_digest.to_vec()
	}

Then add an key_identifier_digest field to the params struct with type Option<Vec<u8>>, which gets populated if the extension is detected?

If you think that would be a better approach, then I could create a separate PR using that approach.

[cpu]

But on the other hand it is also weird when the certificate file is read into params, and then the key_identifier_method (which is a public field) doesn't match the reality because it defaults to Sha256....

I haven't given this a ton of thought yet, so be wary that I might be sending you on a goose chase 😅 My rough idea was extending KeyIdMethod with a PreSpecified(Vec<u8>) variant that carries a specific value to use. I think that might be nicer than a separate field like key_identifier_digest since as you point out ideally it should be in sync - we don't want a pre-specified value and then a KeyIdMethod::Sha256 that isn't used and out of correspondence with the value.

WDYT?

[brocaar]

I like it, it is even simpler :) Please see #197 with the suggested approach implemented.

[brocaar]

@cpu I'll close this PR in favor of #197. I agree that is a cleaner solution.