Timestamp verification fail

[dissoupov]

in the the function func (sd *SignedData) verify
there is a loop

for _, si := range sd.psd.SignerInfos {

		algo := si.X509SignatureAlgorithm()
		if algo == x509.UnknownSignatureAlgorithm {
			return nil, protocol.ErrUnsupported
		}

The line 123 fails, for RSAWithSHA1 signature: OID 1.2.840.113549.1.1.11

The following map is wrong when your code deals with external Timestamp servers.

// SignatureAlgorithms maps digest and signature OIDs to
// x509.SignatureAlgorithm values.
var SignatureAlgorithms = map[string]map[string]x509.SignatureAlgorithm{
	SignatureAlgorithmRSA.String(): map[string]x509.SignatureAlgorithm{
		DigestAlgorithmSHA1.String():   x509.SHA1WithRSA,
		DigestAlgorithmMD5.String():    x509.MD5WithRSA,
		DigestAlgorithmSHA256.String(): x509.SHA256WithRSA,
		DigestAlgorithmSHA384.String(): x509.SHA384WithRSA,
		DigestAlgorithmSHA512.String(): x509.SHA512WithRSA,
	},
	SignatureAlgorithmECDSA.String(): map[string]x509.SignatureAlgorithm{
		DigestAlgorithmSHA1.String():   x509.ECDSAWithSHA1,
		DigestAlgorithmSHA256.String(): x509.ECDSAWithSHA256,
		DigestAlgorithmSHA384.String(): x509.ECDSAWithSHA384,
		DigestAlgorithmSHA512.String(): x509.ECDSAWithSHA512,
	},
}

// X509SignatureAlgorithm gets the x509.SignatureAlgorithm that should be used
// for verifying this SignerInfo's signature.
func (si SignerInfo) X509SignatureAlgorithm() x509.SignatureAlgorithm {
	var (
		sigOID    = si.SignatureAlgorithm.Algorithm.String()
		digestOID = si.DigestAlgorithm.Algorithm.String()
	)

	return oid.SignatureAlgorithms[sigOID][digestOID]
}

You should support the following mappings from SignerInfos:

oid: "1.2.840.113549.1.1.5" => RSAWithSHA1
oid: "1.2.840.113549.1.1.11" => RSAWithSHA256
oid: "1.2.840.113549.1.1.12" => RSAWithSHA384
oid: "1.2.840.113549.1.1.13" => RSAWithSHA512
oid: "1.2.840.10045.4.1" => ECDSAWithSHA1
oid: "2.16.840.1.101.4.3.2" => ECDSAWithSHA256
oid: "2.16.840.1.101.4.3.3" => ECDSAWithSHA384
oid: "2.16.840.1.101.4.3.4" => ECDSAWithSHA512

[btoews]

Thanks for opening this. I don't quite understand the issue you are having. It would be helpful if you could provide an example of a signature/timestamp that isn't working but should be. This library should support all of the algorithm OIDs you list. I think you may have the wrong values for ECDSAWithSHA256 , ECDSAWithSHA384 and ECDSAWithSHA512 though.

Here are the OIDs I see for those algorithms:

oidSignatureECDSAWithSHA256 = asn1.ObjectIdentifier{1, 2, 840, 10045, 4, 3, 2}
oidSignatureECDSAWithSHA384 = asn1.ObjectIdentifier{1, 2, 840, 10045, 4, 3, 3}
oidSignatureECDSAWithSHA512 = asn1.ObjectIdentifier{1, 2, 840, 10045, 4, 3, 4}

[dissoupov]

Sorry for the confusion, I pasted wrong OIDs for ECDSA indeed, but the timestamp verification fails for
oidSignatureSHA256WithRSA = asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 1, 11}

The logic in X509SignatureAlgorithm is incorrect.
You combine sigOID with digestOID which is OK for PubKey alg = digest.
But SignerInfos already contains valid SignatureAlgorithm that should be mapped directly.

// X509SignatureAlgorithm gets the x509.SignatureAlgorithm that should be used
// for verifying this SignerInfo's signature.
func (si SignerInfo) X509SignatureAlgorithm() x509.SignatureAlgorithm {
	var (
		sigOID    = si.SignatureAlgorithm.Algorithm.String()
		digestOID = si.DigestAlgorithm.Algorithm.String()
	)

	return oid.SignatureAlgorithms[sigOID][digestOID]
}

I think you should create another map for:
oidSignatureECDSAWithSHA256 = asn1.ObjectIdentifier{1, 2, 840, 10045, 4, 3, 2}
oidSignatureECDSAWithSHA384 = asn1.ObjectIdentifier{1, 2, 840, 10045, 4, 3, 3}
oidSignatureECDSAWithSHA512 = asn1.ObjectIdentifier{1, 2, 840, 10045, 4, 3, 4}

and use it without digestOID

[dissoupov]

Just get some TS responses for publicly available TSA and test your timestamp package with 3rd party responses. You will see the failures.

[btoews]

I think that makes sense. The spec says

10.1.2.  SignatureAlgorithmIdentifier

   The SignatureAlgorithmIdentifier type identifies a signature
   algorithm, and it can also identify a message digest algorithm.
   Examples include RSA, DSA, DSA with SHA-1, ECDSA, and ECDSA with
   SHA-256.  A signature algorithm supports signature generation and
   verification operations.  The signature generation operation uses the
   message digest and the signer's private key to generate a signature
   value.  The signature verification operation uses the message digest
   and the signer's public key to determine whether or not a signature
   value is valid.  Context determines which operation is intended.

      SignatureAlgorithmIdentifier ::= AlgorithmIdentifier

So, I need to support signature+digest algorithm (eg. SHA256WithRSA) in the SignatureAlgorithm in addition to just the signature algorithm (eg. RSA). If both signature and digest algorithm are specified in the SignatureAlgorithm should the SignerInfo's DigestAlgorithm not be checked at all? What if the DigestAlgorithm is SHA256, but the SignatureAlgorithm is RSAWithSHA1? The spec isn't very clear on this.

[btoews]

All the public timestamp authorities I've worked with just specify RSA (1.2.840.113549.1.1.1) for the SignatureAlgorithm. For example here is a timestamp from GlobalSign. An example that specifies signature+digest algo would be helpful for writing a test of this behavior.

[dissoupov]

You are correct, I also found bunch of Symantec Time Stamping Services CA - G2 responses that have
RSA (1.2.840.113549.1.1.1) as SignatureAlgorithm.

Based on my understanding of the standard:

 The SignatureAlgorithmIdentifier type identifies a signature
   algorithm, and it can also identify a message digest algorithm.

the code should have a look up for complete SignatureAlgorithm OID and for PublicKeyAlgorithm + DigectAlgorithm OIDs

[btoews]

Is it an invalid message if the SignatureAlgorithm is RSAWithSHA1, but the DigestAlgorithm is SHA256?

[dissoupov]

There are two places where digest is used:

• Signature, where the actual signature is verified, let's say RSAWithSHA1
• Then, each Signed Attribute's digest should be verified, to compare digest of its attribute value. I think DigestAlgorithm is used explicitly in this case.

Looks like if digest algorithm is the same in Signature and in Digest, then some vendors ise PublickeyAlgorithm in SignatureAlgorithmIdentifier and combine it with DigestAlgorithm, as we saw above.

[dissoupov]

I'm testing the following logic based on your code:

	x509SigAlgo := signer.X509SignatureAlgorithm()
	if x509SigAlgo == x509.UnknownSignatureAlgorithm {
                // search for full SignatureAlgorithmIdentifier
		sigAlgo, err := cryptoid.SignatureAlgorithmByOID(sigAlgoOid)
		if err != nil {
			return errors.Trace(err)
		}
		x509SigAlgo = sigAlgo.X509
	}

        // use x509SigAlgo

Seems to be working with all 3rd party TSR that I've got.