feat: add cryptographic asset model and parser by knqyf263 · Pull Request #10970 · aquasecurity/trivy

knqyf263 · 2026-07-16T08:37:13Z

Description

Add the format-neutral cryptographic asset foundation for container image CBOM support.

This PR:

introduces format-neutral models for cryptographic assets, identities, occurrences, and relationships under pkg/crypto
carries cryptographic assets through fanal analysis results and image artifact types
adds case-insensitive file selection for common cryptographic file extensions
parses X.509 certificates, public keys, plaintext private keys, and encrypted PKCS#8 containers from PEM and DER input

The parser returns only public metadata for plaintext private keys and does not retain private-key material in its results. Encrypted PKCS#8 containers are treated as opaque assets and identified by their SHA-256 digest.

This PR does not add user-facing scanner behavior. Analyzer and extractor integration, scanner registration, and CycloneDX output will be implemented in follow-up PRs.

Related issues

Add cryptographic asset inventory for container images #10962

Related discussion

CBOM: design & implementation plan (crypto files) #10933

Checklist

I've read the guidelines for contributing to this repository.
I've followed the conventions in the PR title.
I've added tests that prove my fix is effective or that my feature works.
I've updated the documentation with the relevant information (if needed).
I've added usage information (if the PR introduces new options)
I've included a "before" and "after" example to the description (if the PR is a user interface change).

DmitriyLewen · 2026-07-16T13:04:15Z

+}
+
+// Asset describes a format-neutral cryptographic asset.
+type Asset struct {


Have you considered moving Asset and everything related to it into fanal/types?
All the fields in AnalysisResult are fields from ftypes:

trivy/pkg/fanal/analyzer/analyzer.go

Lines 180 to 186 in b7ce181

OS ftypes.OS

Repository *ftypes.Repository

PackageInfos []ftypes.PackageInfo

Applications []ftypes.Application

Misconfigurations []ftypes.Misconfiguration

Secrets []ftypes.Secret

Licenses []ftypes.LicenseFile

On top of that, we use Asset as an analysis result, which also fits this change.
The fields of Asset and the other structs are exported, so there shouldn't be any problems with this move.
I understand that the amount of code involved in this package is many times larger than for any type in fanal/types, but in that case we could think about creating a subpackage.
In addition, this would get rid of the circular import problem with layers.

But I might have missed something.

DmitriyLewen · 2026-07-16T13:06:57Z

+		{name: "lowercase PEM", filePath: "certificates/server.pem", want: true},
+		{name: "uppercase PEM", filePath: "certificates/server.PEM", want: true},
+		{name: "mixed-case PEM", filePath: "certificates/server.PeM", want: true},
+		{name: "lowercase DER", filePath: "certificates/server.der", want: true},
+		{name: "uppercase DER", filePath: "certificates/server.DER", want: true},
+		{name: "mixed-case DER", filePath: "certificates/server.DeR", want: true},
+		{name: "lowercase CRT", filePath: "certificates/server.crt", want: true},
+		{name: "uppercase CRT", filePath: "certificates/server.CRT", want: true},
+		{name: "mixed-case CRT", filePath: "certificates/server.CrT", want: true},
+		{name: "lowercase CER", filePath: "certificates/server.cer", want: true},
+		{name: "uppercase CER", filePath: "certificates/server.CER", want: true},
+		{name: "mixed-case CER", filePath: "certificates/server.CeR", want: true},
+		{name: "lowercase KEY", filePath: "certificates/server.key", want: true},
+		{name: "uppercase KEY", filePath: "certificates/server.KEY", want: true},
+		{name: "mixed-case KEY", filePath: "certificates/server.KeY", want: true},


We already have the tests for the Contains() function from set.NewCaseInsensitive.
So I think checking each ext once will be enough.

DmitriyLewen · 2026-07-16T13:34:09Z

 	})
+
+	// Cryptographic assets
+	sort.SliceStable(r.CryptoAssets, func(i, j int) bool {


Could you explain in more detail why sort.SliceStable is needed here?
If we have assets with the same FilePath and Descriptor and we don't remove them as duplicates, then it might make sense to add more values for comparison.
I don't think we'll have enough entries to worry about the speed difference between sort.Slice and sort.SliceStable, but from a code consistency perspective, we could go with sort.Slice.

DmitriyLewen · 2026-07-17T09:47:24Z

+	case "", KeyFormatPKCS1, KeyFormatPKCS8, KeyFormatSEC1, KeyFormatPKIX:
+	default:
+		return xerrors.Errorf("unknown key format %q", a.Key.Format)
+	}
+	switch a.Key.Encoding {
+	case "", EncodingPEM, EncodingDER:


Are empty Format and Encoding values actually valid?
If I haven't missed anything, all the functions in the parser return both values, so an empty value isn't an expected case, and I'd return an error here.

DmitriyLewen · 2026-07-17T10:34:19Z

+		}
+		recognized = true
+		if err != nil {
+			logParseError(ctx, filePath, block.Type, err)


nit:
I checked the local cert file.
This file contains 128 certificates:

➜ cat /etc/ssl/cert.pem | grep "BEGIN CERTIFICATE" | wc -l 128

If we assume there can be similar files with problems (with the new errors) for most of the certificates, log messages for each object can be noisy.
Perhaps we can show a single log for the file or, as we did in similar cases, a single info/warn log for the file and debug logs for each object.

DmitriyLewen · 2026-07-17T10:43:57Z

+	}
+
+	// No PEM block was decoded, so try the whole file as DER.
+	object, err := parseDERObject(content)


nit:
I don't really like the asymmetry we ended up with.
For PEM, everywhere in parsePEMObject we set cryptotypes.EncodingDER, but then we overwrite it in parsePEMBlock().
For DER, on the other hand, we take the already set value.

What if we make a symmetric function parseDERBlock() and change the encoding value there, removing it from the downstream functions?

DmitriyLewen · 2026-07-17T10:56:07Z

+		"EC PARAMETERS",
+		"DH PARAMETERS",


nit:

Suggested change

"EC PARAMETERS",

"DH PARAMETERS",

"EC PARAMETERS",

"DH PARAMETERS",

"DSA PARAMETERS",

DmitriyLewen · 2026-07-17T11:01:27Z

+
+// MarshalJSON validates and encodes the descriptor as its canonical string.
+func (d Descriptor) MarshalJSON() ([]byte, error) {
+	if err := d.Validate(); err != nil {


Validation on unmarshaling makes sense, but I'm not sure it's worth doing validation on marshaling.
It's better to validate when creating the descriptor.

DmitriyLewen · 2026-07-17T11:08:58Z

+
+// Asset describes a format-neutral cryptographic asset.
+type Asset struct {
+	descriptor *Descriptor


Maybe I don't see "the whole picture":
In this case we end up with duplication of the Kind, KeyType and Identity fields. One set is in the Asset itself, and another one is in the descriptor.
IIUC the main point is to fix the descriptor and its hash as an immutable object, but:

asset.Kind (and the other fields) can still be computed (e.g. after cloning), and then there will be a discrepancy;

we still build the string (Descriptor.String()) on every call, which means we spend resources on computing the string anyway.

Since I don't fully understand the motivation behind this decision, let's discuss whether we can get rid of this field duplication.

DmitriyLewen · 2026-07-17T11:10:43Z

+		left := r.CryptoAssets[i].Descriptor().String()
+		right := r.CryptoAssets[j].Descriptor().String()


We call Descriptor().String() for every element, and each time we build the strings (using url.QueryEscape).
Maybe we could store the computed value for the object?

Related to !!!link comment.

DmitriyLewen · 2026-07-17T13:14:12Z

+)
+
+// Certificate contains certificate-specific metadata.
+type Certificate struct {


Should we add encoding field (as for Key), because we save this field in Parser:
https://github.com/knqyf263/trivy/blob/fea60fea765eede5c73cda310541f2e1d44bd016/pkg/crypto/parser/x509/parser.go#L119-L128

DmitriyLewen · 2026-07-17T13:14:24Z

+	if a.Certificate.Format != CertificateFormatX509 {
+		return xerrors.Errorf("unknown certificate format %q", a.Certificate.Format)
+	}
+	if a.Certificate.MaxPathLen < 0 {


IIUC possible -1 value.
https://github.com/golang/go/blob/c122d7e6c50f8f1950a99b8e7b2b36e8a54cffce/src/crypto/x509/x509.go#L825-L832
So we should check this value or check MaxPathLen + MaxPathLenZero.

DmitriyLewen · 2026-07-17T13:16:22Z

+	Kind     Kind     `json:",omitempty"`
+	KeyType  KeyType  `json:",omitempty"`
+	Identity Identity `json:",omitzero"`
+	Name     string   `json:",omitempty"`


If I understand correctly, we'll use Name as component.name in CycloneDX.
But where will we build it? In the Analyzer?

knqyf263 added the kind/feature Categorizes issue or PR as related to a new feature. label Jul 16, 2026

knqyf263 self-assigned this Jul 16, 2026

knqyf263 force-pushed the feat/crypto-asset-foundation branch from f9f77cf to 0e856fc Compare July 16, 2026 08:47

knqyf263 added 3 commits July 16, 2026 12:58

feat: add neutral cryptographic asset model

877ef65

feat(image): carry cryptographic assets through fanal

b7ce181

feat(parser): parse cryptographic PEM and DER objects

fea60fe

knqyf263 force-pushed the feat/crypto-asset-foundation branch from 0e856fc to fea60fe Compare July 16, 2026 08:59

knqyf263 marked this pull request as ready for review July 16, 2026 10:24

knqyf263 requested a review from DmitriyLewen as a code owner July 16, 2026 10:24

DmitriyLewen reviewed Jul 16, 2026

View reviewed changes

DmitriyLewen reviewed Jul 17, 2026

View reviewed changes

	OS ftypes.OS
	Repository *ftypes.Repository
	PackageInfos []ftypes.PackageInfo
	Applications []ftypes.Application
	Misconfigurations []ftypes.Misconfiguration
	Secrets []ftypes.Secret
	Licenses []ftypes.LicenseFile

		left := r.CryptoAssets[i].Descriptor().String()
		right := r.CryptoAssets[j].Descriptor().String()

Uh oh!

Conversation

knqyf263 commented Jul 16, 2026

Description

Related issues

Related discussion

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants