Skip to content

add tls-exporter api as par RFC 8446, Section 7.5#8

Open
goodspeed34 wants to merge 1 commit into
ptrd:masterfrom
goodspeed34:tls-exporter
Open

add tls-exporter api as par RFC 8446, Section 7.5#8
goodspeed34 wants to merge 1 commit into
ptrd:masterfrom
goodspeed34:tls-exporter

Conversation

@goodspeed34

Copy link
Copy Markdown

When quic was used with protocols (such as XMPP) using SASL, they may provide a *-PLUS mechanism with channel binding to mitigate MIMT attack. This commit introduces the tls-exporter api required by channel binding.

This commit introduces tls-exporter api, which is cruial for avoding MITM attack,
via channel binding.

@ptrd ptrd left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.
I agree that adding this functionality to agent15 / kwik is valuable.
However, I'm surprised (to put it mildly) by the errors you made in the implementation of computing the secrets. Did you actually read the RFC you are quoting from? Or is this AI generated (and hallucinated)?


/**
* Computes the TLS 1.3 exporter secret (RFC 8446, Section 7.5).
* The exporter secret is derived from the server's application traffic secret:

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong. It is derived from the master secret.

/**
* Computes the TLS 1.3 exporter secret (RFC 8446, Section 7.5).
* The exporter secret is derived from the server's application traffic secret:
* exporter_secret = Derive-Secret(server_application_traffic_secret, "exporter", "")

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong, should be master secret and the label should be "exp master"

*/
public void computeExporterSecret() {
// Derive-Secret(Secret, Label, Messages) = HKDF-Expand-Label(Secret, Label, Transcript-Hash(Messages), Hash.length)
// For the exporter, Messages is "" (empty), so Transcript-Hash("") = emptyHash

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong, Messages is not empty.

if (serverApplicationTrafficSecret == null) {
throw new IllegalStateException("Server application traffic secret not available");
}
exporterSecret = hkdfExpandLabel(serverApplicationTrafficSecret, "exporter", emptyHash, hashLength);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong label, wrong hash.
The result should be called exporterMasterSecret

/**
* Implements the TLS 1.3 exporter function (RFC 8446, Section 7.5):
* TLS-Exporter(label, context_value, key_length) =
* HKDF-Expand-Label(exporter_secret, label, context_value, key_length)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the RFC defines:
TLS-Exporter(label, context_value, key_length) =
HKDF-Expand-Label(Derive-Secret(Secret, label, ""),
"exporter", Hash(context_value), key_length)
which is quite different from what you wrote in the comment.

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