Skip to content

Add TLS support for migrillian#1525

Merged
mhutchinson merged 1 commit intogoogle:masterfrom
fghanmi:TrillianTLSSupport
Jul 4, 2024
Merged

Add TLS support for migrillian#1525
mhutchinson merged 1 commit intogoogle:masterfrom
fghanmi:TrillianTLSSupport

Conversation

@fghanmi
Copy link
Copy Markdown
Contributor

@fghanmi fghanmi commented Jul 1, 2024 •
edited
Loading

Summary

This pull request introduces the option to specify a CA certificate for establishing secure connections with the Trillian server.
By using --trillian_tls_ca_cert_file flag, users can provide a CA certificate, that is used to establish a secure communication with Trillian server.

Release Note

  • Feature: Added support for TLS security for Trillian server.
    New Flag: --trillian_tls_ca_cert_file to specify the file path to the CA certificate.
    Behavior: If --trillian_tls_ca_cert_file flag is not provided, the system will default to insecure connections.
    Security: This update significantly enhances the security of data in transit by enabling TLS.

Resolves Issue: #1524

Checklist

@fghanmi fghanmi requested a review from a team as a code owner July 1, 2024 09:36
@fghanmi fghanmi requested review from mhutchinson and removed request for a team July 1, 2024 09:36
@fghanmi
Copy link
Copy Markdown
Contributor Author

fghanmi commented Jul 1, 2024

Hello,
It seems that "Missing permissions: cloudbuild.builds.get" prevents the build from starting/succeeding.
Could you please help ?
Thanks!

@fghanmi fghanmi force-pushed the TrillianTLSSupport branch from f1e140e to 4b960ec Compare July 1, 2024 12:42
mhutchinson
mhutchinson reviewed Jul 3, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@mhutchinson mhutchinson left a comment

Choose a reason for hiding this comment

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

Can you clarify in the PR description that this support is only for TLS certs signed by a CA. This does not add support for self-signed TLS certs.

@fghanmi fghanmi force-pushed the TrillianTLSSupport branch 2 times, most recently from 7b95849 to 9593986 Compare July 3, 2024 15:00
@fghanmi
Copy link
Copy Markdown
Contributor Author

fghanmi commented Jul 3, 2024

Can you clarify in the PR description that this support is only for TLS certs signed by a CA. This does not add support for self-signed TLS certs.

CA certificate can be self-generated. Here, we only need the CA certificate to validate Trillian's TLS certificates to establish the secure communication.
The TLS certs signed by a CA (or self-signed TLS certs) will be used to secure CT log server (in the other issue)

@fghanmi fghanmi requested a review from mhutchinson July 3, 2024 15:30
@mhutchinson
Copy link
Copy Markdown
Contributor

mhutchinson commented Jul 3, 2024

Can you clarify in the PR description that this support is only for TLS certs signed by a CA. This does not add support for self-signed TLS certs.

CA certificate can be self-generated. Here, we only need the CA certificate to validate Trillian's TLS certificates to establish the secure communication. The TLS certs signed by a CA (or self-signed TLS certs) will be used to secure CT log server (in the other issue)

Don't worry about it. Thinking about this some more, the self-signed cert method will still work using this approach as far as I can see because the end-cert is also its own root cert.

@mhutchinson
Copy link
Copy Markdown
Contributor

mhutchinson commented Jul 3, 2024

/gcbrun

@fghanmi
Copy link
Copy Markdown
Contributor Author

fghanmi commented Jul 3, 2024 •
edited
Loading

/gcbrun

I've checked the govulncheck / Run govulncheck (pull_request) error, and it seems, master does have the same issue.

@mhutchinson
Copy link
Copy Markdown
Contributor

mhutchinson commented Jul 3, 2024

/gcbrun

I've checked the govulncheck / Run govulncheck (pull_request) error, and it seems, master does have the same issue.

Yeah that's because govulncheck isn't hermetic and so the master branch fails now, even though it passed when it was last updated. I'll try to get a PR in that fixes it 👍

@mhutchinson
Copy link
Copy Markdown
Contributor

mhutchinson commented Jul 3, 2024

#1527

@roger2hk
Copy link
Copy Markdown
Contributor

roger2hk commented Jul 3, 2024

@fghanmi #1527 has been merged. Please rebase the branch.

@fghanmi fghanmi force-pushed the TrillianTLSSupport branch from 9593986 to ec708d9 Compare July 3, 2024 18:27
@mhutchinson
Copy link
Copy Markdown
Contributor

mhutchinson commented Jul 4, 2024

I was just about to merge this and noticed that you haven't updated the CHANGELOG file. Can you copy in the notes from this PR description into the CHANGELOG.md file? Thanks!

@fghanmi
Add TLS support for Trillian server
c270698 
Signed-off-by: Firas Ghanmi <fghanmi@redhat.com>
@fghanmi fghanmi force-pushed the TrillianTLSSupport branch from ec708d9 to c270698 Compare July 4, 2024 09:31
@mhutchinson
Copy link
Copy Markdown
Contributor

mhutchinson commented Jul 4, 2024

/gcbrun

@mhutchinson mhutchinson merged commit a549614 into google:master Jul 4, 2024
fghanmi added a commit to securesign/certificate-transparency-go that referenced this pull request Jul 15, 2024
@fghanmi
Add TLS support for Trillian server (google#1525)
7ccd159 
Signed-off-by: Firas Ghanmi <fghanmi@redhat.com>
@fghanmi fghanmi mentioned this pull request Jul 15, 2024
Merged
@fghanmi fghanmi mentioned this pull request Aug 8, 2024
Merged
2 tasks
@roger2hk roger2hk changed the title Add TLS support for Trillian server Add TLS support for migrillian Aug 8, 2024
@fghanmi fghanmi mentioned this pull request Aug 12, 2024
Merged
2 tasks
ompushkara pushed a commit to securesign/certificate-transparency-go that referenced this pull request Jan 14, 2026
@fghanmi
Add TLS support for Trillian server (google#1525)
c1fa5df 
Signed-off-by: Firas Ghanmi <fghanmi@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mhutchinson mhutchinson mhutchinson approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@fghanmi @mhutchinson @roger2hk