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

Add a runner for botan tls_client #28

Merged
merged 1 commit into from
May 24, 2024

Conversation

randombit
Copy link
Contributor

No description provided.

@JackOfMostTrades
Copy link
Contributor

This is great, thanks!

For reference, I ran this using both the version of botan shipped with Ubuntu (jammy) and the latest release built from source (3.4.0). Looks like I got the same results for both:

$ ./bettertls run-tests --implementation botan
Implementation: botan
Version: Botan 2.19.1 (unreleased, revision unknown, distribution Ubuntu)

Suite: nameconstraints
  Supported Features: NAME_CONSTRAINTS, 
  Unsupported Features: VALIDATE_DNS, VALIDATE_IP, 
  Passed: 1300
    Passed with warnings: 480
  Skipped: 6571
  Failures: 1620
    False negatives: 1620

Suite: pathbuilding
  Supported Features: BRANCHING, 
  Unsupported Features: INVALID_REASON_EXPIRED, INVALID_REASON_NAME_CONSTRAINTS, INVALID_REASON_BAD_EKU, INVALID_REASON_MISSING_BASIC_CONSTRAINTS, INVALID_REASON_NOT_A_CA, INVALID_REASON_DEPRECATED_CRYPTO, 
  Passed: 9
  Skipped: 72
  Failures: 0
$ ./bettertls run-tests --implementation botan
Implementation: botan
Version: Botan 3.4.0 (release, dated 20240408, revision git:afd43c536ff96e85d45837caa6f99125483c160a, distribution unspecified)

Suite: nameconstraints
  Supported Features: NAME_CONSTRAINTS, 
  Unsupported Features: VALIDATE_DNS, VALIDATE_IP, 
  Passed: 1300
    Passed with warnings: 480
  Skipped: 6571
  Failures: 1620
    False negatives: 1620

Suite: pathbuilding
  Supported Features: BRANCHING, 
  Unsupported Features: INVALID_REASON_EXPIRED, INVALID_REASON_NAME_CONSTRAINTS, INVALID_REASON_BAD_EKU, INVALID_REASON_MISSING_BASIC_CONSTRAINTS, INVALID_REASON_NOT_A_CA, INVALID_REASON_DEPRECATED_CRYPTO, 
  Passed: 9
  Skipped: 72
  Failures: 0

@randombit
Copy link
Contributor Author

Indeed, there are a number of issues that prevented BetterTLS from running correctly against the latest release. Some of these were actual name constraint bugs, others were more along the lines of SNI handling, etc. (For example if the cli tool tls_client detecting you connecting to an IP address, it would not set the hostname, to avoid sending the IP in the TLS SNI. However in doing so as a side effect it also disabled any IP matching against the cert, which broke how BetterTLS detects if name constraints are supported at all)

With various changes (some on master some still WIP) things are looking rather more satisfactory:

Implementation: botan
Version: Botan 3.5.0 (unreleased, revision git:08217e54e685a37011e0fc4e12c5e23e16f97c6e, distribution unspecified)

Suite: nameconstraints
  Supported Features: NAME_CONSTRAINTS, VALIDATE_DNS, VALIDATE_IP,
  Unsupported Features:
  Passed: 9491
    Passed with warnings: 36
  Skipped: 0
  Failures: 0

Suite: pathbuilding
  Supported Features: BRANCHING, INVALID_REASON_EXPIRED, INVALID_REASON_NAME_CONSTRAINTS, INVALID_REASON_MISSING_BASIC_CONSTRAINTS, INVALID_REASON_NOT_A_CA, INVALID_REASON_DEPRECATED_CRYPTO,
  Unsupported Features: INVALID_REASON_BAD_EKU,
  Passed: 69
  Skipped: 12
  Failures: 0

Going forward the plan would be to integrate this into our CI flow to prevent any future regressions.

@JackOfMostTrades
Copy link
Contributor

Awesome! I've certainly seen a handful of the other implementation runners not quite handle the SNI or hostname/IP check, more as an artifact of how the CLI tool is invoked than an issue with the TLS implementation itself. Thanks for doing the tweaking to make this more effectively testable!

@JackOfMostTrades JackOfMostTrades merged commit 5b56e72 into Netflix:master May 24, 2024
1 check passed
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.

2 participants