-
Notifications
You must be signed in to change notification settings - Fork 92
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
Appliance shared TLS certificate #1272
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm. Is it possible to use this cert for admiral and harbor too?
yes i will definitely be making sure everything on the appliance uses this. i'm waiting until your systemd refactor merges first :-) |
9c57eaf
to
e5dd891
Compare
make sure this survives rebase #1381 |
83ca04e
to
e2d0996
Compare
@morris-jason ready for another review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with some comments.
@@ -5,6 +5,8 @@ Documentation=https://github.com/vmware/vic-product | |||
[Service] | |||
Type=oneshot | |||
ExecStart=/etc/vmware/vic-appliance-environment.sh | |||
Requires=ovf-network.service |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably want network-online.target here too.
|
||
func getTLSCertFingerprint() string { | ||
certFile := "/storage/data/certs/server.crt" | ||
if _, err := os.Stat(certFile); err == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be simplified to:
func getTLSCertFingerprint() string {
certFile := "/storage/data/certs/server.crt"
certPEM, e := ioutil.ReadFile(certFile)
if e != nil {
log.Debugf("Read error: %s", e.Error())
return certNotAvail
}
block, _ := pem.Decode([]byte(certPEM))
if block == nil {
log.Debugf("Parse error")
return certNotAvail
}
cert, err := x509.ParseCertificate(block.Bytes)
if err != nil {
log.Debugf("Parse error: %s", err.Error())
return certNotAvail
}
return fmt.Sprintf("% X", sha1.Sum(cert.Raw))
if err != nil { | ||
return fmt.Sprintf("Parse error: %s", err.Error()) | ||
} | ||
return fmt.Sprintf("% X", sha1.Sum(cert.Raw)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be "%X" instead of "% X"? Does the extra space break this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
space adds a space after each octet for readability
2c68ab0
to
f72e592
Compare
- Shares the same TLS certificate for all services running on the appliance - Places the certificate in a well known location - Simplify manifest for files included in appliance - Displays cert fingerprint to DCUI - Automatically convert private key to correct format - Update appliance-support.sh and SUPPORT.md - Move hostname detection for cert generation to vic-appliance-environment
824cad4
to
0b26eee
Compare
Fixes #881
Fixes #1007
Towards #1179
tested with
generated self signed DHCP
generated self signed static
custom cert DHCP
custom cert static