-
Notifications
You must be signed in to change notification settings - Fork 114
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
Tighten up string type representations to prevent illegal values #181
Comments
This branch adds basic support emitting and parsing distinguished name values that are Ia5Strings. For example, email address attributes in a certificate subject distinguished name. Note that because of #181 this code will panic when emitting invalid Ia5String values. This problem is general to rcgen's handling of ASN.1 string types and so isn't addressed with additional care in this branch. A broader rework is required. Along the way I also fixed a warning from #176 related to where we were defining the custom `profile.dev.package.num-bigint-dig` profile metadata. Resolves #180
Does using newtype would make the job? Something in the idea of the nutype crate but without using derive macro |
Yeah, we could probably do newtypes for each of the inner variants (that need additional constraints), and have |
That sounds like the right approach to me 👍 |
The created types will look like a lot to the one from the der::asn1 module of the |
I don't think we should depend on the der crate for that alone. It has a huge list of dependencies of its own. |
Actually, it has no dependency for our use case. All the dependencies listed in their |
These types will end up in the public API of rcgen and so I think it would be preferential to not tie them to a third party crate. |
Fix #181 Mainly inspired by the [der](https://docs.rs/der/0.7.8/der/asn1/index.html) and the [asn1_rs](https://github.com/rusticata/asn1-rs/tree/d05f860d07998b4e7f152005ec9a923bb284a708) crates.
In several places Rcgen defines representations of ASN.1 string types, but in a way that doesn't enforce the values meet the restrictions imposed by the type in use.
For example, consider the
DnValue
enum withTeletexString
,PrintableString
,UniversalString
,Utf8String
andBmpString
variants. Those variants express their values as eitherString
orVec<u8>
, but in many cases the ASN.1 definitions for these string types introduce further restrictions. As one example, aPrintableString
can only containA-Z, a-z, 0-9, '()+,-./:=?
and<SPACE>
where as a RustString
can contain any valid UTF-8.Because of this mismatch invalid values can be expressed in
CertificateParams
, and won't be caught by any validation. In the cases where we use a yasna writer andwrite_bytes
generically, we'll emit an invalid encoding for the type in use. In the cases where we use a more specific yasna helper likewrite_ia5_string
orwrite_printable_string
, invalid values will cause a panic.I think we should look at creating a different representation that can impose more validation in order to reject invalid values at construction time as opposed to emitting invalid encodings or panicing when serializing.
The text was updated successfully, but these errors were encountered: