Regression in master branch: replica fails to retrieve subca key #4032
Comments
|
@ckelleyRH the commits between the 2 builds were the following: 47b3aff looks like a good candidates to investigate. |
|
custodia-related logs look fine.
|
|
How is the key retriever configured in IPA? Does the command in |
The ExternalProcessKeyRetriever has been updated to provide more detailed log messages and to improve JSON validation of the result. A new constructor has been added to create a JSONObject from a String. dogtagpki#4032
The ExternalProcessKeyRetriever has been updated to provide more detailed log messages for troubleshooting and to improve the validation of the JSON result. A new constructor has been added to create a JSONObject from a String. dogtagpki#4032
|
I created a PR to update the log messages and the validation: #4037 Please try again with this build. Please feel free to comment directly in the PR as well. Thanks. |
The ExternalProcessKeyRetriever has been updated to provide more detailed log messages and to perform proper JsonNode validations of the result to help troubleshooting. A new constructor has been added to create a JSONObject from a String. dogtagpki#4032
|
@edewata the command is required to return JSON. The actual program is part of the IPA package, and FreeIPA configures Dogtag to use that command. I'll continue investigating. |
The ExternalProcessKeyRetriever has been updated to provide more detailed log messages and to perform proper JsonNode validations of the result to help troubleshooting. A new constructor has been added to create a JSONObject from a String. #4032
|
Just FYI, the PR has been merged and the COPR build is available from |
|
Its still failing in report |
|
@ckelleyRH I strongly suspect this is a regression from commit 47b3aff (June 10, which fits the timeline of this issue). What was the motivation for the above commit? I don't (yet) fully understand the change but the usage in JsonNode responseNode = parser.getJsonNode().get("Response");
String status = responseNode.get("Status").asText();
logger.debug("Status: " + status); Compared with JsonNode root = new JSONObject(result).getRootNode();
JsonNode certNode = root.path("certificate");
...Perhaps we just need to replace |
|
@ckelleyRH yeah, that was the issue. There was another regression caused by the more recent commit to emit more diagnostic info from ExternalProcessKeyRetriever. I'll have a PR ready soon. |
Commit 47b3aff refactors the JSON handling in ExternalProcessKeyRetriever. It introduced a regression that causes lightweight CA key replication to fail. It scrutinises an empty node instead of the JSON node containing the object loaded from the InputStream. Update the code to scrutinise the correct node. Separately, commit 6c116bf enhanced the ExternalProcessKeyRetriever diagnostic output. It also broke key retrieval. The `wrapped_key` field value is expected to be a text node containing base64-encoded data. We retrieve its decoded value using `.binaryValue()`. That commit added a check that throws an exception if `node.isBinary()` is false. However, the node is parsed as a com.fasterxml.jackson.databind.node.TextNode, for which `.isBinary()` returns false. This commit removes the check. `.binaryValue()` will (as before) throw an `IOException` if base64 decoding fails. Fixes: dogtagpki#4032
|
It was part of a series of changes to encapsulate JSON parsing and reduce boilerplate, to pave the road for the eventual total removal of XML, thanks for figuring this one out! |
|
@frasertweedale @ckelleyRH We can of course modify the test code to ignore this unexpected orphan key but the behavior is different with pki 11.1.0-1 (no orphan key detected). Does it ring any bell, do you have any idea if it could be caused by new pki code from the master branch? |
|
Update: the orphan key is not present with dogtag-pki-base-11.2.0-2.fc37.noarch, it only happens when using pki from the copr repo @pki/master. |
|
@flo-renaud I think this might be another consequence of the change to have JSS issue the temp cert on server startup rather than NSS, there is no orphan when deploying the main CA. I have not tried with a clone yet but the PKI CI doesn't seem to have any orphans. |
|
Ticket #4103 opened to track the orphan key. |
|
@flo-renaud just brain-dumping my thoughts about how the IPA test could be made more resilient. I don't suggest any particular course of action, just sharing observations.
This would make the test resilient if more, unrelated certificates were to appear in the NSSDB (for whatever reason - maybe new features, but also maybe bugs, as in the current case). So it is a tradeoff. On the one hand, the test spuriously fails in the presence of new certificates (which may be a legitimate change). On the other hand, the failure can reveal real issues, although those issues are probably not related to LWCA key replication. |
FreeIPA CI detected a regression using the nightly build from @pki/master copr repo, see PR#1784 with the test
test_replica_promotion_TestSubCAkeyReplication(Details, report).Test scenario:
ipa ca-add test_subca_master --subject cn=test_subca_master --desc subcaThe logs in /var/log/pki/pki-tomcat/ca/debug show that PKI properly sees the CA LDAP entry on the replica but it fails to retrieve the key.
The issue was first detected with dogtag-pki-ca-11.3.0-0.1.alpha1.20220613143838UTC.6bc4706b.fc36.noarch (the run from the previous week was ok, with dogtag-pki-ca-11.3.0-0.1.alpha1.20220526131710UTC.1ca4e256.fc36.noarch).
Companion issue on freeipa side: https://pagure.io/freeipa/issue/9182
The text was updated successfully, but these errors were encountered: