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.

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

Migrate more types #9254

Merged
merged 15 commits into from
Jul 26, 2023
Merged

Migrate more types #9254

merged 15 commits into from
Jul 26, 2023

Conversation

woodruffw
Copy link
Contributor

@woodruffw woodruffw commented Jul 17, 2023

This moves additional types from cryptography_x509_validation. In particular:

  1. IA5String is an invariant-preserving String wrapper (the inner string must be ASCII only).
  2. DNSName is an invariant preserving IA5String wrapper (the inner value must look like a domain name, and is normalized as lowercase before being stored).
  3. DNSPattern is an invariant preserving DNSName wrapper (giving us the ability to both exact and RFC 6125-style wildcard matches).

These types roughly mirror some of the Python APIs (service IDs, CertificatePattern) in functionality; my thinking here is that those APIs could be replaced with bindings to these once they're rounded out more (e.g. IP address and other types) if desired.

See #8873.

Cleans up the types a bit; patterns are now their own type,
simplifying our matching logic.

Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
src/rust/cryptography-x509/src/common.rs Outdated Show resolved Hide resolved
Comment on lines 393 to 399
pub fn new(pat: &str) -> Option<Self> {
if let Some(pat) = pat.strip_prefix("*.") {
DNSName::new(pat).map(Self::Wildcard)
} else {
DNSName::new(pat).map(Self::Exact)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

It should probably be an error to have a wildcard in any other position, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, and it is -- DNSName::new(...) will return None if there are any wildcards present (since they aren't valid domain characters).

e.g., in tests below:

        assert_eq!(DNSPattern::new("f*o.example.com"), None);
        assert_eq!(DNSPattern::new("*oo.example.com"), None);
        assert_eq!(DNSPattern::new("fo*.example.com"), None);
        assert_eq!(DNSPattern::new("foo.*.example.com"), None);
        assert_eq!(DNSPattern::new("*.foo.*.example.com"), None);

src/rust/cryptography-x509/src/name.rs Outdated Show resolved Hide resolved
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
@alex
Copy link
Member

alex commented Jul 25, 2023

CI failing here, FYI

Signed-off-by: William Woodruff <[email protected]>
src/rust/cryptography-x509/src/common.rs Outdated Show resolved Hide resolved
src/rust/cryptography-x509/src/common.rs Outdated Show resolved Hide resolved
@alex
Copy link
Member

alex commented Jul 26, 2023

Oh. I think maybe this code should live in the new x509-validation crate?

@woodruffw
Copy link
Contributor Author

Oh. I think maybe this code should live in the new x509-validation crate?

Yeah, I think you're right. I can turn this PR into a breakout of a skeleton for the crate + this changeset. Or I can do the skeleton separately, if you'd like to keep things small.

@woodruffw
Copy link
Contributor Author

aa78be8 adds the cryptography_x509_validation crate skeleton and moves these back to types.rs.

@alex alex merged commit 0071b3e into pyca:main Jul 26, 2023
@woodruffw woodruffw deleted the ww/more-types branch July 26, 2023 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants