improve verification of RFC3161 timestamps (#921)

Signed-off-by: Brian DeHamer <bdehamer@github.com>
sigstore · Jan 3, 2024 · 6a6bfbc · 6a6bfbc
1 parent 922a1be
commit 6a6bfbc
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 2 deletions.
diff --git a/.changeset/khaki-chefs-smile.md b/.changeset/khaki-chefs-smile.md
@@ -0,0 +1,5 @@
+---
+"@sigstore/core": patch
+---
+
+Add more checks to the `RFC3161Timestamp.verify` method
diff --git a/packages/core/src/__tests__/rfc3161/timestamp.test.ts b/packages/core/src/__tests__/rfc3161/timestamp.test.ts
@@ -83,7 +83,7 @@ describe('RFC3161Timestamp', () => {
       });
     });
 
-    describe('when the artifact does NOT the one which was signed', () => {
+    describe('when the artifact does NOT match the one which was signed', () => {
       const subject = RFC3161Timestamp.parse(ts);
       const key = createPublicKey(publicKey);
       const data = Buffer.from('oops');
@@ -112,6 +112,28 @@ describe('RFC3161Timestamp', () => {
       });
     });
 
+    describe('when the content type is NOT signedData', () => {
+      const data = artifact;
+      const subject = RFC3161Timestamp.parse(
+        Buffer.from(
+          'MIICHDADAgEAMIICEwYJKoZIhvcNAQcDoIICBDCCAgACAQMxDzANBglghkgBZQMEAgEFADB0BgsqhkiG9w0BCRABBKBlBGMwYQIBAQYJKwYBBAGDvzACMC8wCwYJYIZIAWUDBAIBBCCFP/k3YqBt2/cixOvp3dZtj2Pdrql/Uhw+zCDafJdgIAIE3q2+7xgPMjAzMDAxMDEwMDAwMDBaMAMCAQECBEmWAtKgADGCAXAwggFsAgEBMCswJjEMMAoGA1UEAxMDdHNhMRYwFAYDVQQKEw1zaWdzdG9yZS5tb2NrAgECMA0GCWCGSAFlAwQCAQUAoIHVMBoGCSqGSIb3DQEJAzENBgsqhkiG9w0BCRABBDAcBgkqhkiG9w0BCQUxDxcNMzAwMTAxMDAwMDAwWjAvBgkqhkiG9w0BCQQxIgQg9EVaiVg5rwrcg1wj/j99tIJrWKYn60AIthUWerryKLcwaAYLKoZIhvcNAQkQAi8xWTBXMFUwUwQgvgCzJyhCRVJAO8TE9bSZaZbQEA7BBdhR2jom19ZI/icwLzAqpCgwJjEMMAoGA1UEAxMDdHNhMRYwFAYDVQQKEw1zaWdzdG9yZS5tb2NrAgECMAoGCCqGSM49BAMCBEcwRQIhAN7VR7nM4ppVskxDo5PiWGiIaPB6Cf76vRohGAmU5uo7AiAWlZC7auQlm8xTwpBkgJDd0dXsTesTfeMoREFKXdDzMw==',
+          'base64'
+        )
+      );
+      const key = createPublicKey(
+        '-----BEGIN PUBLIC KEY-----\n' +
+          'MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEhVfmhwUnwzdna6vDGCIqWmBy44j+\n' +
+          'RT/YrDe5a0NeFf8njTyxcML4pp2VsI6clbXCTqCcoTDTy1pgow3QJXFdsw==\n' +
+          '-----END PUBLIC KEY-----\n'
+      );
+
+      it('throws an error', () => {
+        expect(() => subject.verify(data, key)).toThrow(
+          RFC3161TimestampVerificationError
+        );
+      });
+    });
+
     describe('when the encapsulated content type is NOT tstInfo', () => {
       const data = artifact;
       const subject = RFC3161Timestamp.parse(
@@ -134,6 +156,19 @@ describe('RFC3161Timestamp', () => {
       });
     });
 
+    describe('when the timestamp token is missing', () => {
+      const data = artifact;
+      const key = createPublicKey(publicKey);
+      const subject = RFC3161Timestamp.parse(
+        Buffer.from('30053003020100', 'hex')
+      );
+      it('throws an error', () => {
+        expect(() => subject.verify(data, key)).toThrow(
+          RFC3161TimestampVerificationError
+        );
+      });
+    });
+
     describe('when the signed message does NOT match the tstInfo', () => {
       const data = artifact;
       const subject = RFC3161Timestamp.parse(
@@ -148,6 +183,7 @@ describe('RFC3161Timestamp', () => {
           'u3T+xRv5VpDSfvYJPT46EMpov8AJkR1G4rs0iV1csuammXKF+BQzvLh/qQ==\n' +
           '-----END PUBLIC KEY-----\n'
       );
+
       it('throws an error', () => {
         expect(() => subject.verify(data, key)).toThrow(
           RFC3161TimestampVerificationError

diff --git a/packages/core/src/__tests__/rfc3161/tstinfo.test.ts b/packages/core/src/__tests__/rfc3161/tstinfo.test.ts
@@ -61,7 +61,7 @@ describe('TSTInfo', () => {
     describe('when the messageImprintHashedMessage does NOT match the artifact', () => {
       const artifact = Buffer.from('oops');
 
-      it('does not throw an error', () => {
+      it('throws an error', () => {
         expect(() => subject.verify(artifact)).toThrow(
           RFC3161TimestampVerificationError
         );

diff --git a/packages/core/src/rfc3161/timestamp.ts b/packages/core/src/rfc3161/timestamp.ts
@@ -19,6 +19,7 @@ import { ECDSA_SIGNATURE_ALGOS, SHA2_HASH_ALGOS } from '../oid';
 import { RFC3161TimestampVerificationError } from './error';
 import { TSTInfo } from './tstinfo';
 
+const OID_PKCS9_CONTENT_TYPE_SIGNED_DATA = '1.2.840.113549.1.7.2';
 const OID_PKCS9_CONTENT_TYPE_TSTINFO = '1.2.840.113549.1.9.16.1.4';
 const OID_PKCS9_MESSAGE_DIGEST_KEY = '1.2.840.113549.1.9.4';
 
@@ -38,6 +39,10 @@ export class RFC3161Timestamp {
     return this.pkiStatusInfoObj.subs[0].toInteger();
   }
 
+  get contentType(): string {
+    return this.contentTypeObj.toOID();
+  }
+
   get eContentType(): string {
     return this.eContentTypeObj.toOID();
   }
@@ -74,6 +79,17 @@ export class RFC3161Timestamp {
   }
 
   public verify(data: Buffer, publicKey: crypto.KeyObject): void {
+    if (!this.timeStampTokenObj) {
+      throw new RFC3161TimestampVerificationError('timeStampToken is missing');
+    }
+
+    // Check for expected ContentInfo content type
+    if (this.contentType !== OID_PKCS9_CONTENT_TYPE_SIGNED_DATA) {
+      throw new RFC3161TimestampVerificationError(
+        `incorrect content type: ${this.contentType}`
+      );
+    }
+
     // Check for expected encapsulated content type
     if (this.eContentType !== OID_PKCS9_CONTENT_TYPE_TSTINFO) {
       throw new RFC3161TimestampVerificationError(
@@ -136,6 +152,10 @@ export class RFC3161Timestamp {
     return this.root.subs[1];
   }
 
+  // https://datatracker.ietf.org/doc/html/rfc5652#section-3
+  private get contentTypeObj(): ASN1Obj {
+    return this.timeStampTokenObj.subs[0];
+  }
   // https://www.rfc-editor.org/rfc/rfc5652#section-3
   private get signedDataObj(): ASN1Obj {
     const obj = this.timeStampTokenObj.subs.find((sub) =>