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

gen-certurl: Read SCT files from directory specified with -sctDir #259

Merged
merged 2 commits into from
Jul 18, 2018
Merged

gen-certurl: Read SCT files from directory specified with -sctDir #259

merged 2 commits into from
Jul 18, 2018

Conversation

irori
Copy link
Collaborator

@irori irori commented Jul 18, 2018

Instead of reading a serialized SignedCertificateTimestampList from a
file, read all *.sct files in a directory given by the -sctDir flag
and serialize them into SignedCertificateTimestampList.

This matches the way how nginx-ct and Apache's mod_ssl_ct module work,
and allows users to use existing SCT generation tools.

Instead of reading a serialized SignedCertificateTimestampList from a
file, read all *.sct files in a directory given by the -sctDir flag
and serialize them into SignedCertificateTimestampList.

This matches the way how nginx-ct and Apache's mod_ssl_ct module work,
and allows users to use existing SCT generation tools.
@irori irori requested review from hajimehoshi and nyaxt July 18, 2018 01:58
# Fill in dummy data for OCSP/SCT, since the certificate is self-signed.
gen-certurl -pem cert.pem -ocsp <(echo ocsp) -sct <(echo sct) > cert.cbor
# Fill in dummy data for OCSP, since the certificate is self-signed.
gen-certurl -pem cert.pem -ocsp <(echo ocsp) > cert.cbor
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I removed -sct here since SCT is not necessary for self-signed certs.

I plan to add a separate section for instructions to create signed exchanges with trusted certificates, and will explain the usage of -sctDir there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

sgtm

# Fill in dummy data for OCSP/SCT, since the certificate is self-signed.
gen-certurl -pem cert.pem -ocsp <(echo ocsp) -sct <(echo sct) > cert.cbor
# Fill in dummy data for OCSP, since the certificate is self-signed.
gen-certurl -pem cert.pem -ocsp <(echo ocsp) > cert.cbor
Copy link
Collaborator

Choose a reason for hiding this comment

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

sgtm

}

var buf bytes.Buffer
buf.Grow(total_length + 2) // +2 for length
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this line necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed.


var buf bytes.Buffer
buf.Grow(total_length + 2) // +2 for length
binary.Write(&buf, binary.BigEndian, uint16(total_length))
Copy link
Collaborator

Choose a reason for hiding this comment

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

please catch err:

if err := binary.Write(...); err != nil {
  panic(err) or return nil, err
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done (here and elsewhere).

buf.Grow(total_length + 2) // +2 for length
binary.Write(&buf, binary.BigEndian, uint16(total_length))
for _, sct := range scts {
binary.Write(&buf, binary.BigEndian, uint16(len(sct)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto

binary.Write(&buf, binary.BigEndian, uint16(total_length))
for _, sct := range scts {
binary.Write(&buf, binary.BigEndian, uint16(len(sct)))
buf.Write(sct)
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto

@nyaxt nyaxt merged commit 89928f9 into WICG:master Jul 18, 2018
@irori irori deleted the sct branch July 18, 2018 03:57
aarongable pushed a commit to chromium/chromium that referenced this pull request Jul 30, 2018
gen-certurl's -sct option is deprecated in
WICG/webpackage#259.

Bug: 803774
Change-Id: I4fbc575366aba978b262418d2cf415121e840099
Reviewed-on: https://chromium-review.googlesource.com/1154745
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578999}
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