Bugfix hunt

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))

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))

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)

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)

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))

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",

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)
 	}

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
 }
 

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:

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()
 		}