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

Agent Certificate has a circular dependency on itself #276

Open
sleevi opened this issue Apr 6, 2021 · 9 comments · May be fixed by #332
Open

Agent Certificate has a circular dependency on itself #276

sleevi opened this issue Apr 6, 2021 · 9 comments · May be fixed by #332
Assignees
Labels
F2F security-tracker Group bringing to attention of security, or tracked by the security Group but not needing response. v1-spec

Comments

@sleevi
Copy link

sleevi commented Apr 6, 2021

The value fp is defined as:

openscreenprotocol/index.bs

Lines 285 to 291 in 5488c7b

: fp
:: The <dfn>agent fingerprint</dfn> of the advertising agent, computed over the
[=agent certificate=]. The format of the fingerprint is defined by [[!RFC5122]],
excluding the "fingerprint:" prefix and including the hash function, space,
and hex-encoded fingerprint. The fingerprint value also functions as an ID
for the agent. All agents must support the following hash functions: [=sha-256=],
[=sha-512=]. Agents must not support the following hash functions: [=md2=], [=md5=].

Agent Certificate then follows, with the definition at:

openscreenprotocol/index.bs

Lines 354 to 417 in 5488c7b

Agent Certificates {#certificates}
----------------------------------
Each OSP Agent must generate an [[!RFC5280|X.509 v3]] <dfn>agent
certificate</dfn> containing a public key to be used with the TLS 1.3
certificate exchange. Both [=advertising agents=] and [=listening agents=] must
use the [=agent certificate=] in TLS 1.3 `Certificate` messages when making a
QUIC connection.
The [=agent certificate=] must have the following characteristics:
* 256-bit, 384-bit, or 521-bit ECDSA public key
* Self-signed
* Supporting the at least one of the following signature algorithms:
* secp256r1_sha256
* secp384r1_sha384
* secp521r1_sha512
* Valid for signing
The following X.509 v3 fields are to be set as follows:
<table>
<thead>
<th>Field</th>
<th>Value</th>
</thead>
<tbody>
<tr>
<td>Version Number</td>
<td>3</td>
</tr>
<tr>
<td>Serial Number</td>
<td>`<fp>`</td>
</tr>
<tr>
<td>Signature Algorithm ID</td>
<td>One of the values listed above.</td>
</tr>
<tr>
<td>Issuer Name</td>
<td>CN = The `model-name` from the `agent-info` message.<br/>
O = See note.<br/>
L = See note.<br/>
ST = See note.<br/>
C = See note.<br/>
</td>
</tr>
<tr>
<td>Subject Name</td>
<td>CN = `<fp>`._openscreen._udp.local<br/>
O = See note.<br/>
</td>
</tr>
<tr>
<td>Subject Public Key Algorithm</td>
<td>Elliptic Curve Public Key</td>
</tr>
<tr>
<td>Certificate Key usage</td>
<td>Signing</td>
</tr>
</tbody>
</table>

The issue is that the certificate Serial Number field makes use of fp, as described at

openscreenprotocol/index.bs

Lines 385 to 388 in 5488c7b

<tr>
<td>Serial Number</td>
<td>`<fp>`</td>
</tr>

The problem is that fp is computed over the certificate, so one cannot compute fp apriori to include within the certificate, and any inclusion within the certificate would naturally change the derived fingerprint.

@mfoltzgoogle
Copy link
Contributor

We also need to fix the certificate subject name not to refer to the certificate fingerprint.

@backkem
Copy link

backkem commented Apr 29, 2024

Regarding the subject name, similarly replacing it with the serial number could look as follows:

  1. Advertise the serial number as a sn mDNS TXT record.
  2. use <sn>._openscreen._udp as the certificate Subject CommonName.
  3. The dialing client can use the sn TXT record to construct the <sn>._openscreen._udp to use in the server_name extension.

This keeps the 1:1 relation between server name and certificate. It don't think it impacts the certificate validation based on the fingerprint. That being said, I may have missed some of the original considerations put into the current spec. If someone can validate that, I'm happy to do the writeup and send a PR for review.

@wangw-1991
Copy link
Contributor

It seems "Issuer Name" also has problem. According to the spec, "Issuer Name" is set as the model-name from the agent-info message. However, the model-name information is exchanged during the metadata phase which happens after the connection. Is there something wrong with my understanding?

@backkem
Copy link

backkem commented Apr 30, 2024

It seems "Issuer Name" also has problem. According to the spec, "Issuer Name" is set as the model-name from the agent-info message. However, the model-name information is exchanged during the metadata phase which happens after the connection. Is there something wrong with my understanding?

My interpretation of this is that it's the model name of the agent creating the certificate itself, meaning the model name as it is also added to the agent-info message; not the model name as is received from the remote agent. Can someone confirm/deny this? If my interpretation is correct, we may want to rephrase this to avoid any confusion.

@mfoltzgoogle
Copy link
Contributor

Regarding the subject name, similarly replacing it with the serial number could look as follows:

That sounds like a great approach. bakkem@, do you have the capacity to put up a proposed PR that implements that?

@mfoltzgoogle
Copy link
Contributor

It seems "Issuer Name" also has problem. According to the spec, "Issuer Name" is set as the model-name from the agent-info message

This should be same as the the model-name of the agent that is generating the certificate. The text here could be clearer on this.

The entire Issuer Name field is mostly for debugging purposes as the certificates are self-signed.

@backkem
Copy link

backkem commented Apr 30, 2024

Regarding the subject name, similarly replacing it with the serial number could look as follows:

That sounds like a great approach. bakkem@, do you have the capacity to put up a proposed PR that implements that?

I will start with the spec changes. Afterwards I may look at the C implementation. My Go implementation already does this as the spec was implementable otherwise 😅

@backkem
Copy link

backkem commented Apr 30, 2024

I just remembered there is another precedent for this set by WebRTC's local ICE candidates:

Generate a unique mDNS hostname. The unique name MUST consist of
a version 4 UUID as defined in [RFC4122], followed by ".local".

I'm not sure if the added entropy would be warranted in our case.

backkem added a commit to backkem/openscreenprotocol that referenced this issue Apr 30, 2024
This resolves the circular dependency in agent certificate fingerprint.

Resolves w3c#276
@backkem backkem linked a pull request Apr 30, 2024 that will close this issue
backkem added a commit to backkem/openscreenprotocol that referenced this issue Apr 30, 2024
This resolves the circular dependency in agent certificate fingerprint.

Resolves w3c#276
@wangw-1991
Copy link
Contributor

It seems "Issuer Name" also has problem. According to the spec, "Issuer Name" is set as the model-name from the agent-info message

This should be same as the the model-name of the agent that is generating the certificate. The text here could be clearer on this.

The entire Issuer Name field is mostly for debugging purposes as the certificates are self-signed.

Thanks, this makes sense to me.

backkem added a commit to backkem/openscreenprotocol that referenced this issue May 16, 2024
This resolves the circular dependency in agent certificate fingerprint.

Resolves w3c#276
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F2F security-tracker Group bringing to attention of security, or tracked by the security Group but not needing response. v1-spec
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants