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

Adds expiry parser with ability to specify hours, days, months and years #47

Merged
merged 2 commits into from
Sep 20, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions cmd/expiry.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package cmd

import (
"errors"
"regexp"
"strconv"
"time"
)

var nowFunc = time.Now

func parseExpiry(fromNow string) (time.Time, error) {
re := regexp.MustCompile("\\s*(\\d+)\\s*(day|month|year|hour)s?")
matches := re.FindAllStringSubmatch(fromNow, -1)
addDate := map[string]int{
"day": 0,
"month": 0,
"year": 0,
"hour": 0,
}
for _, r := range matches {
addDate[r[2]], _ = strconv.Atoi(r[1])
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not check the error here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

r[1] should have been matched from the first group in the regex which is \d+ so we can say with fair certainty that it's going to be a contiguous string of integers.

Thinking about it though Atoi could still fail with an integer overflow (on e.g. "999999999999 days"). Should also probably check for signed int overflow because Atoi would not error in that case but the result is not valid in this context.

I will add those checks.

}

now := nowFunc().UTC()
result := now.
AddDate(addDate["year"], addDate["month"], addDate["day"]).
Add(time.Hour * time.Duration(addDate["hour"]))
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit, but I would probably reverse this to make it more readable (i.e. put time.Hour at the end of the multiplication).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing


if now == result {
return now, errors.New("Invalid expiry format")
}

return result, nil
}
88 changes: 88 additions & 0 deletions cmd/expiry_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package cmd

import (
"fmt"
"testing"
"time"
)

const dateFormat = "2006-01-02"

func init() {
nowFunc = func() time.Time {
t, _ := time.Parse(dateFormat, "2017-01-01")
return t
}
}

func TestParseExpiryWithDays(t *testing.T) {
t1, _ := parseExpiry("1 day")
t2, _ := parseExpiry("1 days")
expected, _ := time.Parse(dateFormat, "2017-01-02")

if t1 != expected {
t.Fatalf("Parsing expiry 1 day from now (singular) did not return expected value (wanted: %s, got: %s)", expected, t1)
}

if t2 != expected {
t.Fatalf("Parsing expiry 1 day from now (plural) did not return expected value (wanted: %s, got: %s)", expected, t2)
}
}

func TestParseExpiryWithMonths(t *testing.T) {
t1, _ := parseExpiry("1 month")
t2, _ := parseExpiry("1 months")
expected, _ := time.Parse(dateFormat, "2017-02-01")

if t1 != expected {
t.Fatalf("Parsing expiry 1 month from now (singular) did not return expected value (wanted: %s, got: %s)", expected, t1)
}

if t2 != expected {
t.Fatalf("Parsing expiry 1 month from now (plural) did not return expected value (wanted: %s, got: %s)", expected, t2)
}
}

func TestParseExpiryWithYears(t *testing.T) {
t1, _ := parseExpiry("1 year")
t2, _ := parseExpiry("1 years")
expected, _ := time.Parse(dateFormat, "2018-01-01")

if t1 != expected {
t.Fatalf("Parsing expiry 1 year from now (singular) did not return expected value (wanted: %s, got: %s)", expected, t1)
}

if t2 != expected {
t.Fatalf("Parsing expiry 1 year from now (plural) did not return expected value (wanted: %s, got: %s)", expected, t2)
}
}

func TestParseExpiryWithMixed(t *testing.T) {
t1, _ := parseExpiry("2 days 3 months 1 year")
t2, _ := parseExpiry("5 years 5 days 6 months")
expectedt1, _ := time.Parse(dateFormat, "2018-04-03")
expectedt2, _ := time.Parse(dateFormat, "2022-07-06")

if t1 != expectedt1 {
t.Fatalf("Parsing expiry for mixed format t1 did not return expected value (wanted: %s, got: %s)", expectedt1, t1)
}

if t2 != expectedt2 {
t.Fatalf("Parsing expiry for mixed format t2 did not return expected value (wanted: %s, got: %s)", expectedt2, t2)
}
}

func TestParseInvalidExpiry(t *testing.T) {
t1, err1 := parseExpiry("53257284647843897")
t2, err2 := parseExpiry("5 y")
expectedt1, _ := time.Parse(dateFormat, "2017-01-01")
expectedt2, _ := time.Parse(dateFormat, "2017-01-01")

if t1 != expectedt1 && err1 != nil && fmt.Sprintf("%s", err1) == "Invalid expiry format" {
t.Fatalf("Parsing invalid expiry t1 did not produce an error as expected")
}

if t2 != expectedt2 && err2 != nil && fmt.Sprintf("%s", err2) == "Invalid expiry format" {
t.Fatalf("Parsing invalid expiry t2 did not produce an error as expected")
}
}
20 changes: 17 additions & 3 deletions cmd/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ func NewInitCommand() cli.Command {
Flags: []cli.Flag{
cli.StringFlag{"passphrase", "", "Passphrase to encrypt private-key PEM block", ""},
cli.IntFlag{"key-bits", 4096, "Bit size of RSA keypair to generate", ""},
cli.IntFlag{"years", 10, "How long until the CA certificate expires", ""},
cli.IntFlag{"years", 0, "DEPRECATED; Use --expires instead", ""},
cli.StringFlag{"expires", "10 years", "How long until the certificate expires. Example: 1 year 2 days 3 months 4 hours", ""},
Copy link
Contributor

Choose a reason for hiding this comment

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

We could change this to 18 months as a default while we're at it. @mcpherrinm what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe default to 1 year?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Happy to change to whatever you guys want; I was maintaining existing behaviour as far as possible.

cli.StringFlag{"organization, o", "", "CA Certificate organization", ""},
cli.StringFlag{"organizational-unit, ou", "", "CA Certificate organizational unit", ""},
cli.StringFlag{"country, c", "", "CA Certificate country", ""},
Expand Down Expand Up @@ -99,7 +100,20 @@ func initAction(c *cli.Context) {
}
}

crt, err := pkix.CreateCertificateAuthority(key, c.String("organizational-unit"), c.Int("years"), c.String("organization"), c.String("country"), c.String("province"), c.String("locality"), c.String("common-name"))
expires := c.String("expires")
if years := c.Int("years"); years != 0 {
expires = fmt.Sprintf("%s %s years", expires, years)
}

// Expiry parsing is a naive regex implementation
// Token based parsing would provide better feedback but
expiresTime, err := parseExpiry(expires)
if err != nil {
fmt.Fprintln(os.Stderr, "Invalid expiry format")
os.Exit(1)
}

crt, err := pkix.CreateCertificateAuthority(key, c.String("organizational-unit"), expiresTime, c.String("organization"), c.String("country"), c.String("province"), c.String("locality"), c.String("common-name"))
if err != nil {
fmt.Fprintln(os.Stderr, "Create certificate error:", err)
os.Exit(1)
Expand Down Expand Up @@ -130,7 +144,7 @@ func initAction(c *cli.Context) {
}

// Create an empty CRL, this is useful for Java apps which mandate a CRL.
crl, err := pkix.CreateCertificateRevocationList(key, crt, c.Int("years"))
crl, err := pkix.CreateCertificateRevocationList(key, crt, expiresTime)
if err != nil {
fmt.Fprintln(os.Stderr, "Create CRL error:", err)
os.Exit(1)
Expand Down
21 changes: 18 additions & 3 deletions cmd/sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ func NewSignCommand() cli.Command {
Description: "Sign certificate request with CA, and generate certificate for the host.",
Flags: []cli.Flag{
cli.StringFlag{"passphrase", "", "Passphrase to decrypt private-key PEM block of CA", ""},
cli.IntFlag{"years", 2, "How long until the certificate expires", ""},
cli.IntFlag{"years", 0, "DEPRECATED; Use --expires instead", ""},
cli.StringFlag{"expires", "2 years", "How long until the certificate expires. Example: 1 year 2 days 3 months 4 hours", ""},
cli.StringFlag{"CA", "", "CA to sign cert", ""},
cli.BoolFlag{"stdout", "Print certificate to stdout in addition to saving file", ""},
cli.BoolFlag{"intermediate", "Generated certificate should be a intermediate", ""},
Expand All @@ -49,6 +50,7 @@ func newSignAction(c *cli.Context) {
fmt.Fprintln(os.Stderr, "One host name must be provided.")
os.Exit(1)
}

formattedReqName := strings.Replace(c.Args()[0], " ", "_", -1)
formattedCAName := strings.Replace(c.String("CA"), " ", "_", -1)

Expand Down Expand Up @@ -90,12 +92,25 @@ func newSignAction(c *cli.Context) {
}
}

expires := c.String("expires")
if years := c.Int("years"); years != 0 {
expires = fmt.Sprintf("%s %s years", expires, years)
}

// Expiry parsing is a naive regex implementation
// Token based parsing would provide better feedback but
expiresTime, err := parseExpiry(expires)
if err != nil {
fmt.Fprintln(os.Stderr, "Invalid expiry format")
os.Exit(1)
}

var crtOut *pkix.Certificate
if c.Bool("intermediate") {
fmt.Fprintf(os.Stderr, "Building intermediate")
crtOut, err = pkix.CreateIntermediateCertificateAuthority(crt, key, csr, c.Int("years"))
crtOut, err = pkix.CreateIntermediateCertificateAuthority(crt, key, csr, expiresTime)
} else {
crtOut, err = pkix.CreateCertificateHost(crt, key, csr, c.Int("years"))
crtOut, err = pkix.CreateCertificateHost(crt, key, csr, expiresTime)
}

if err != nil {
Expand Down
7 changes: 3 additions & 4 deletions pkix/cert_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,13 @@ var (

// CreateCertificateAuthority creates Certificate Authority using existing key.
// CertificateAuthorityInfo returned is the extra infomation required by Certificate Authority.
func CreateCertificateAuthority(key *Key, organizationalUnit string, years int, organization string, country string, province string, locality string, commonName string) (*Certificate, error) {
func CreateCertificateAuthority(key *Key, organizationalUnit string, expiry time.Time, organization string, country string, province string, locality string, commonName string) (*Certificate, error) {
subjectKeyID, err := GenerateSubjectKeyID(key.Public)
if err != nil {
return nil, err
}
authTemplate.SubjectKeyId = subjectKeyID
authTemplate.NotAfter = time.Now().AddDate(years, 0, 0).UTC()
authTemplate.NotAfter = expiry
if len(country) > 0 {
authTemplate.Subject.Country = []string{country}
}
Expand Down Expand Up @@ -113,7 +113,7 @@ func CreateCertificateAuthority(key *Key, organizationalUnit string, years int,

// CreateIntermediateCertificateAuthority creates an intermediate
// CA certificate signed by the given authority.
func CreateIntermediateCertificateAuthority(crtAuth *Certificate, keyAuth *Key, csr *CertificateSigningRequest, years int) (*Certificate, error) {
func CreateIntermediateCertificateAuthority(crtAuth *Certificate, keyAuth *Key, csr *CertificateSigningRequest, proposedExpiry time.Time) (*Certificate, error) {
serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128)
serialNumber, err := rand.Int(rand.Reader, serialNumberLimit)
if err != nil {
Expand All @@ -130,7 +130,6 @@ func CreateIntermediateCertificateAuthority(crtAuth *Certificate, keyAuth *Key,
authTemplate.RawSubject = rawCsr.RawSubject

caExpiry := time.Now().Add(crtAuth.GetExpirationDuration())
proposedExpiry := time.Now().AddDate(years, 0, 0).UTC()
// ensure cert doesn't expire after issuer
if caExpiry.Before(proposedExpiry) {
authTemplate.NotAfter = caExpiry
Expand Down
2 changes: 1 addition & 1 deletion pkix/cert_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func TestCreateCertificateAuthority(t *testing.T) {
t.Fatal("Failed creating rsa key:", err)
}

crt, err := CreateCertificateAuthority(key, "OU", 5, "test", "US", "California", "San Francisco", "CA Name")
crt, err := CreateCertificateAuthority(key, "OU", time.Now().AddDate(5, 0, 0), "test", "US", "California", "San Francisco", "CA Name")
if err != nil {
t.Fatal("Failed creating certificate authority:", err)
}
Expand Down
3 changes: 1 addition & 2 deletions pkix/cert_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ var (

// CreateCertificateHost creates certificate for host.
// The arguments include CA certificate, CA key, certificate request.
func CreateCertificateHost(crtAuth *Certificate, keyAuth *Key, csr *CertificateSigningRequest, years int) (*Certificate, error) {
func CreateCertificateHost(crtAuth *Certificate, keyAuth *Key, csr *CertificateSigningRequest, proposedExpiry time.Time) (*Certificate, error) {
serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128)
serialNumber, err := rand.Int(rand.Reader, serialNumberLimit)
if err != nil {
Expand All @@ -81,7 +81,6 @@ func CreateCertificateHost(crtAuth *Certificate, keyAuth *Key, csr *CertificateS
hostTemplate.RawSubject = rawCsr.RawSubject

caExpiry := time.Now().Add(crtAuth.GetExpirationDuration())
proposedExpiry := time.Now().AddDate(years, 0, 0).UTC()
// ensure cert doesn't expire after issuer
if caExpiry.Before(proposedExpiry) {
hostTemplate.NotAfter = caExpiry
Expand Down
3 changes: 2 additions & 1 deletion pkix/cert_host_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package pkix
import (
"bytes"
"testing"
"time"
)

func TestCreateCertificateHost(t *testing.T) {
Expand All @@ -38,7 +39,7 @@ func TestCreateCertificateHost(t *testing.T) {
t.Fatal("Failed parsing certificate request from PEM:", err)
}

crt, err := CreateCertificateHost(crtAuth, key, csr, 5000)
crt, err := CreateCertificateHost(crtAuth, key, csr, time.Now().AddDate(5000, 0, 0))
if err != nil {
t.Fatal("Failed creating certificate for host:", err)
}
Expand Down
4 changes: 2 additions & 2 deletions pkix/crl.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,13 @@ const (
crlPEMBlockType = "X509 CRL"
)

func CreateCertificateRevocationList(key *Key, ca *Certificate, years int) (*CertificateRevocationList, error) {
func CreateCertificateRevocationList(key *Key, ca *Certificate, expiry time.Time) (*CertificateRevocationList, error) {
rawCrt, err := ca.GetRawCertificate()
if err != nil {
return nil, err
}

crlBytes, err := rawCrt.CreateCRL(rand.Reader, key.Private, []pkix.RevokedCertificate{}, time.Now(), time.Now().AddDate(years, 0, 0))
crlBytes, err := rawCrt.CreateCRL(rand.Reader, key.Private, []pkix.RevokedCertificate{}, time.Now(), expiry)
if err != nil {
return nil, err
}
Expand Down
5 changes: 3 additions & 2 deletions pkix/crl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package pkix
import (
"bytes"
"testing"
"time"
)

const (
Expand Down Expand Up @@ -48,11 +49,11 @@ func TestCreateCertificateRevocationList(t *testing.T) {
t.Fatal("Failed creating rsa key:", err)
}

crt, err := CreateCertificateAuthority(key, "OU", 5, "test", "US", "California", "San Francisco", "CA Name")
crt, err := CreateCertificateAuthority(key, "OU", time.Now().AddDate(5, 0, 0), "test", "US", "California", "San Francisco", "CA Name")
if err != nil {
t.Fatal("Failed creating certificate authority:", err)
}
_, err = CreateCertificateRevocationList(key, crt, 5)
_, err = CreateCertificateRevocationList(key, crt, time.Now().AddDate(5, 0, 0))
if err != nil {
t.Fatal("Failed creating crl:", err)
}
Expand Down
2 changes: 1 addition & 1 deletion test
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
source ./build

if [ -z "$PKG" ]; then
PKG="depot pkix tests"
PKG="cmd depot pkix tests"
fi

# Unit tests
Expand Down