Skip to content

Commit

Permalink
Refactor features.
Browse files Browse the repository at this point in the history
Test all feature configurations in CI.

Remove the `trust_anchor_utils` feature flag.

Guard all features that directly require allocation with a new `alloc` feature.
The RSA features will be handled separately.

Document the features. Tell docs.rs to document all features.

Adjust some tests so that tests are run in more configurations.
  • Loading branch information
briansmith committed Jan 7, 2021
1 parent 64708f1 commit 2deeb79
Show file tree
Hide file tree
Showing 10 changed files with 51 additions and 27 deletions.
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ jobs:
matrix:
features:
- # Default
- --features=alloc
- --all-features

target:
Expand Down
7 changes: 5 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,16 @@ include = [
"third-party/chromium/**/*",
]

[package.metadata.docs.rs]
all-features = true

[lib]
name = "webpki"
path = "src/webpki.rs"

[features]
trust_anchor_util = ["std"]
std = []
alloc = []
std = ["alloc"]

[dependencies]
ring = { version = "0.16.19", default-features = false, features = ["alloc"] }
Expand Down
1 change: 1 addition & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,5 +93,6 @@ impl fmt::Display for Error {
}
}

/// Requires the `std` feature.
#[cfg(feature = "std")]
impl ::std::error::Error for Error {}
3 changes: 2 additions & 1 deletion src/name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
mod dns_name;
pub use dns_name::{DnsNameRef, InvalidDnsNameError};

#[cfg(feature = "std")]
/// Requires the `alloc` feature.
#[cfg(feature = "alloc")]
pub use dns_name::DnsName;

mod ip_address;
Expand Down
32 changes: 21 additions & 11 deletions src/name/dns_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
// ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
// OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

#[cfg(feature = "alloc")]
use alloc::string::String;

/// A DNS Name suitable for use in the TLS Server Name Indication (SNI)
/// extension and/or for use as the reference hostname for which to verify a
/// certificate.
Expand All @@ -28,27 +31,32 @@
/// depend on the specific circumstances in which the comparison is done.
///
/// [RFC 5280 Section 7.2]: https://tools.ietf.org/html/rfc5280#section-7.2
#[cfg(feature = "std")]
///
/// Requires the `alloc` feature.
#[cfg(feature = "alloc")]
#[derive(Clone, Debug, Eq, PartialEq, Hash)]
pub struct DnsName(String);

#[cfg(feature = "std")]
/// Requires the `alloc` feature.
#[cfg(feature = "alloc")]
impl DnsName {
/// Returns a `DnsNameRef` that refers to this `DnsName`.
pub fn as_ref(&self) -> DnsNameRef {
DnsNameRef(self.0.as_bytes())
}
}

#[cfg(feature = "std")]
/// Requires the `alloc` feature.
#[cfg(feature = "alloc")]
impl AsRef<str> for DnsName {
fn as_ref(&self) -> &str {
self.0.as_ref()
}
}

/// Requires the `alloc` feature.
// Deprecated
#[cfg(feature = "std")]
#[cfg(feature = "alloc")]
impl From<DnsNameRef<'_>> for DnsName {
fn from(dns_name: DnsNameRef) -> Self {
dns_name.to_owned()
Expand Down Expand Up @@ -89,6 +97,7 @@ impl core::fmt::Display for InvalidDnsNameError {
}
}

/// Requires the `std` feature.
#[cfg(feature = "std")]
impl ::std::error::Error for InvalidDnsNameError {}

Expand All @@ -110,7 +119,9 @@ impl<'a> DnsNameRef<'a> {
}

/// Constructs a `DnsName` from this `DnsNameRef`
#[cfg(feature = "std")]
///
/// Requires the `alloc` feature.
#[cfg(feature = "alloc")]
pub fn to_owned(&self) -> DnsName {
// DnsNameRef is already guaranteed to be valid ASCII, which is a
// subset of UTF-8.
Expand All @@ -119,7 +130,8 @@ impl<'a> DnsNameRef<'a> {
}
}

