Bug 1805541 improvement over verifySCT - RFE] CA Certificate Transpar… #440
Conversation
| * - then deleted again | ||
| * It can be used for comparison with | ||
| * tbsCert_save_s to check if things get | ||
| * out of order for debugging purpose. |
There was a problem hiding this comment.
By the term "order", if you are referring to the order of extensions within the exts object the current implementation of CertificateExtension uses Hashtable, which does not guarantee the order.
As you mentioned earlier, using LinkedHashMap in JSS would preserve the order of the extensions.
There was a problem hiding this comment.
Yes. As I explained in my comment, I do not wan to pose dependency on JSS yet as I'm just doing this for @frasertweedale to help taking a look quickly without waiting for JSS build. As you can see, the code that supports LinkedHashMap has been commented out temporarily for now.
There was a problem hiding this comment.
OK.
Not that once you push the changes to JSS, the CI does the JSS builds automatically (in COPR). This new build will be automatically pulled into this CI execution (ie) you don't have to wait for official jss builds :-)
There was a problem hiding this comment.
oh? I did not know that!! In that case, I'd rather switch over to LinkedHashMap on both sides (JSS and PKI). The build magical world never seizes to amaze me. ;-)
| String extensions_s = response.getExtensions(); | ||
| byte[] extensions = null; | ||
| int extensions_len = 0; | ||
| if (extensions_s != null && !extensions_s.equals("")) { |
There was a problem hiding this comment.
nitpick: use extensions_s.isEmpty() instead of !extensions_s.equals("")
There was a problem hiding this comment.
sure. sounds good. Will do.
ladycfu
force-pushed
the
Bug1805541-verifySCT-Certificate-Transparency-with-Embedded-SCT
branch
from
9168ae9
to
b21fe97
Compare
…rency with Embedded Signed Certificate Time stamp
This patch made some more attempt to improve on verifySCT
(though still not working; lack of the signed blob from sender
makes it a bit challenging)
It adds the following:
- Include code to use LinkedHashMap instead of Hashtable (requires jss fix)
- Added debugging code to be sure that the extensions didn’t get out of order through manipulation
- Allow for CT lg connection issue, but disallow for failed CT verification (though still temporarily disable failure for signature verification)
- For verifySCT
- Added missing 3 byte length for tbsCert
- Added processing for extensions, though most likely not needed for some time
Note: the global on/off is rigid at this point without "per-profile" control;
https://bugzilla.redhat.com/show_bug.cgi?id=1805541
ladycfu
force-pushed
the
Bug1805541-verifySCT-Certificate-Transparency-with-Embedded-SCT
branch
from
b21fe97
to
db4b03f
Compare
|
|
||
| byte[] log_key_hash = null; | ||
| try { | ||
| byte[] log_key_hash = null; | ||
| MessageDigest SHA256Digest = MessageDigest.getInstance("SHA256"); | ||
|
|
||
| log_key_hash = SHA256Digest.digest(log_pubKey.getEncoded()); |
There was a problem hiding this comment.
Does getEncoded() return a DER-encoded SubjectPublicKeyInfo? That is what it needs to be.
If it is not, maybe digest logPublicKey_b which should be in the correct format.
| /* compose data */ | ||
| byte[] version = new byte[] {0}; // 1 byte; v1(0) | ||
| byte[] signature_type = new byte[] {0}; // 1 byte; certificate_timestamp(0) | ||
| byte[] entry_type = new byte[] {0, 1}; // 2 bytes; LogEntryType: precert_entry(1) |
There was a problem hiding this comment.
This is a good pickup! I missed this.
|
Closing in favour of #450 which includes the commit from this PR. |
…ency with Embedded Signed Certificate Time stamp