diff --git a/cmd/experimental/migrate/gcp/main.go b/cmd/experimental/migrate/gcp/main.go index ae03a9e9a..6106f4c52 100644 --- a/cmd/experimental/migrate/gcp/main.go +++ b/cmd/experimental/migrate/gcp/main.go @@ -81,6 +81,10 @@ func main() { // TODO(AlCutter): We should be properly verifying and opening the checkpoint here with the source log's // public key. bits := strings.Split(string(sourceCP), "\n") + if len(bits) < 3 { + slog.ErrorContext(ctx, "malformed source checkpoint", slog.String("checkpoint", string(sourceCP))) + os.Exit(1) + } sourceSize, err := strconv.ParseUint(bits[1], 10, 64) if err != nil { slog.ErrorContext(ctx, "invalid CP size", slog.Any("arg", bits[1]), slog.Any("error", err)) diff --git a/cmd/experimental/migrate/posix/main.go b/cmd/experimental/migrate/posix/main.go index 68147d187..3d1ea76f3 100644 --- a/cmd/experimental/migrate/posix/main.go +++ b/cmd/experimental/migrate/posix/main.go @@ -90,6 +90,10 @@ func main() { // TODO(AlCutter): We should be properly verifying and opening the checkpoint here with the source log's // public key. bits := strings.Split(string(sourceCP), "\n") + if len(bits) < 3 { + slog.ErrorContext(ctx, "malformed source checkpoint", slog.String("checkpoint", string(sourceCP))) + os.Exit(1) + } sourceSize, err := strconv.ParseUint(bits[1], 10, 64) if err != nil { slog.ErrorContext(ctx, "invalid CP size", slog.Any("arg", bits[1]), slog.Any("error", err)) diff --git a/cmd/fsck/main.go b/cmd/fsck/main.go index a30fd19db..f24fc9f32 100644 --- a/cmd/fsck/main.go +++ b/cmd/fsck/main.go @@ -181,16 +181,20 @@ func (l *logStateCollector) checkIssuersTask(ctx context.Context, readIssuer fun } // addIssuers adds the issuers in the provided byte string to the set of issuer to be checked. -func (l *logStateCollector) addIssuers(fpRaw cryptobyte.String) { +func (l *logStateCollector) addIssuers(fpRaw cryptobyte.String) error { + if len(fpRaw)%sha256.Size != 0 { + return fmt.Errorf("chain fingerprints are %d bytes, want a multiple of %d", len(fpRaw), sha256.Size) + } var fp []byte for len(fpRaw) > 0 { - fp, fpRaw = fpRaw[:32], fpRaw[32:] + fp, fpRaw = fpRaw[:sha256.Size], fpRaw[sha256.Size:] _, existed := l.issuersSeen.LoadOrStore(string(fp), true) if !existed { logger.DebugExtraContext(context.Background(), "Found issuer", slog.String("fp", fmt.Sprintf("%x", fp))) l.issuersToCheck <- fp } } + return nil } // merkleLeafHasher returns a function which knows how to: @@ -250,7 +254,9 @@ func (l *logStateCollector) merkleLeafHasher() func(bundle []byte) ([][]byte, er if !b.ReadUint16LengthPrefixed(&fpRaw) { return nil, fmt.Errorf("failed to read chain fingerprints at entry index %d of bundle", i) } - l.addIssuers(fpRaw) + if err := l.addIssuers(fpRaw); err != nil { + return nil, fmt.Errorf("invalid chain fingerprints at entry index %d of bundle: %v", i, err) + } h := rfc6962.DefaultHasher.HashLeaf(preimage.BytesOrPanic()) r = append(r, h) diff --git a/cmd/tesseract/posix/main.go b/cmd/tesseract/posix/main.go index e0b0ad718..a14fdf2bd 100644 --- a/cmd/tesseract/posix/main.go +++ b/cmd/tesseract/posix/main.go @@ -261,12 +261,12 @@ func newStorage(ctx context.Context, signer note.Signer) (st *storage.CTStorage, } antispamCacheSize, unit, error := humanize.ParseSI(*inMemoryAntispamCacheSize) - if unit != "" { - return nil, fmt.Errorf("invalid antispam cache size, used unit %q, want none", unit) - } if error != nil { return nil, fmt.Errorf("invalid antispam cache size: %v", error) } + if unit != "" { + return nil, fmt.Errorf("invalid antispam cache size, used unit %q, want none", unit) + } var extraSigners []note.Signer for _, as := range additionalSigners { @@ -377,8 +377,8 @@ func signerFromFlags() crypto.Signer { os.Exit(1) } block, _ := pem.Decode(r) - if err != nil { - slog.ErrorContext(context.Background(), "Failed to parse PEM private key", slog.Any("error", err)) + if block == nil { + slog.ErrorContext(context.Background(), "Failed to parse PEM private key", slog.String("path", kf)) os.Exit(1) } k, err := x509.ParseECPrivateKey(block.Bytes) diff --git a/internal/client/fetcher.go b/internal/client/fetcher.go index 0955b8914..68a565d89 100644 --- a/internal/client/fetcher.go +++ b/internal/client/fetcher.go @@ -75,7 +75,7 @@ func (h *HTTPFetcher) SetUserAgent(ua string) { // EnableRetries causes requests which result in a non-permanent error to be retried with up to maxRetries attempts. func (h *HTTPFetcher) EnableRetries(maxRetries uint) { - h.backOff = []backoff.RetryOption{backoff.WithBackOff(backoff.NewExponentialBackOff()), backoff.WithMaxTries(10)} + h.backOff = []backoff.RetryOption{backoff.WithBackOff(backoff.NewExponentialBackOff()), backoff.WithMaxTries(maxRetries)} } func (h HTTPFetcher) fetch(ctx context.Context, p string) ([]byte, error) { @@ -171,7 +171,8 @@ func (f FileFetcher) ReadEntryBundle(ctx context.Context, i uint64, p uint8) ([] return PartialOrFullResource(ctx, p, func(ctx context.Context, p uint8) (r []byte, rErr error) { data, err := os.ReadFile(path.Join(f.Root, ctEntriesPath(i, p))) if err != nil { - return nil, fmt.Errorf("failed to read file: %v", err) + // Must wrap so that callers (e.g. PartialOrFullResource) can detect os.ErrNotExist. + return nil, fmt.Errorf("failed to read file: %w", err) } if f.DecompressBundles { reader, err := gzip.NewReader(bytes.NewReader(data)) diff --git a/internal/ct/handlers.go b/internal/ct/handlers.go index e8ce8fcdd..d71c082bf 100644 --- a/internal/ct/handlers.go +++ b/internal/ct/handlers.go @@ -107,10 +107,9 @@ func setupMetrics() { metric.WithDescription("CT HTTP responses"), metric.WithUnit("{response}"))) - // TODO(phboneff): switch back to s, in Tessera as well. reqDuration = mustCreate(meter.Float64Histogram("tesseract.http.request.duration", metric.WithDescription("CT HTTP response duration"), - metric.WithUnit("ms"), + metric.WithUnit("s"), metric.WithExplicitBucketBoundaries(otel.SubSecondLatencyHistogramBuckets...))) notBeforeAgeUnverified = mustCreate(meter.Float64Histogram("tesseract.notbefore.age.unverified", diff --git a/internal/ct/signatures.go b/internal/ct/signatures.go index ec787cf5e..0b846f786 100644 --- a/internal/ct/signatures.go +++ b/internal/ct/signatures.go @@ -151,10 +151,10 @@ func (cts *cpSigner) Sign(msg []byte) ([]byte, error) { ckpt := &tfl.Checkpoint{} rest, err := ckpt.Unmarshal(msg) - if len(rest) != 0 { - return nil, fmt.Errorf("checkpoint contains trailing data: %s", string(rest)) - } else if err != nil { + if err != nil { return nil, fmt.Errorf("ckpt.Unmarshal: %v", err) + } else if len(rest) != 0 { + return nil, fmt.Errorf("checkpoint contains trailing data: %s", string(rest)) } else if ckpt.Origin != cts.origin { return nil, fmt.Errorf("checkpoint's origin %s doesn't match signer's origin %s", ckpt.Origin, cts.origin) } diff --git a/internal/lax509/verify.go b/internal/lax509/verify.go index f0b24da21..aecb3e13d 100644 --- a/internal/lax509/verify.go +++ b/internal/lax509/verify.go @@ -201,18 +201,18 @@ func Verify(c *x509.Certificate, opts VerifyOptions) (chains [][]*x509.Certifica opts.KeyUsages = []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth} } - if slices.Contains(opts.KeyUsages, x509.ExtKeyUsageAny) { - // If any key usage is acceptable, no need to check the chain for - // key usages. - return candidateChains, nil - } - if len(candidateChains) == 0 { var details []string err = x509.CertificateInvalidError{Cert: c, Reason: x509.NoValidChains, Detail: strings.Join(details, ", ")} return nil, err } + if slices.Contains(opts.KeyUsages, x509.ExtKeyUsageAny) { + // If any key usage is acceptable, no need to check the chain for + // key usages. + return candidateChains, nil + } + return candidateChains, nil } diff --git a/internal/types/tls/tls.go b/internal/types/tls/tls.go index 15c50817a..02f8c3116 100644 --- a/internal/types/tls/tls.go +++ b/internal/types/tls/tls.go @@ -340,7 +340,7 @@ func parseField(v reflect.Value, data []byte, initOffset int, info *fieldInfo) ( if len(rest) < 3 { return offset, syntaxError{info.fieldName(), "truncated uint24"} } - v.SetUint(uint64(data[0])<<16 | uint64(data[1])<<8 | uint64(data[2])) + v.SetUint(uint64(rest[0])<<16 | uint64(rest[1])<<8 | uint64(rest[2])) offset += 3 return offset, nil case uint32Type: diff --git a/storage/storage.go b/storage/storage.go index ca46abb31..34ca09a4d 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -206,11 +206,12 @@ func cachedStoreIssuers(s IssuerStorage) func(context.Context, []KV) error { return fmt.Errorf("issuerStorage.AddIfNotExist(): error storing issuer data in the underlying IssuerStorage: %v", err) } for _, kv := range req { + mu.Lock() if len(m) >= maxCachedIssuerKeys { + mu.Unlock() logger.DebugExtraContext(ctx, "cachedStoreIssuers wrapper: local issuer cache full, will stop caching issuers.") return nil } - mu.Lock() m[string(kv.K)] = struct{}{} mu.Unlock() }