#[cfg(feature = "std")]
/// Requires the `alloc` feature.
#[cfg(feature = "alloc")]
impl core::fmt::Debug for DnsNameRef<'_> {
fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> {
let lowercase = self.clone().to_owned();
Expand Down Expand Up @@ -768,18 +780,16 @@ mod tests {
#[test]
fn presented_matches_reference_test() {
for &(presented, reference, expected_result) in PRESENTED_MATCHES_REFERENCE {
use std::string::String;

let actual_result = presented_id_matches_reference_id(
untrusted::Input::from(presented),
untrusted::Input::from(reference),
);
assert_eq!(
actual_result,
expected_result,
"presented_dns_id_matches_reference_dns_id(\"{}\", IDRole::ReferenceID, \"{}\")",
String::from_utf8_lossy(presented),
String::from_utf8_lossy(reference)
"presented_dns_id_matches_reference_dns_id(\"{:?}\", IDRole::ReferenceID, \"{:?}\")",
presented,
reference
);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/signed_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,8 @@ const ED_25519: AlgorithmIdentifier = AlgorithmIdentifier {
#[cfg(test)]
mod tests {
use crate::{der, signed_data, Error};

use std::{self, io::BufRead, string::String, vec::Vec};
use alloc::{string::String, vec::Vec};
use std::io::BufRead as _;

// TODO: The expected results need to be modified for SHA-1 deprecation.

Expand Down
5 changes: 2 additions & 3 deletions src/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@

//! Conversions into the library's time type.
#[cfg(feature = "std")]
use {ring, std};

/// The time type.
///
/// Internally this is merely a UNIX timestamp: a count of non-leap
Expand Down Expand Up @@ -45,6 +42,8 @@ impl Time {
/// # Ok(())
/// # }
/// ```
///
/// Requires the `std` feature.
#[cfg(feature = "std")]
pub fn try_from(time: std::time::SystemTime) -> Result<Time, ring::error::Unspecified> {
time.duration_since(std::time::UNIX_EPOCH)
Expand Down
6 changes: 6 additions & 0 deletions src/trust_anchor_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@

//! Utilities for efficiently embedding trust anchors in programs.
#[cfg(feature = "alloc")]
use alloc::string::String;

use super::der;
use crate::{
cert::{certificate_serial_number, parse_cert_internal, Cert, EndEntityOrCA},
Expand Down Expand Up @@ -60,6 +63,9 @@ fn possibly_invalid_certificate_serial_number(input: &mut untrusted::Reader) ->
/// Generates code for hard-coding the given trust anchors into a program. This
/// is designed to be used in a build script. `name` is the name of the public
/// static variable that will contain the TrustAnchor array.
///
/// Requires the `alloc` feature.
#[cfg(feature = "alloc")]
pub fn generate_code_for_trust_anchors(name: &str, trust_anchors: &[TrustAnchor]) -> String {
let decl = format!(
"static {}: [TrustAnchor<'static>; {}] = ",
Expand Down
13 changes: 11 additions & 2 deletions src/webpki.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,23 @@
//!
//! See `EndEntityCert`'s documentation for a description of the certificate
//! processing steps necessary for a TLS connection.
//!
//! # Features
//!
//! | Feature | Description |
//! | ------- | ----------- |
//! | `alloc` | Enable features that require use of the heap. |
//! | `std` | Enable features that require libstd. Implies `alloc`. |
#![doc(html_root_url = "https://briansmith.org/rustdoc/")]
#![cfg_attr(not(feature = "std"), no_std)]
#![allow(clippy::single_match)]

#[cfg(all(test, not(feature = "std")))]
#[cfg(any(test, feature = "alloc"))]
#[macro_use]
extern crate alloc;

#[cfg(test)]
extern crate std;

#[macro_use]
Expand All @@ -35,7 +45,6 @@ mod name;
mod signed_data;
mod time;

#[cfg(feature = "trust_anchor_util")]
pub mod trust_anchor_util;

mod verify_cert;
Expand Down
6 changes: 0 additions & 6 deletions tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,8 @@
// ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
// OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

#[cfg(any(feature = "std", feature = "trust_anchor_util"))]
extern crate webpki;

#[cfg(feature = "trust_anchor_util")]
static ALL_SIGALGS: &[&webpki::SignatureAlgorithm] = &[
&webpki::ECDSA_P256_SHA256,
&webpki::ECDSA_P256_SHA384,
Expand All @@ -30,7 +28,6 @@ static ALL_SIGALGS: &[&webpki::SignatureAlgorithm] = &[

/* Checks we can verify netflix's cert chain. This is notable
* because they're rooted at a Verisign v1 root. */
#[cfg(feature = "trust_anchor_util")]
#[test]
pub fn netflix() {
let ee = include_bytes!("netflix/ee.der");
Expand All @@ -50,7 +47,6 @@ pub fn netflix() {
);
}

#[cfg(feature = "trust_anchor_util")]
#[test]
pub fn ed25519() {
let ee = include_bytes!("ed25519/ee.der");
Expand All @@ -69,15 +65,13 @@ pub fn ed25519() {
);
}

#[cfg(feature = "trust_anchor_util")]
#[test]
fn read_root_with_zero_serial() {
let ca = include_bytes!("misc/serial_zero.der");
let _ = webpki::trust_anchor_util::cert_der_as_trust_anchor(ca)
.expect("godaddy cert should parse as anchor");
}

#[cfg(feature = "trust_anchor_util")]
#[test]
fn read_root_with_neg_serial() {
let ca = include_bytes!("misc/serial_neg.der");
Expand Down

0 comments on commit 2deeb79

Please sign in to comment.