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

Clarify usage and requirements for random numbers #82

Conversation

timfromdigicert
Copy link
Contributor

No description provided.

Copy link
Contributor

@sleevi sleevi left a comment

Choose a reason for hiding this comment

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

I'm not a big fan of this approach to changes. I think it overloads too much normative behaviour into the definitions, and I think it makes it much harder to read (and to see the impact of the normative change)

From what I can tell, the substance of normative changes is isolated to 3.2.2.4.10, which seems a much better place to make a localized change (e.g. consistent with line 606[old] or 610[new] regarding freshness)

@@ -267,6 +267,8 @@ No stipulation.

**Expiry Date**: The "Not After" date in a Certificate that defines the end of a Certificate's validity period.

**Freshness Value**: A Random Value that cannot be predicted in advance, and is intended to prevent the re-use of stale data and replay attacks. It need not remain secret.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this is meaningfully distinct from a Random Value - specifically, the "predicted in advance" aspect.

I don't think this would be a definitional thing, I think it's something that would be part of the explicit requirements themselves.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

More than happy to move requirements out of definitions. The only real intent of this is to distinguish the two uses of random values. It has caused confusion in the past and will continue to cause confusion in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I fully understand what you see as "two uses"

Your PR touches on two dimensions - secret/non-secret (although that seems to be an expression of intent rather than what the method guarantees?) and fresh/stale (where fresh is defined by the validity period of the random value within the validation method)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, if you look at the validation requirements, we only use two of the four combinations:

Fresh and must be Secret
Fresh and need not be Secret

where as you note, it's actually pseudo-freshness, which is why Wayne wanted to avoid using the word nonce.

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not believe that's the case.

We also have
Not fresh and need not be secret

3.2.2.4.10 is this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I noticed that and believe it's actually a bug in #10.

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean, it was by design from GlobalSign :)

It's not desirable for security, I think, but it's desirable to at least one CA.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

GlobalSign is method #9. Method #10 came from Let's Encrypt through Gerv and suffered from a bad case of inattention to detail and the consequences of playing telephone instead of talking directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -347,6 +349,8 @@ The binding SHALL use a digital signature algorithm or a cryptographic hash algo

**Root Certificate**: The self-signed Certificate issued by the Root CA to identify itself and to facilitate verification of Certificates issued to its Subordinate CAs.

**Secret Value**: A Random Value intended to be communicated to a third party, where the security of the associated process relies upon that value remaining secret.
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly, I don't think this is an improvement. I think it hinders readability, and the goal of what you're trying to add ("where the security of ...") is an expression of intent more than it is a definitional issue.

@wthayer wthayer changed the base branch from master to main July 31, 2020 18:20
@dzacharo
Copy link
Contributor

@timfromdigicert @sleevi are you happy to close this PR? If you still see that the current language in the BRs is ambiguous, perhaps we should convert this into an issue and close the PR.

@wthayer
Copy link
Contributor

wthayer commented Nov 5, 2023

@timfromdigicert @CBonnell discussed this at the 11/2 validation sc meeting and agreed to close it.

@wthayer wthayer closed this Nov 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

None yet

4 participants