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

Escape forward slashes in certificate Subject names when used as user quota id strings #1059

Merged

Conversation

robstradling
Copy link
Contributor

@robstradling robstradling commented Apr 21, 2023

add-chain requests to Sectigo's Dodo log are failing for leaf certificates issued by certain Actalis CAs, with errors of this form:
AddChain handler error: backend QueueLeaves request failed: rpc error: code = ResourceExhausted desc = quota exhausted: invalid name: "quotas/users/@intermediate CN=Actalis Authentication Root CA,O=Actalis S.p.A./03358520967,L=Milan,C=IT 25d4913cf5/write/config"
Note the forward slash character in the "O" field ("Actalis S.p.A./03358520967").

Example certs: https://crt.sh/?id=9077408990, https://crt.sh/?id=9077429351

IINM, these errors are occurring because the string form of the issuer certificate's Subject name is matched against a regex that allows any character except the forward slash character.

To resolve this issue, this PR simply escapes (to "%2F") any forward slash character in the Subject name.

Checklist

@robstradling robstradling changed the title Escape forward slashes in user quota id strings Escape forward slashes in certificate Subject names when used as user quota id strings Apr 21, 2023
@robstradling
Copy link
Contributor Author

@defacto64 FYI.

Copy link
Member

@AlCutter AlCutter left a comment

Choose a reason for hiding this comment

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

Hi Rob, this seems like a good idea.

Is the PR in draft because you're waiting for some other input?

@AlCutter AlCutter self-assigned this Apr 25, 2023
@AlCutter AlCutter added the bug label Apr 25, 2023
@AlCutter
Copy link
Member

/gcbrun

@robstradling
Copy link
Contributor Author

Is the PR in draft because you're waiting for some other input?

It's just 'cos I haven't actually tested it yet. I'm trying to persuade our Ops team to deploy this change to our Dodo log so that I can do that.

@defacto64
Copy link

Hi Rob,

I acknowledge receipt, but I don't know what else to say.

I'm sorry that the peculiarity of that certificate has caused annoyance to Sectigo's Dodo log , however that certificate has been around for a very long time (since 2011) and in all this time it has never created problems for anyone, nor has any root store ever had anything to say on that certificate (and we are trusted in all platforms).

@robstradling
Copy link
Contributor Author

@defacto64 No need to apologise, Adriano! There's nothing wrong with that certificate. I just thought you might find it useful to be aware of this bug with Trillian-based logs, since the bug seems to prevent the acceptance (by the logs) of certificates issued by that Actalis CA.

@robstradling
Copy link
Contributor Author

I'm also seeing this same problem with Dodo (and Sabre) and leaf certificates issued by a fairly high-volume GoDaddy Sub-CA (https://crt.sh/?caid=904), due to forward-slash characters in the Issuer OU field. This example precert - https://crt.sh/?id=9393888385 - that expires in 2024 has been accepted by several other Trillian-based logs (Argon, Nimbus, Yeti).

Any idea why these other logs are seemingly unaffected by this bug? Is it because they've disabled quotas?

(Whatever the reason, I can see now why @defacto64 hadn't previously seen any issues).

@robstradling robstradling force-pushed the escape_slashes_in_quota_id_strings branch from 72d67ef to 98a9180 Compare May 17, 2023 16:29
@robstradling robstradling force-pushed the escape_slashes_in_quota_id_strings branch from 3d29e2a to f376497 Compare June 9, 2023 15:48
@robstradling robstradling marked this pull request as ready for review June 9, 2023 15:56
@robstradling robstradling requested a review from a team as a code owner June 9, 2023 15:56
@robstradling robstradling requested review from pphaneuf and removed request for a team June 9, 2023 15:56
@robstradling
Copy link
Contributor Author

Today we've deployed the fix in this PR to our Dodo log. I just submitted both of the certs mentioned in #1059 (comment), and this time both submissions were successful. :-)

@robstradling
Copy link
Contributor Author

Cloud Build failed. Any idea why? I suspect it's unrelated to the proposed changes in this PR.

@roger2hk
Copy link
Contributor

roger2hk commented Jun 9, 2023

Cloud Build failed. Any idea why? I suspect it's unrelated to the proposed changes in this PR.

The cloud build check has passed after re-run.

@robstradling
Copy link
Contributor Author

We've also deployed the fix in this PR to our Sabre log.

@AlCutter AlCutter merged commit 88a932b into google:master Jun 12, 2023
4 checks passed
@AlCutter
Copy link
Member

Thanks, @robstradling!

@robstradling robstradling deleted the escape_slashes_in_quota_id_strings branch June 12, 2023 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants