Add function to return a public key from an x509 certificate #261

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
4 participants

Pekkari commented Jan 18, 2017

The public key will be ready to be written in a config file. This is valuable to
generate a certificateduring the initial configuration of juju client that will
become a default certificate in case of trying to use lxd https remotes.

cert/cert.go
@@ -292,3 +292,18 @@ func ParseCertAndKey(certPEM, keyPEM string) (*x509.Certificate, *rsa.PrivateKey
}
return cert, key, nil
}
+
+// Generate public key from keyPEM string
@perrito666

perrito666 Jan 18, 2017

Contributor

the expected form for go doc is:
// ComputePublicKey generates public key from keyPEM string

@Pekkari

Pekkari Jan 18, 2017

Sorry, I'm just a newbie in go, so I ignore this, I'll fix this.

cert/cert.go
+ if err != nil {
+ return "", err
+ }
+ pubKey := cert.PublicKey.(interface {})
@perrito666

perrito666 Jan 18, 2017

Contributor

sorry I dont fully understand the purpose of this typecasting.

@Pekkari

Pekkari Jan 18, 2017

That gets the public key as it's stored inside the certificate, and using the type that x509.MarshalPKIXPublicKey
expects in the input.

@perrito666

perrito666 Jan 18, 2017

Contributor

interface{} means that is a blank interface, absolutely anything will match this, you dont need a typecast, just pass whatever cert.PublicKey is and it will work.

cert/cert.go
+ return "", err
+ }
+ pubKey := cert.PublicKey.(interface {})
+ marshalledPubKey, _ := x509.MarshalPKIXPublicKey(pubKey)
@perrito666

perrito666 Jan 18, 2017

Contributor

what is the other ret value of MarshalPKIXPublicKey and why is it being ignored?

@Pekkari

Pekkari Jan 18, 2017

it is an error as usual, I didn't want to go so fine grained with several checks step after step, but if you need it, I certainly can add it.

cert/cert_test.go
@@ -219,6 +219,13 @@ func (certSuite) TestNewClientCertRSASize(c *gc.C) {
}
}
+func (certSuite) TestComputePublicKey(c *gc.C) {
+ computedPublicKey, err := cert.ComputePublicKey(caCertPEM)
+ if err == nil {
@perrito666

perrito666 Jan 18, 2017

Contributor

actually you want to c.Assert(err, gc.ErrorIsNil), if err is not nil this should fail too.

@Pekkari

Pekkari Jan 18, 2017

Good one, I'll do, thanks!

Add a funtion to return a public key from an x509 certificate ready to
be written in a config file. This is valuable to generate a certificate
during the initial configuration of juju client that will become a
default certificate in case of trying to use lxd https remotes.

Pekkari commented Jan 18, 2017

I'm heading to add another commit to this, as soon as I get some test case written, so don't be in a hurry to merge it.

cert/cert.go
+ for file, content := range map[string]string{ "/juju-cert.pem": pem, "/juju-cert.key": key,
+ "/juju-cert.pub": public } {
+ if _, err := os.Stat(location + file); os.IsNotExist(err) {
+ err := ioutil.WriteFile(location + file, []byte(content), 0600)
@axw

axw Jan 19, 2017

Member

please use utils.AtomicWriteFile, otherwise the Stat/WriteFile pattern will be prone to half-written files

Add funtion to store a certificate with default names in the given
location. This intends to store a default x509 in the configuration
folder of juju during initialixation of the cli.

@mjs mjs changed the title from Add a funtion to return a public key from an x509 certificate. to Add function to return a public key from an x509 certificate. Apr 6, 2017

@mjs mjs changed the title from Add function to return a public key from an x509 certificate. to Add function to return a public key from an x509 certificate Apr 6, 2017

Contributor

mjs commented Apr 6, 2017

@Pekkari: where are things at with this?

Pekkari commented Apr 6, 2017

Well, the juju pull request got stalled forever, if juju team doesn't like the idea of
generating a certificate on first run, there is no user for this code. Either we can wait
to them to finally reject the pull, or we can simple close it, as it's most likely not
going to proceed.

Thanks!

José.

Member

axw commented Apr 28, 2017

Seeing as there's no pressing need for this any more, I'm going to close it. We can resurrect it if needed.

@axw axw closed this Apr 28, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment