Added certificates generate command by sajayantony · Pull Request #68 · notaryproject/notation

sajayantony · 2021-08-13T22:59:03Z

The command will generate an 2084 bits rsa based crt and key file.
The files will be placed in $HOME/.notary/keys

Signed-off-by: Sajay Antony sajaya@microsoft.com

Fixes #66

❯ ./bin/nv2 certificates generate wabbit-networks
Generating RSA Key with 3072 bits
Generated certificates expiring on 2022-08-25T12:22:00-07:00
Wrote self-signed certificate file: /home/sajay/.notation/keys/wabbit-networks.crt
Wrote private key file: /home/sajay/.notation/keys/wabbit-networks.key

The command will generate an 2084 bits rsa based crt and key file. The files will be placed in $HOME/.notary/keys Signed-off-by: Sajay Antony <sajaya@microsoft.com>

sajayantony · 2021-08-14T01:19:37Z

@lachie83 - This composes with my other PR that will make adding and removing the certs much cleaner - #70

- Default to 3047 for RSA Bits - Added --not-after flag for supporting expiry - Remove organization defaults - Remove TLS capabilities Signed-off-by: Sajay Antony <sajaya@microsoft.com>

cmd/nv2/gencert.go

shizhMSFT · 2021-08-25T08:09:08Z

cmd/nv2/gencert.go

+		}
+	}
+
+	return keysDir, nil


We need to ensure that keysDir is really a directory. It could be a regular file or symbolic link.

Updated to use IsDir. Is that good enough or do we need to ensure more checks. I believe you handled this in ORAS a well.

cmd/nv2/gencert.go

shizhMSFT · 2021-08-25T08:17:25Z

cmd/nv2/gencert.go

+	fmt.Printf("Generating certificates expiring on %s\n", notAfter.Format(time.RFC3339))
+	fmt.Printf("Writing public key file: %s\n", crtFilePath)


Should be Generated and Wrote.

BTW, it is not just a public key, but a self-signed cert.

cmd/nv2/gencert.go

Signed-off-by: Sajay Antony <sajaya@microsoft.com>

priteshbandi · 2021-09-02T04:32:16Z

cmd/nv2/gencert.go

+
+	// Set certificate validity
+	notBefore := time.Now()
+	notAfter := notBefore.Add(time.Duration(365 * 24 * time.Hour))


priteshbandi · 2021-09-02T05:20:03Z

cmd/nv2/gencert.go

+	hosts := strings.Split(host, ",")
+	for _, h := range hosts {
+		if ip := net.ParseIP(h); ip != nil {
+			template.IPAddresses = append(template.IPAddresses, ip)
+		} else {
+			template.DNSNames = append(template.DNSNames, h)
+		}
+	}


as this is codesigning not ssl certificate, we don't need to support DNSName or ip-address.

We still need them as the x509 package checks SAN instead of CN starting from golang 1.15. See https://golang.org/doc/go1.15#commonname

priteshbandi · 2021-09-02T05:21:26Z

cmd/nv2/gencert.go

+		Organization: []string{hosts[0]},
+		CommonName:   hosts[0],


Can we take Organization Name from user instead of host? host is confusing as certificate is not used for tls.

Organization is removed in the Notation CLI Alpha PR.

priteshbandi · 2021-09-02T05:22:40Z

cmd/nv2/gencert.go

+	keysDir, err := ensureKeysDir()
+	if err != nil {
+		return fmt.Errorf("Could not access keys directory: %v", err)
+	}


Should be done as first thing after validating arguments.

priteshbandi · 2021-09-02T05:28:56Z

cmd/nv2/gencert.go

+func ensureKeysDir() (string, error) {
+
+	// Expected to ensure ~/.notation/keys
+	dirname, err := os.UserHomeDir()


As discussed in #76 we should support XDG_Base_Directory.

Notation CLI Alpha honors XDG directories.

priteshbandi · 2021-09-02T05:45:12Z

cmd/nv2/gencert.go

+
+	certOut, err := os.OpenFile(crtFileName, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0600)
+	if err != nil {
+		return fmt.Errorf("Failed to open %s for writing: %v", crtFilePath, err)


s/open/create file?

priteshbandi · 2021-09-02T05:49:49Z

cmd/nv2/gencert.go

+			Value:    3072,
+		},
+		&cli.StringFlag{
+			Name:     "not-after",


Will it be a better customer experience to take validity period validForDays instead of actual date?
Pros:

easy to associate default value of 1 year

No need to worry about not-after being in past.

certificates generate --not-after 2006-01-02T15:04:05-07:00 certificates generate --valid-for-days 365

validForDays might not be a good one. For testing purposes, people may want short-lived certs like serveral hours or minutes.

sajayantony · 2021-09-03T16:32:10Z

@priteshbandi #83 PR adds all this capability into notation alpha.
I will close this draft PR and we should be now trying to get this all into one alpha release which we can iterate on is my thinking.

@shizhMSFT - can see if there are inputs here you might not have already considered in your PR?

Added generate-certificates command.

9b5179a

The command will generate an 2084 bits rsa based crt and key file. The files will be placed in $HOME/.notary/keys Signed-off-by: Sajay Antony <sajaya@microsoft.com>

sajayantony force-pushed the cert branch from 9537ec9 to 9b5179a Compare August 13, 2021 23:36

Fix PR feedback for test certificate generation

f12c23b

- Default to 3047 for RSA Bits - Added --not-after flag for supporting expiry - Remove organization defaults - Remove TLS capabilities Signed-off-by: Sajay Antony <sajaya@microsoft.com>

dlorenc reviewed Aug 17, 2021

View reviewed changes

cmd/nv2/gencert.go Outdated Show resolved Hide resolved

cmd/nv2/gencert.go Outdated Show resolved Hide resolved

trishankatdatadog reviewed Aug 17, 2021

View reviewed changes

cmd/nv2/gencert.go Outdated Show resolved Hide resolved

cmd/nv2/gencert.go Outdated Show resolved Hide resolved

cmd/nv2/gencert.go Outdated Show resolved Hide resolved

sajayantony force-pushed the cert branch from f3a8f1a to f12c23b Compare August 17, 2021 19:01

shizhMSFT self-requested a review August 25, 2021 06:04

shizhMSFT reviewed Aug 25, 2021

View reviewed changes

Moved to subcommand and other PR comments

e846844

Signed-off-by: Sajay Antony <sajaya@microsoft.com>

sajayantony force-pushed the cert branch from 40505df to e846844 Compare August 25, 2021 19:21

sajayantony changed the title ~~Added generate-certificates command.~~ Added certificates generate command Aug 25, 2021

sajayantony marked this pull request as draft August 31, 2021 07:15

SteveLasker added the cli Issue or PR released to Notation CLI label Sep 1, 2021

priteshbandi reviewed Sep 2, 2021

View reviewed changes

shizhMSFT mentioned this pull request Sep 3, 2021

Notation CLI baseline #83

Merged

sajayantony closed this Sep 14, 2021

		fmt.Printf("Generating certificates expiring on %s\n", notAfter.Format(time.RFC3339))
		fmt.Printf("Writing public key file: %s\n", crtFilePath)

Comments

Conversation

sajayantony commented Aug 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sajayantony commented Aug 14, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sajayantony commented Sep 3, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

sajayantony commented Aug 13, 2021 •

edited

Loading