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

Fix typo in nss_keylog_export #372

Open
wants to merge 1 commit into
base: tls13-prototype
Choose a base branch
from

Conversation

lhuang04
Copy link
Collaborator

@lhuang04 lhuang04 commented Sep 7, 2021

Summary:
CLIENT RANDOM -> CLIENT_RANDOM. This appears a typo as it is CLIENT_RANDOM in upstream and in the document.

Test Plan:
tests/ssl-opt.sh

Reviewers:

Subscribers:

Tasks:

Tags:

Notes:

  • Pull requests cannot be accepted until the PR follows the contributing guidelines. In particular, each commit must have at least one Signed-off-by: line from the committer to certify that the contribution is made under the terms of the Developer Certificate of Origin.
  • This is just a template, so feel free to use/remove the unnecessary things

Description

A few sentences describing the overall goals of the pull request's commits.

Status

READY/IN DEVELOPMENT/HOLD

Requires Backporting

When there is a bug fix, it should be backported to all maintained and supported branches.
Changes do not have to be backported if:

  • This PR is a new feature\enhancement
  • This PR contains changes in the API. If this is true, and there is a need for the fix to be backported, the fix should be handled differently in the legacy branch

Yes | NO
Which branch?

Migrations

If there is any API change, what's the incentive and logic for it.

YES | NO

Additional comments

Any additional information that could be of interest

Todos

  • Tests
  • Documentation
  • Changelog updated
  • Backported

Steps to test or reproduce

Outline the steps to test or reproduce the PR here.

Summary:
`CLIENT RANDOM` -> `CLIENT_RANDOM`. This appears a typo as it is
[CLIENT_RANDOM](https://github.com/ARMmbed/mbedtls/blob/development/programs/ssl/ssl_test_common_source.c#L72) in upstream and in the
[document](https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/Key_Log_Format).

Test Plan:
`tests/ssl-opt.sh`

Reviewers:

Subscribers:

Tasks:

Tags:
@hannestschofenig
Copy link
Owner

The change suggested refers to the exporter used with TLS 1.2. However, the label used "CLIENT RANDOM" is not one of the registered labels, see https://datatracker.ietf.org/doc/html/rfc5705 and https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#exporter-labels.

Since even the two key derivation functions are different, I wonder whether a separate code path for 1.2 is required and whether this entry should be removed.

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