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

Increase test coverage of IP address support #14

Closed

Conversation

ereslibre
Copy link
Contributor

@ereslibre ereslibre commented Nov 25, 2022

As discussed in #5, this will increase the code coverage to the full extent of the code additions.

@ereslibre ereslibre mentioned this pull request Nov 25, 2022
3 tasks
@ereslibre ereslibre changed the base branch from feat-ip-address to main November 25, 2022 18:55
@ereslibre ereslibre changed the title WIP: increase test coverage WIP: increase test coverage of IP address support Nov 25, 2022
@ereslibre ereslibre changed the base branch from main to feat-ip-address November 25, 2022 19:16
@codecov-commenter
Copy link

codecov-commenter commented Nov 25, 2022

Codecov Report

Merging #14 (fb34551) into main (dcfe0b4) will increase coverage by 6.88%.
The diff coverage is 92.48%.

@@            Coverage Diff             @@
##             main      #14      +/-   ##
==========================================
+ Coverage   74.60%   81.48%   +6.88%     
==========================================
  Files          19       20       +1     
  Lines        1788     2377     +589     
==========================================
+ Hits         1334     1937     +603     
+ Misses        454      440      -14     
Impacted Files Coverage Δ
src/lib.rs 13.51% <ø> (ø)
src/name/dns_name.rs 71.69% <ø> (+2.20%) ⬆️
src/name/name.rs 30.00% <30.00%> (ø)
src/name/verify.rs 36.23% <70.00%> (+32.08%) ⬆️
src/end_entity.rs 52.17% <100.00%> (+14.07%) ⬆️
src/name/ip_address.rs 92.90% <100.00%> (+92.90%) ⬆️
src/verify_cert.rs 88.60% <100.00%> (ø)
tests/integration.rs 100.00% <100.00%> (ø)
... and 2 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ereslibre ereslibre changed the base branch from feat-ip-address to main November 25, 2022 22:32
@ereslibre ereslibre force-pushed the rustls-feat-ip-address branch 12 times, most recently from fb34551 to 708d649 Compare November 29, 2022 20:42
@ereslibre ereslibre changed the base branch from main to feat-ip-address December 11, 2022 10:39
@ereslibre ereslibre marked this pull request as ready for review December 11, 2022 10:40
@ereslibre ereslibre changed the title WIP: increase test coverage of IP address support Increase test coverage of IP address support Dec 11, 2022
ctz and others added 5 commits December 11, 2022 17:21
This was currently broken, since one of the tests required
RSA ( = alloc feature) and real time (= std feature).
The latter is a mistake, cos tests should really be time-invariant.
Introduce `IpAddressRef`, `DnsNameOrIpRef` and the owned type
`IpAddress`.

Introduce a new public function `verify_is_valid_for_dns_name_or_ip`
that validates a given host name or IP address against a
certificate. IP addresses are only compared against Subject
Alternative Names.

It's possible to convert the already existing types `DnsNameRef` and
`IpAddressRef` into a `DnsNameOrIpRef` for better ergonomics when
calling to `verify_cert_dns_name_or_ip`.

The behavior of `verify_cert_dns_name` has not been altered, and works
in the same way as it has done until now, so that if `webpki` gets
bumped as a dependency, it won't start accepting certificates that
would have been rejected until now without notice.

Neither `IpAddressRef`, `DnsNameOrIpRef` nor `IpAddress` can be
instantiated directly. They must be instantiated through the
`try_from_ascii` and `try_from_ascii_str` public functions. This
ensures that instances of these types are correct by construction.

IPv6 addresses are only validated and supported in their uncompressed
form.

Signed-off-by: Rafael Fernández López <ereslibre@ereslibre.es>
current_textual_octet is [u8; 3] but it was indexed by an
unbounded count of octets if they matched 1..9.
rfc5952 says both are allowed.
Seems better to convert from ascii to radix-10 at the time that is
known, rather than doing that validation twice (and skipping a digit
as an error handling strategy).
src/name/dns_name.rs Outdated Show resolved Hide resolved
src/name/ip_address.rs Show resolved Hide resolved
This adds 100% line coverage to the IPv4 and IPv6 subject alternative
names validation implementation.
@ereslibre ereslibre force-pushed the rustls-feat-ip-address branch 2 times, most recently from 2d7c7e5 to d8ab051 Compare December 11, 2022 18:55
This adds 100% line coverage to the IPv4 and IPv6 subject alternative
names validation implementation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants