diff --git a/Codec/Encryption/OpenPGP/Encrypt.hs b/Codec/Encryption/OpenPGP/Encrypt.hs --- a/Codec/Encryption/OpenPGP/Encrypt.hs +++ b/Codec/Encryption/OpenPGP/Encrypt.hs @@ -2078,7 +2078,9 @@ ECDSAPubKey (ECDSA_PublicKey recipientPub) -> do (ephemeralPub, ephemeralPriv) <- ECCGen.generate (ECDSA.public_curve recipientPub) - case point2MBS (ECDSA.public_q ephemeralPub) of + case point2MBS + (ECDSA.public_curve ephemeralPub) + (ECDSA.public_q ephemeralPub) of Nothing -> pure ( Left @@ -2164,7 +2166,9 @@ ECDSAPubKey (ECDSA_PublicKey recipientPub) -> do (ephemeralPub, ephemeralPriv) <- ECCGen.generate (ECDSA.public_curve recipientPub) - case point2MBS (ECDSA.public_q ephemeralPub) of + case point2MBS + (ECDSA.public_curve ephemeralPub) + (ECDSA.public_q ephemeralPub) of Nothing -> pure ( Left diff --git a/Codec/Encryption/OpenPGP/Internal.hs b/Codec/Encryption/OpenPGP/Internal.hs --- a/Codec/Encryption/OpenPGP/Internal.hs +++ b/Codec/Encryption/OpenPGP/Internal.hs @@ -190,14 +190,14 @@ pubkeyToMPIs (ElGamalPubKey p g y) = [MPI p, MPI g, MPI y] pubkeyToMPIs ( ECDHPubKey - (ECDSAPubKey (ECDSA_PublicKey (ECDSA.PublicKey _ q))) + (ECDSAPubKey (ECDSA_PublicKey (ECDSA.PublicKey curve q))) _ _ ) = - [MPI (os2ip (pointToBSOrError q))] + [MPI (os2ip (pointToBSOrError curve q))] pubkeyToMPIs (ECDHPubKey (EdDSAPubKey _ ep) _ _) = [MPI (edPointInteger ep)] -pubkeyToMPIs (ECDSAPubKey ((ECDSA_PublicKey (ECDSA.PublicKey _ q)))) = - [MPI (os2ip (pointToBSOrError q))] +pubkeyToMPIs (ECDSAPubKey ((ECDSA_PublicKey (ECDSA.PublicKey curve q)))) = + [MPI (os2ip (pointToBSOrError curve q))] pubkeyToMPIs (EdDSAPubKey _ ep) = [MPI (edPointInteger ep)] edPointInteger :: EdPoint -> Integer @@ -240,22 +240,20 @@ [0x2B, 0x06, 0x01, 0x04, 0x01, 0x97, 0x55, 0x01, 0x05, 0x01] curveToCurveoidBS Curve448 = Right $ B.pack [0x2B, 0x65, 0x6F] -point2MBS :: ECCT.PublicPoint -> Maybe B.ByteString -point2MBS (ECCT.Point x y) - | B.null xb || B.null yb = Nothing - | B.length xb /= B.length yb = Nothing - | otherwise = Just (B.concat [B.singleton 0x04, xb, yb]) +point2MBS :: ECCT.Curve -> ECCT.PublicPoint -> Maybe B.ByteString +point2MBS curve (ECCT.Point x y) = Just $ B.concat [B.singleton 0x04, pad x, pad y] where - xb = i2osp x - yb = i2osp y -point2MBS ECCT.PointO = Nothing + pad n = leftPadTo coordBytes (i2osp n) + coordBytes = (ECCT.curveSizeBits curve + 7) `div` 8 +point2MBS _ ECCT.PointO = Nothing -pointToBSOrError :: ECCT.PublicPoint -> B.ByteString -pointToBSOrError point = +pointToBSOrError + :: ECCT.Curve -> ECCT.PublicPoint -> B.ByteString +pointToBSOrError curve point = case point of ECCT.PointO -> error "OpenPGP forbids serializing the point at infinity" _ -> - case point2MBS point of + case point2MBS curve point of Just bs -> bs Nothing -> error diff --git a/Codec/Encryption/OpenPGP/Types/Internal/Base.hs b/Codec/Encryption/OpenPGP/Types/Internal/Base.hs --- a/Codec/Encryption/OpenPGP/Types/Internal/Base.hs +++ b/Codec/Encryption/OpenPGP/Types/Internal/Base.hs @@ -2224,7 +2224,7 @@ = NISTP256 | NISTP384 | NISTP521 - | Curve25519 + | Curve25519 -- FIXME: rename this to Curve25519 legacy | Curve448 deriving (Data, Eq, Generic, Ord, Show, Typeable) @@ -2232,8 +2232,8 @@ pretty NISTP256 = pretty "NIST P-256" pretty NISTP384 = pretty "NIST P-384" pretty NISTP521 = pretty "NIST P-521" - pretty Curve25519 = pretty "Curve25519" - pretty Curve448 = pretty "Curve448" + pretty Curve25519 = pretty "Curve25519 (legacy)" + pretty Curve448 = pretty "Curve448 (legacy)" instance Hashable ECCCurve diff --git a/hOpenPGP.cabal b/hOpenPGP.cabal --- a/hOpenPGP.cabal +++ b/hOpenPGP.cabal @@ -1,6 +1,6 @@ Cabal-version: 3.4 Name: hOpenPGP -Version: 3.6 +Version: 3.6.1 Synopsis: native Haskell implementation of OpenPGP (RFC9580) Description: native Haskell implementation of OpenPGP (RFC9580), with some backwards compatibility Homepage: https://salsa.debian.org/clint/hOpenPGP @@ -346,4 +346,4 @@ source-repository this type: git location: https://salsa.debian.org/clint/hOpenPGP.git - tag: v3.6 + tag: v3.6.1 diff --git a/tests/Tests/Encryption.hs b/tests/Tests/Encryption.hs --- a/tests/Tests/Encryption.hs +++ b/tests/Tests/Encryption.hs @@ -4720,7 +4720,7 @@ maybe (error "failed to encode ephemeral point") id - (point2MBS (ECDSA.public_q ephPub)) + (point2MBS (ECDSA.public_curve ephPub) (ECDSA.public_q ephPub)) pkesk = PKESKPkt ( PKESKPayloadV3Packet @@ -4830,7 +4830,7 @@ maybe (error "failed to encode ephemeral point") id - (point2MBS (ECDSA.public_q ephPub)) + (point2MBS (ECDSA.public_curve ephPub) (ECDSA.public_q ephPub)) truncatedEphemeral = B.take (B.length ephPointBytes - 1) ephPointBytes pkesk = PKESKPkt @@ -6255,7 +6255,7 @@ maybe (error "failed to encode ephemeral point") id - (point2MBS (ECDSA.public_q ephPub)) + (point2MBS (ECDSA.public_curve ephPub) (ECDSA.public_q ephPub)) pkesk = PKESKPkt ( PKESKPayloadV3Packet diff --git a/tests/Tests/Serialization.hs b/tests/Tests/Serialization.hs --- a/tests/Tests/Serialization.hs +++ b/tests/Tests/Serialization.hs @@ -10,7 +10,7 @@ import Control.Lens ((^.)) import Control.Monad (forM_) import qualified Crypto.Error as CE -import Crypto.Number.Serialize (os2ip) +import Crypto.Number.Serialize (i2osp, os2ip) import qualified Crypto.PubKey.ECC.ECDSA as ECDSA import qualified Crypto.PubKey.ECC.Types as ECCT import qualified Crypto.PubKey.Ed25519 as Ed25519 @@ -51,6 +51,7 @@ import Codec.Encryption.OpenPGP.Internal ( PktStreamContext (..) , emptyPSC + , leftPadTo , point2MBS ) import Codec.Encryption.OpenPGP.KeyringParser @@ -353,8 +354,11 @@ "should not serialize point at infinity" testPointAtInfinitySerialization , testCase - "should reject mismatched EC coordinate widths" - testPointSerializationRejectsMismatchedCoordinateWidths + "should pad EC coordinates to fixed width per RFC 9580 §13.2" + testPointSerializationPadsCoordinatesToFixedWidth + , testCase + "should pad P-521 EC coordinates to 66-byte field width" + testPointSerializationP521PadsTo66Bytes ] , testGroup "TKUnknown Serialization group" @@ -653,15 +657,32 @@ assertEqual "point at infinity should not serialize" Nothing - (point2MBS ECCT.PointO) + (point2MBS (ECCT.getCurveByName ECCT.SEC_p256r1) ECCT.PointO) -testPointSerializationRejectsMismatchedCoordinateWidths +testPointSerializationPadsCoordinatesToFixedWidth :: Assertion -testPointSerializationRejectsMismatchedCoordinateWidths = +testPointSerializationPadsCoordinatesToFixedWidth = assertEqual - "point serialization should reject mismatched coordinate widths" - Nothing - (point2MBS (ECCT.Point 1 256)) + "point serialization should pad coordinates to the fixed field width (RFC 9580 §13.2)" + ( Just + (B.cons 0x04 (leftPadTo 32 (i2osp 1) <> leftPadTo 32 (i2osp 1))) + ) + (point2MBS (ECCT.getCurveByName ECCT.SEC_p256r1) (ECCT.Point 1 1)) + +{- | Regression test: P-521 has a 521-bit prime, so coordinates +can be 1..66 bytes long when minimally encoded. Per RFC 9580 +section 13.2 the wire form must use the fixed field width +@B = ceil(521/8) = 66@ bytes per coordinate regardless. +-} +testPointSerializationP521PadsTo66Bytes + :: Assertion +testPointSerializationP521PadsTo66Bytes = + assertEqual + "P-521 EC point serialization should always produce 1 + 2*66 = 133 bytes" + (Just 133) + ( B.length + <$> point2MBS (ECCT.getCurveByName ECCT.SEC_p521r1) (ECCT.Point 1 1) + ) testSEIPDv2PacketRoundTrip :: Assertion testSEIPDv2PacketRoundTrip = do