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

Add tests for conflicts for CA with SSNv2 #4896

Merged
merged 1 commit into from
Nov 6, 2024

Conversation

edewata
Copy link
Contributor

@edewata edewata commented Nov 6, 2024

The test for CA with SSNv2 has been modified to check how the CA handles conflicting requests and certs in the database.

The test for CA with SSNv2 has been modified to check how the
CA handles conflicting requests and certs in the database.
@edewata edewata requested a review from fmarco76 November 6, 2024 17:34
Copy link

sonarqubecloud bot commented Nov 6, 2024

Copy link
Member

@fmarco76 fmarco76 left a comment

Choose a reason for hiding this comment

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

LGTM

I have already tested this behaviour using the provided PR which we are not using, with some range combination I was getting range overlap.
Not sure this should work. The system is thought without conflicts and if it happens there should be a configuration problem. Should the range be discarded and move to the next one? Using just the first possible serial could create problems. As an example it is possible muiltiple clones write in the same range

@edewata
Copy link
Contributor Author

edewata commented Nov 6, 2024

@fmarco76 Thanks!

Yeah, I'm not sure if we need to fix it right now, but from the user's perspective it would be nice if the CA can handle this automatically. I'm just hoping that we can use the same strategy for both SSN and RSN, basically something like this:

while true:
    serial_number = generate_id()
    try:
        cert = issue_cert(serial_number)
        break
    catch exception:
        continue

return cert

@edewata edewata merged commit 4d59421 into dogtagpki:master Nov 6, 2024
159 of 168 checks passed
@fmarco76
Copy link
Member

fmarco76 commented Nov 7, 2024

@edewata Yes, we could perform some automatic recover but the error could be generated by different problems and not sure about this strategy for SSN. If the genereate id is already in use then there is a range conflict and the following id could be used by other clones or the allocated range overlap with the previous one. In either case, I think the CA with problem should move to a new range and report the same problem of range exhausted.

However, since we are pushing towards RSNv3 I think this is very low priority.

@edewata
Copy link
Contributor Author

edewata commented Nov 7, 2024

There's actually a bug in the test causing the CLI to fail (see PR #4898).

I was actually thinking about a single conflict within the range, possibly caused by importing a cert manually into DS or mixing SSN and RSNv3 in the same environment, so the above pseudo code should be sufficient to hide the issue from the user.

Suppose there are many conflicts caused by conflicting ranges in multiple servers, moving to a new range will fix it, but there's no guarantee that the conflicting ranges are actually identical (and it might be difficult to check programmatically), so there's a risk that it will introduce a new gap. Regardless, there is no requirement to fix this right now.

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