ChenSammi commented on PR #5404: URL: https://github.com/apache/ozone/pull/5404#issuecomment-1769869597
> This - if I am correct - means that we will generate a new certificate that is identical with the old one, and store it in SCM's rocksDB with a different certificate serial ID. So we will have lingering certificates in the db, and we currently do not have any facility to revoke them. I am not sure if this can cause any other problem, but I am worried a bit for Delegation Token related functionalities in this case, especially if we remove the previous cert from the DB. > @fapifta , thanks for the review. Yes, a new CSR will generate a new certificate. For the old certificate, we don't have to revoke it since the private key is still there, not leaked. As for delegation token, it's fine, sine the delegation token generation and verification requires private key and public key, and key is not changed in this case. The only thing is the old certificate will be remained in SCM rocksdb until it's expired and cleaned up by daemon thread. Previously I think this case is of low priority since both the public key and certificate missing is a very rare case. But if the patch can help to reduce one chance that originally requires admin to manually intervene and fix, then I think it's worth to do if it doesn't introduce too much complexity. Regarding the fetching old certificate back proposal, besides the missing service certificates, there could be also CA and root CA certificates are missing too. So a bunch of certificates need to fetched back. And I checked the current SCMSecurityProtocolPB, looks we need to add new APIs to handle this case. Given the case is very rare, I'm not convinced myself to introduce all these new logic to solve this rare case. > * OzoneManager does call the recoverStateIfNeeded only in the constructor, and leaves the old logic in initializeSecurity() which is called in omInit(). > Previously OzoneManager call recoverStateIfNeeded, but it doesn't work. Because recoverStateIfNeeded requires a certIdSaveCallback callback to save the new certificate ID into the VERSION file, while initializeSecurity is a static, saveNewCertId cannot be passed into it. Let me check if there is other possible ways. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
