Skip to content

Commit d153553

Browse files
peterdettmanpeterdettman
committed
PGP: fix expiry time calc when later packets increase or remove it.
1 parent e707854 commit d153553

3 files changed

Lines changed: 94 additions & 30 deletions

File tree

‎crypto/src/openpgp/PgpPublicKey.cs‎

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using Org.BouncyCastle.Asn1.Gnu;
88
using Org.BouncyCastle.Asn1.X509;
99
using Org.BouncyCastle.Asn1.X9;
10+
using Org.BouncyCastle.Bcpg.Sig;
1011
using Org.BouncyCastle.Crypto;
1112
using Org.BouncyCastle.Crypto.EC;
1213
using Org.BouncyCastle.Crypto.Parameters;
@@ -437,34 +438,41 @@ public long GetValidSeconds()
437438

438439
private long GetExpirationTimeFromSig(bool selfSigned, int signatureType)
439440
{
440-
long expiryTime = -1;
441-
long lastDate = -1;
441+
long expiryTime = -1L;
442+
long lastDate = -1L;
442443

443444
foreach (PgpSignature sig in GetSignaturesOfType(signatureType))
444445
{
445-
if (selfSigned && sig.KeyId != this.KeyId)
446-
continue;
447-
448-
PgpSignatureSubpacketVector hashed = sig.GetHashedSubPackets();
449-
if (hashed == null)
450-
continue;
451-
452-
if (!hashed.HasSubpacket(SignatureSubpacketTag.KeyExpireTime))
453-
continue;
454-
455-
long current = hashed.GetKeyExpirationTime();
456-
457-
if (sig.KeyId == this.KeyId)
446+
if (sig.KeyId == KeyId)
458447
{
459-
if (sig.CreationTime.Ticks > lastDate)
448+
/*
449+
* RFC 4880 5.2.4.1: the most recent self-signature wins, even if it omits the Key Expiration Time
450+
* subpacket (which removes any previously asserted expiry).
451+
*/
452+
var thisDate = sig.CreationTime.Ticks;
453+
if (thisDate > lastDate)
460454
{
461-
lastDate = sig.CreationTime.Ticks;
462-
expiryTime = current;
455+
lastDate = thisDate;
456+
457+
PgpSignatureSubpacketVector hashed = sig.GetHashedSubPackets();
458+
expiryTime = hashed == null ? 0L : hashed.GetKeyExpirationTime();
463459
}
464460
}
465-
else if (current == 0 || current > expiryTime)
461+
else if (!selfSigned)
466462
{
467-
expiryTime = current;
463+
PgpSignatureSubpacketVector hashed = sig.GetHashedSubPackets();
464+
if (hashed != null)
465+
{
466+
var keyExpireTime = hashed.GetSubpacket(SignatureSubpacketTag.KeyExpireTime);
467+
if (keyExpireTime != null)
468+
{
469+
long current = ((KeyExpirationTime)keyExpireTime).Time;
470+
if (current == 0 || current > expiryTime)
471+
{
472+
expiryTime = current;
473+
}
474+
}
475+
}
468476
}
469477
}
470478

‎crypto/src/openpgp/PgpSignatureSubpacketVector.cs‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -129,10 +129,7 @@ public long GetIssuerKeyId()
129129
return p == null ? 0 : ((IssuerKeyId)p).KeyId;
130130
}
131131

132-
public bool HasSignatureCreationTime()
133-
{
134-
return GetSubpacket(SignatureSubpacketTag.CreationTime) != null;
135-
}
132+
public bool HasSignatureCreationTime() => GetSubpacket(SignatureSubpacketTag.CreationTime) != null;
136133

137134
public DateTime GetSignatureCreationTime()
138135
{
@@ -142,10 +139,7 @@ public DateTime GetSignatureCreationTime()
142139
return ((SignatureCreationTime)p).GetTime();
143140
}
144141

145-
public bool HasSignatureExpirationTime()
146-
{
147-
return GetSubpacket(SignatureSubpacketTag.ExpireTime) != null;
148-
}
142+
public bool HasSignatureExpirationTime() => GetSubpacket(SignatureSubpacketTag.ExpireTime) != null;
149143

150144
/// <summary>
151145
/// Return the number of seconds a signature is valid for after its creation date.
@@ -159,6 +153,8 @@ public long GetSignatureExpirationTime()
159153
return p == null ? 0 : ((SignatureExpirationTime)p).Time;
160154
}
161155

156+
public bool HasKeyExpirationTime() => GetSubpacket(SignatureSubpacketTag.KeyExpireTime) != null;
157+
162158
/// <summary>
163159
/// Return the number of seconds a key is valid for after its creation date.
164160
/// A value of zero means the key never expires.

‎crypto/test/src/openpgp/test/PGPRSATest.cs‎

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
using System;
22
using System.IO;
33
using System.Text;
4+
using System.Threading;
45

56
using NUnit.Framework;
67

78
using Org.BouncyCastle.Bcpg.Attr;
9+
using Org.BouncyCastle.Bcpg.Sig;
810
using Org.BouncyCastle.Crypto;
911
using Org.BouncyCastle.Crypto.Parameters;
1012
using Org.BouncyCastle.Math;
@@ -538,6 +540,64 @@ private void EmbeddedJpegTest()
538540
}
539541
}
540542

543+
private void RemovedExpiryTest()
544+
{
545+
SecureRandom random = new SecureRandom();
546+
547+
// RFC 4880 5.2.4.1: a more recent self-signature without a Key Expiration
548+
// Time subpacket cancels an earlier self-signature's expiration.
549+
char[] passPhrase = "test".ToCharArray();
550+
string identity = "TEST <test@test.org>";
551+
DateTime date = DateTime.UtcNow;
552+
553+
var kpg = GeneratorUtilities.GetKeyPairGenerator("RSA");
554+
kpg.Init(new RsaKeyGenerationParameters(BigInteger.ValueOf(0x10001), random, 1024, 25));
555+
556+
var kpSgn = kpg.GenerateKeyPair();
557+
558+
PgpKeyPair sgnKeyPair = new PgpKeyPair(PublicKeyAlgorithmTag.RsaSign, kpSgn, date);
559+
560+
PgpSignatureSubpacketGenerator svg = new PgpSignatureSubpacketGenerator();
561+
svg.SetKeyExpirationTime(isCritical: true, 86400L * 366 * 2);
562+
svg.SetKeyFlags(isCritical: true, KeyFlags.CertifyOther | KeyFlags.SignData);
563+
PgpSignatureSubpacketVector hashedPcks = svg.Generate();
564+
565+
PgpKeyRingGenerator keyRingGen = new PgpKeyRingGenerator(PgpSignature.PositiveCertification, sgnKeyPair,
566+
identity, SymmetricKeyAlgorithmTag.Aes256, passPhrase, useSha1: true, hashedPcks, null, random);
567+
568+
PgpPublicKeyRing keyRing = keyRingGen.GeneratePublicKeyRing();
569+
570+
// Encode/decode
571+
keyRing = new PgpPublicKeyRing(keyRing.GetEncoded());
572+
573+
PgpPublicKey pKey = keyRing.GetPublicKey();
574+
575+
if (pKey.GetValidSeconds() != 86400L * 366 * 2)
576+
{
577+
Fail("initial key expiration time wrong");
578+
}
579+
580+
// Add a newer self-cert that omits KeyExpireTime (intent: remove the expiry).
581+
Thread.Sleep(millisecondsTimeout: 1100); // ensure later creation time at one-second granularity
582+
583+
// TODO[pgp] Add constructor that accepts also sgnKeyPair.PublicKey
584+
PgpSignatureGenerator keySigGen = new PgpSignatureGenerator(PublicKeyAlgorithmTag.RsaGeneral,
585+
HashAlgorithmTag.Sha1);
586+
keySigGen.InitSign(PgpSignature.PositiveCertification, sgnKeyPair.PrivateKey);
587+
588+
PgpSignatureSubpacketGenerator noExpiry = new PgpSignatureSubpacketGenerator();
589+
noExpiry.SetKeyFlags(isCritical: true, KeyFlags.CertifyOther | KeyFlags.SignData);
590+
keySigGen.SetHashedSubpackets(noExpiry.Generate());
591+
592+
pKey = PgpPublicKey.AddCertification(pKey, keySigGen.GenerateCertification(identity, pKey));
593+
594+
if (pKey.GetValidSeconds() != 0)
595+
{
596+
Fail("expected getValidSeconds() == 0 after newer self-sig without KEY_EXPIRE_TIME, got "
597+
+ pKey.GetValidSeconds());
598+
}
599+
}
600+
541601
public override void PerformTest()
542602
{
543603
//
@@ -897,8 +957,7 @@ public override void PerformTest()
897957
//
898958
// use of PgpKeyPair
899959
//
900-
PgpKeyPair pgpKp = new PgpKeyPair(PublicKeyAlgorithmTag.RsaGeneral,
901-
kp.Public, kp.Private, DateTime.UtcNow);
960+
PgpKeyPair pgpKp = new PgpKeyPair(PublicKeyAlgorithmTag.RsaGeneral, kp, DateTime.UtcNow);
902961

903962
PgpPublicKey k1 = pgpKp.PublicKey;
904963
PgpPrivateKey k2 = pgpKp.PrivateKey;
@@ -1125,6 +1184,7 @@ public override void PerformTest()
11251184
FingerPrintTest();
11261185
ExistingEmbeddedJpegTest();
11271186
EmbeddedJpegTest();
1187+
RemovedExpiryTest();
11281188
}
11291189

11301190
private void PerformTestSig(

0 commit comments

Comments
 (0)