Skip to content

Commit 397a181

Browse files
dopeydopey
committedJan 28, 2020
Add backdate validation to sshCertValidityValidator.
1 parent 3d6a181 commit 397a181

File tree

7 files changed

+47
-23
lines changed

7 files changed

+47
-23
lines changed
 

Diff for: ‎authority/provisioner/sign_options.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ func (v *validityValidator) Valid(cert *x509.Certificate, o Options) error {
290290
// apply a backdate). This is good enough.
291291
if d > v.max+o.Backdate {
292292
return errors.Errorf("requested duration of %v is more than the authorized maximum certificate duration of %v",
293-
d, v.max)
293+
d, v.max+o.Backdate)
294294
}
295295
return nil
296296
}

Diff for: ‎authority/provisioner/sign_ssh_options.go

+8-13
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ type SSHCertOptionModifier interface {
3636
// SSHCertValidator is the interface used to validate an SSH certificate.
3737
type SSHCertValidator interface {
3838
SignOption
39-
Valid(cert *ssh.Certificate) error
39+
Valid(cert *ssh.Certificate, opts SSHOptions) error
4040
}
4141

4242
// SSHCertOptionsValidator is the interface used to validate the custom
@@ -310,7 +310,7 @@ type sshCertValidityValidator struct {
310310
*Claimer
311311
}
312312

313-
func (v *sshCertValidityValidator) Valid(cert *ssh.Certificate) error {
313+
func (v *sshCertValidityValidator) Valid(cert *ssh.Certificate, opts SSHOptions) error {
314314
switch {
315315
case cert.ValidAfter == 0:
316316
return errors.New("ssh certificate validAfter cannot be 0")
@@ -336,20 +336,15 @@ func (v *sshCertValidityValidator) Valid(cert *ssh.Certificate) error {
336336

337337
// To not take into account the backdate, time.Now() will be used to
338338
// calculate the duration if ValidAfter is in the past.
339-
var dur time.Duration
340-
if t := now().Unix(); t > int64(cert.ValidAfter) {
341-
dur = time.Duration(int64(cert.ValidBefore)-t) * time.Second
342-
} else {
343-
dur = time.Duration(cert.ValidBefore-cert.ValidAfter) * time.Second
344-
}
339+
dur := time.Duration(cert.ValidBefore-cert.ValidAfter) * time.Second
345340

346341
switch {
347342
case dur < min:
348343
return errors.Errorf("requested duration of %s is less than minimum "+
349344
"accepted duration for selected provisioner of %s", dur, min)
350-
case dur > max:
345+
case dur > max+opts.Backdate:
351346
return errors.Errorf("requested duration of %s is greater than maximum "+
352-
"accepted duration for selected provisioner of %s", dur, max)
347+
"accepted duration for selected provisioner of %s", dur, max+opts.Backdate)
353348
default:
354349
return nil
355350
}
@@ -360,7 +355,7 @@ func (v *sshCertValidityValidator) Valid(cert *ssh.Certificate) error {
360355
type sshCertDefaultValidator struct{}
361356

362357
// Valid returns an error if the given certificate does not contain the necessary fields.
363-
func (v *sshCertDefaultValidator) Valid(cert *ssh.Certificate) error {
358+
func (v *sshCertDefaultValidator) Valid(cert *ssh.Certificate, o SSHOptions) error {
364359
switch {
365360
case len(cert.Nonce) == 0:
366361
return errors.New("ssh certificate nonce cannot be empty")
@@ -395,7 +390,7 @@ func (v *sshCertDefaultValidator) Valid(cert *ssh.Certificate) error {
395390
type sshDefaultPublicKeyValidator struct{}
396391

397392
// Valid checks that certificate request common name matches the one configured.
398-
func (v sshDefaultPublicKeyValidator) Valid(cert *ssh.Certificate) error {
393+
func (v sshDefaultPublicKeyValidator) Valid(cert *ssh.Certificate, o SSHOptions) error {
399394
if cert.Key == nil {
400395
return errors.New("ssh certificate key cannot be nil")
401396
}
@@ -425,7 +420,7 @@ func (v sshDefaultPublicKeyValidator) Valid(cert *ssh.Certificate) error {
425420
type sshCertKeyIDValidator string
426421

427422
// Valid returns an error if the given certificate does not contain the necessary fields.
428-
func (v sshCertKeyIDValidator) Valid(cert *ssh.Certificate) error {
423+
func (v sshCertKeyIDValidator) Valid(cert *ssh.Certificate, o SSHOptions) error {
429424
if string(v) != cert.KeyId {
430425
return errors.Errorf("invalid ssh certificate KeyId; want %s, but got %s", string(v), cert.KeyId)
431426
}

Diff for: ‎authority/provisioner/sign_ssh_options_test.go

+32-3
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,7 @@ func Test_sshCertDefaultValidator_Valid(t *testing.T) {
659659
}
660660
for _, tt := range tests {
661661
t.Run(tt.name, func(t *testing.T) {
662-
if err := v.Valid(tt.cert); err != nil {
662+
if err := v.Valid(tt.cert, SSHOptions{}); err != nil {
663663
if assert.NotNil(t, tt.err) {
664664
assert.HasPrefix(t, err.Error(), tt.err.Error())
665665
}
@@ -678,26 +678,31 @@ func Test_sshCertValidityValidator(t *testing.T) {
678678
tests := []struct {
679679
name string
680680
cert *ssh.Certificate
681+
opts SSHOptions
681682
err error
682683
}{
683684
{
684685
"fail/validAfter-0",
685686
&ssh.Certificate{CertType: ssh.UserCert},
687+
SSHOptions{},
686688
errors.New("ssh certificate validAfter cannot be 0"),
687689
},
688690
{
689691
"fail/validBefore-in-past",
690692
&ssh.Certificate{CertType: ssh.UserCert, ValidAfter: uint64(now().Unix()), ValidBefore: uint64(now().Add(-time.Minute).Unix())},
693+
SSHOptions{},
691694
errors.New("ssh certificate validBefore cannot be in the past"),
692695
},
693696
{
694697
"fail/validBefore-before-validAfter",
695698
&ssh.Certificate{CertType: ssh.UserCert, ValidAfter: uint64(now().Add(5 * time.Minute).Unix()), ValidBefore: uint64(now().Add(3 * time.Minute).Unix())},
699+
SSHOptions{},
696700
errors.New("ssh certificate validBefore cannot be before validAfter"),
697701
},
698702
{
699703
"fail/cert-type-not-set",
700704
&ssh.Certificate{ValidAfter: uint64(now().Unix()), ValidBefore: uint64(now().Add(10 * time.Minute).Unix())},
705+
SSHOptions{},
701706
errors.New("ssh certificate type has not been set"),
702707
},
703708
{
@@ -707,6 +712,7 @@ func Test_sshCertValidityValidator(t *testing.T) {
707712
ValidAfter: uint64(now().Unix()),
708713
ValidBefore: uint64(now().Add(10 * time.Minute).Unix()),
709714
},
715+
SSHOptions{},
710716
errors.New("unknown ssh certificate type 3"),
711717
},
712718
{
@@ -716,16 +722,38 @@ func Test_sshCertValidityValidator(t *testing.T) {
716722
ValidAfter: uint64(n.Unix()),
717723
ValidBefore: uint64(n.Add(4 * time.Minute).Unix()),
718724
},
725+
SSHOptions{Backdate: time.Second},
719726
errors.New("requested duration of 4m0s is less than minimum accepted duration for selected provisioner of 5m0s"),
720727
},
728+
{
729+
"ok/duration-exactly-min",
730+
&ssh.Certificate{
731+
CertType: 1,
732+
ValidAfter: uint64(n.Unix()),
733+
ValidBefore: uint64(n.Add(5 * time.Minute).Unix()),
734+
},
735+
SSHOptions{Backdate: time.Second},
736+
nil,
737+
},
721738
{
722739
"fail/duration>max",
723740
&ssh.Certificate{
724741
CertType: 1,
725742
ValidAfter: uint64(n.Unix()),
726743
ValidBefore: uint64(n.Add(48 * time.Hour).Unix()),
727744
},
728-
errors.New("requested duration of 48h0m0s is greater than maximum accepted duration for selected provisioner of 24h0m0s"),
745+
SSHOptions{Backdate: time.Second},
746+
errors.New("requested duration of 48h0m0s is greater than maximum accepted duration for selected provisioner of 24h0m1s"),
747+
},
748+
{
749+
"ok/duration-exactly-max",
750+
&ssh.Certificate{
751+
CertType: 1,
752+
ValidAfter: uint64(n.Unix()),
753+
ValidBefore: uint64(n.Add(24*time.Hour + time.Second).Unix()),
754+
},
755+
SSHOptions{Backdate: time.Second},
756+
nil,
729757
},
730758
{
731759
"ok",
@@ -734,12 +762,13 @@ func Test_sshCertValidityValidator(t *testing.T) {
734762
ValidAfter: uint64(now().Unix()),
735763
ValidBefore: uint64(now().Add(8 * time.Hour).Unix()),
736764
},
765+
SSHOptions{Backdate: time.Second},
737766
nil,
738767
},
739768
}
740769
for _, tt := range tests {
741770
t.Run(tt.name, func(t *testing.T) {
742-
if err := v.Valid(tt.cert); err != nil {
771+
if err := v.Valid(tt.cert, tt.opts); err != nil {
743772
if assert.NotNil(t, tt.err) {
744773
assert.HasPrefix(t, err.Error(), tt.err.Error())
745774
}

Diff for: ‎authority/provisioner/ssh_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ func signSSHCertificate(key crypto.PublicKey, opts SSHOptions, signOpts []SignOp
116116

117117
// User provisioners validators
118118
for _, v := range validators {
119-
if err := v.Valid(cert); err != nil {
119+
if err := v.Valid(cert, opts); err != nil {
120120
return nil, err
121121
}
122122
}

Diff for: ‎authority/ssh.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ func (a *Authority) SignSSH(key ssh.PublicKey, opts provisioner.SSHOptions, sign
269269

270270
// User provisioners validators
271271
for _, v := range validators {
272-
if err := v.Valid(cert); err != nil {
272+
if err := v.Valid(cert, opts); err != nil {
273273
return nil, errs.Wrap(http.StatusForbidden, err, "signSSH")
274274
}
275275
}
@@ -428,9 +428,9 @@ func (a *Authority) RekeySSH(oldCert *ssh.Certificate, pub ssh.PublicKey, signOp
428428
}
429429
cert.Signature = sig
430430

431-
// Apply validators from provisioner..
431+
// Apply validators from provisioner.
432432
for _, v := range validators {
433-
if err := v.Valid(cert); err != nil {
433+
if err := v.Valid(cert, provisioner.SSHOptions{Backdate: backdate}); err != nil {
434434
return nil, errs.Wrap(http.StatusForbidden, err, "rekeySSH")
435435
}
436436
}

Diff for: ‎authority/ssh_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func (m sshTestCertModifier) Modify(cert *ssh.Certificate) error {
6262

6363
type sshTestCertValidator string
6464

65-
func (v sshTestCertValidator) Valid(crt *ssh.Certificate) error {
65+
func (v sshTestCertValidator) Valid(crt *ssh.Certificate, opts provisioner.SSHOptions) error {
6666
if v == "" {
6767
return nil
6868
}

Diff for: ‎authority/tls_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ func TestAuthority_Sign(t *testing.T) {
178178
csr: csr,
179179
extraOpts: extraOpts,
180180
signOpts: _signOpts,
181-
err: errors.New("authority.Sign: requested duration of 25h0m0s is more than the authorized maximum certificate duration of 24h0m0s"),
181+
err: errors.New("authority.Sign: requested duration of 25h0m0s is more than the authorized maximum certificate duration of 24h1m0s"),
182182
code: http.StatusUnauthorized,
183183
}
184184
},

0 commit comments

Comments
 (0)
Please sign in to comment.