Skip to content

Find a new home for some functions #245

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 11, 2025
Merged

Find a new home for some functions #245

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
51ab92d
move entryFromChain to x509util
phbnf Apr 8, 2025
deee81b
move verifyAddChain
phbnf Apr 8, 2025
6392bf0
convert functions to methods
phbnf Apr 8, 2025
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
79 changes: 1 addition & 78 deletions internal/hammer/loadtest/workers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package loadtest

import (
"context"
"crypto/sha256"
"crypto/x509"
"encoding/base64"
"encoding/json"
Expand All @@ -32,7 +31,6 @@ import (
"github.com/transparency-dev/static-ct/internal/types/rfc6962"
"github.com/transparency-dev/static-ct/internal/x509util"
"github.com/transparency-dev/trillian-tessera/api/layout"
"github.com/transparency-dev/trillian-tessera/ctonly"
"k8s.io/klog/v2"
)

Expand Down Expand Up @@ -360,7 +358,7 @@ func (v *MMDVerifier) Run(ctx context.Context) {
if err != nil {
panic(fmt.Sprintf("Failed to parse certificates: %v", err))
}
entry, err := entryFromChain(certs, false, leafMMD.timestamp)
entry, err := x509util.EntryFromChain(certs, false, leafMMD.timestamp)
if err != nil {
panic(fmt.Sprintf("Failed to create entry from chain: %v", err))
}
Expand All @@ -380,78 +378,3 @@ func (v *MMDVerifier) Kill() {
v.cancel()
}
}

// entryFromChain generates an Entry from a chain and timestamp.
// Copied from certificate-transparency-go/serialization.go and internal/scti/handlers.go.
// TODO(phboneff): move in a different file maybe?
func entryFromChain(chain []*x509.Certificate, isPrecert bool, timestamp uint64) (*ctonly.Entry, error) {
leaf := ctonly.Entry{
IsPrecert: isPrecert,
Timestamp: timestamp,
}

if len(chain) > 1 {
issuersChain := make([][32]byte, len(chain)-1)
for i, c := range chain[1:] {
issuersChain[i] = sha256.Sum256(c.Raw)
}
leaf.FingerprintsChain = issuersChain
}

if !isPrecert {
leaf.Certificate = chain[0].Raw
return &leaf, nil
}

// Pre-certs are more complicated. First, parse the leaf pre-cert and its
// putative issuer.
if len(chain) < 2 {
return nil, fmt.Errorf("no issuer cert available for precert leaf building")
}
issuer := chain[1]
cert := chain[0]

var preIssuer *x509.Certificate
if isPreIssuer(issuer) {
// Replace the cert's issuance information with details from the pre-issuer.
preIssuer = issuer

// The issuer of the pre-cert is not going to be the issuer of the final
// cert. Change to use the final issuer's key hash.
if len(chain) < 3 {
return nil, fmt.Errorf("no issuer cert available for pre-issuer")
}
issuer = chain[2]
}

// Next, post-process the DER-encoded TBSCertificate, to remove the CT poison
// extension and possibly update the issuer field.
defangedTBS, err := x509util.BuildPrecertTBS(cert.RawTBSCertificate, preIssuer)
if err != nil {
return nil, fmt.Errorf("failed to remove poison extension: %v", err)
}

leaf.Precertificate = cert.Raw
// TODO(phboneff): do we need this?
// leaf.PrecertSigningCert = issuer.Raw
leaf.Certificate = defangedTBS

issuerKeyHash := sha256.Sum256(issuer.RawSubjectPublicKeyInfo)
leaf.IssuerKeyHash = issuerKeyHash[:]
return &leaf, nil
}

// isPreIssuer indicates whether a certificate is a pre-cert issuer with the specific
// certificate transparency extended key usage.
// Copied from certificate-transparency-go/serialization.go and internal/scti/handlers.go.
// TODO(phboneff): unify these.
func isPreIssuer(cert *x509.Certificate) bool {
// Look for the extension in the Extensions field and not ExtKeyUsage
// since crypto/x509 does not recognize this extension as an ExtKeyUsage.
for _, ext := range cert.Extensions {
if rfc6962.OIDExtKeyUsageCertificateTransparency.Equal(ext.Id) {
return true
}
}
return false
}
56 changes: 43 additions & 13 deletions internal/scti/chain_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func isPrecertificate(cert *x509.Certificate) (bool, error) {
// supplied in the chain. Then applies the RFC requirement that the path must involve all
// the submitted chain in the order of submission.
// TODO(phboneff): make this a method func([][]byte) ([]*x509.Certificate, error)
func validateChain(rawChain [][]byte, validationOpts ChainValidationOpts) ([]*x509.Certificate, error) {
func (opts ChainValidationOpts) validateChain(rawChain [][]byte) ([]*x509.Certificate, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It sounds weird to add the validation function to the ChainValidationOpts type which is supposed to store the options.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah sorry I had not seen this comment! Your comment is on point: it's actually the next pr in the chain: phbnf/tesseract@functionsmove...phbnf:static-ct:interfaceverifier. This next PR actually goes further and defines an interface. I did not send it in this PR to make reviewing easier: this one just moves methods, then I'll define interface and rename everything.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great. I'm looking forward to reviewing the next PR.

if len(rawChain) == 0 {
return nil, errors.New("empty certificate chain")
}
Expand All @@ -172,8 +172,8 @@ func validateChain(rawChain [][]byte, validationOpts ChainValidationOpts) ([]*x5
}
}

naStart := validationOpts.notAfterStart
naLimit := validationOpts.notAfterLimit
naStart := opts.notAfterStart
naLimit := opts.notAfterLimit
cert := chain[0]

// Check whether the expiry date of the cert is within the acceptable range.
Expand All @@ -184,24 +184,24 @@ func validateChain(rawChain [][]byte, validationOpts ChainValidationOpts) ([]*x5
return nil, fmt.Errorf("certificate NotAfter (%v) >= %v", cert.NotAfter, *naLimit)
}

now := validationOpts.currentTime
now := opts.currentTime
if now.IsZero() {
now = time.Now()
}
expired := now.After(cert.NotAfter)
if validationOpts.rejectExpired && expired {
if opts.rejectExpired && expired {
return nil, errors.New("rejecting expired certificate")
}
if validationOpts.rejectUnexpired && !expired {
if opts.rejectUnexpired && !expired {
return nil, errors.New("rejecting unexpired certificate")
}

// Check for unwanted extension types, if required.
// TODO(al): Refactor CertValidationOpts c'tor to a builder pattern and
// pre-calc this in there
if len(validationOpts.rejectExtIds) != 0 {
if len(opts.rejectExtIds) != 0 {
badIDs := make(map[string]bool)
for _, id := range validationOpts.rejectExtIds {
for _, id := range opts.rejectExtIds {
badIDs[id.String()] = true
}
for idx, ext := range cert.Extensions {
Expand All @@ -214,9 +214,9 @@ func validateChain(rawChain [][]byte, validationOpts ChainValidationOpts) ([]*x5

// TODO(al): Refactor CertValidationOpts c'tor to a builder pattern and
// pre-calc this in there too.
if len(validationOpts.extKeyUsages) > 0 {
if len(opts.extKeyUsages) > 0 {
acceptEKUs := make(map[x509.ExtKeyUsage]bool)
for _, eku := range validationOpts.extKeyUsages {
for _, eku := range opts.extKeyUsages {
acceptEKUs[eku] = true
}
good := false
Expand All @@ -227,7 +227,7 @@ func validateChain(rawChain [][]byte, validationOpts ChainValidationOpts) ([]*x5
}
}
if !good {
return nil, fmt.Errorf("rejecting certificate without EKU in %v", validationOpts.extKeyUsages)
return nil, fmt.Errorf("rejecting certificate without EKU in %v", opts.extKeyUsages)
}
}

Expand All @@ -237,9 +237,9 @@ func validateChain(rawChain [][]byte, validationOpts ChainValidationOpts) ([]*x5
// - allow certificate without policing them since this is not CT's responsibility
// See /internal/lax509/README.md for further information.
verifyOpts := lax509.VerifyOptions{
Roots: validationOpts.trustedRoots.CertPool(),
Roots: opts.trustedRoots.CertPool(),
Intermediates: intermediatePool.CertPool(),
KeyUsages: validationOpts.extKeyUsages,
KeyUsages: opts.extKeyUsages,
}

verifiedChains, err := lax509.Verify(cert, verifyOpts)
Expand All @@ -263,6 +263,36 @@ func validateChain(rawChain [][]byte, validationOpts ChainValidationOpts) ([]*x5
return nil, errors.New("no RFC compliant path to root found when trying to validate chain")
}

// verifyAddChain is used by add-chain and add-pre-chain. It does the checks that the supplied
// cert is of the correct type and chains to a trusted root.
// TODO(phbnf): add tests
func (opts ChainValidationOpts) verifyAddChain(req rfc6962.AddChainRequest, expectingPrecert bool) ([]*x509.Certificate, error) {
// We already checked that the chain is not empty so can move on to verification
validPath, err := opts.validateChain(req.Chain)
if err != nil {
// We rejected it because the cert failed checks or we could not find a path to a root etc.
// Lots of possible causes for errors
return nil, fmt.Errorf("chain failed to verify: %s", err)
}

isPrecert, err := isPrecertificate(validPath[0])
if err != nil {
return nil, fmt.Errorf("precert test failed: %s", err)
}

// The type of the leaf must match the one the handler expects
if isPrecert != expectingPrecert {
if expectingPrecert {
klog.Warningf("Cert (or precert with invalid CT ext) submitted as precert chain: %q", req.Chain)
} else {
klog.Warningf("Precert (or cert with invalid CT ext) submitted as cert chain: %q", req.Chain)
}
return nil, fmt.Errorf("cert / precert mismatch: %T", expectingPrecert)
}

return validPath, nil
}

func chainsEquivalent(inChain []*x509.Certificate, verifiedChain []*x509.Certificate) bool {
// The verified chain includes a root, but the input chain may or may not include a
// root (RFC 6962 s4.1/ s4.2 "the last [certificate] is either the root certificate
Expand Down
28 changes: 14 additions & 14 deletions internal/scti/chain_validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ func TestValidateChain(t *testing.T) {
if !fakeCARoots.AppendCertsFromPEM([]byte(testdata.RealPrecertIntermediatePEM)) {
t.Fatal("failed to load real intermediate")
}
validateOpts := ChainValidationOpts{
opts := ChainValidationOpts{
trustedRoots: fakeCARoots,
}

Expand Down Expand Up @@ -403,11 +403,11 @@ func TestValidateChain(t *testing.T) {
}
for _, test := range tests {
t.Run(test.desc, func(t *testing.T) {
validateOpts := validateOpts
opts := opts
if test.modifyOpts != nil {
test.modifyOpts(&validateOpts)
test.modifyOpts(&opts)
}
gotPath, err := validateChain(test.chain, validateOpts)
gotPath, err := opts.validateChain(test.chain)
if err != nil {
if !test.wantErr {
t.Errorf("ValidateChain()=%v,%v; want _,nil", gotPath, err)
Expand All @@ -433,7 +433,7 @@ func TestNotAfterRange(t *testing.T) {
if !fakeCARoots.AppendCertsFromPEM([]byte(testdata.FakeCACertPEM)) {
t.Fatal("failed to load fake root")
}
validateOpts := ChainValidationOpts{
opts := ChainValidationOpts{
trustedRoots: fakeCARoots,
rejectExpired: false,
}
Expand Down Expand Up @@ -473,12 +473,12 @@ func TestNotAfterRange(t *testing.T) {
for _, test := range tests {
t.Run(test.desc, func(t *testing.T) {
if !test.notAfterStart.IsZero() {
validateOpts.notAfterStart = &test.notAfterStart
opts.notAfterStart = &test.notAfterStart
}
if !test.notAfterLimit.IsZero() {
validateOpts.notAfterLimit = &test.notAfterLimit
opts.notAfterLimit = &test.notAfterLimit
}
gotPath, err := validateChain(test.chain, validateOpts)
gotPath, err := opts.validateChain(test.chain)
if err != nil {
if !test.wantErr {
t.Errorf("ValidateChain()=%v,%v; want _,nil", gotPath, err)
Expand All @@ -500,7 +500,7 @@ func TestRejectExpiredUnexpired(t *testing.T) {
}
// Validity period: May 13, 2016 - Jul 12, 2019.
chain := pemsToDERChain(t, []string{testdata.LeafSignedByFakeIntermediateCertPEM, testdata.FakeIntermediateCertPEM})
validateOpts := ChainValidationOpts{
opts := ChainValidationOpts{
trustedRoots: fakeCARoots,
extKeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
}
Expand Down Expand Up @@ -587,10 +587,10 @@ func TestRejectExpiredUnexpired(t *testing.T) {
},
} {
t.Run(tc.desc, func(t *testing.T) {
validateOpts.currentTime = tc.now
validateOpts.rejectExpired = tc.rejectExpired
validateOpts.rejectUnexpired = tc.rejectUnexpired
_, err := validateChain(chain, validateOpts)
opts.currentTime = tc.now
opts.rejectExpired = tc.rejectExpired
opts.rejectUnexpired = tc.rejectUnexpired
_, err := opts.validateChain(chain)
if err != nil {
if len(tc.wantErr) == 0 {
t.Errorf("ValidateChain()=_,%v; want _,nil", err)
Expand Down Expand Up @@ -692,7 +692,7 @@ func TestPreIssuedCert(t *testing.T) {
trustedRoots: roots,
extKeyUsages: tc.eku,
}
chain, err := validateChain(rawChain, opts)
chain, err := opts.validateChain(rawChain)
if err != nil {
t.Fatalf("failed to ValidateChain: %v", err)
}
Expand Down
Loading
Loading