From d3d1b114cebd2d675adb81720431707b467fa40f Mon Sep 17 00:00:00 2001 From: Dhruba Basu <7675102+dhrubabasu@users.noreply.github.com> Date: Thu, 18 Jan 2024 12:17:13 -0500 Subject: [PATCH 1/2] Always attempt to install mockgen `v0.4.0` before execution (#2628) --- scripts/mock.gen.sh | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/scripts/mock.gen.sh b/scripts/mock.gen.sh index 75997d18b86e..16b00a42a569 100755 --- a/scripts/mock.gen.sh +++ b/scripts/mock.gen.sh @@ -7,12 +7,8 @@ if ! [[ "$0" =~ scripts/mock.gen.sh ]]; then exit 255 fi -if ! command -v mockgen &> /dev/null -then - echo "mockgen not found, installing..." - # https://github.com/uber-go/mock - go install -v go.uber.org/mock/mockgen@v0.4.0 -fi +# https://github.com/uber-go/mock +go install -v go.uber.org/mock/mockgen@v0.4.0 source ./scripts/constants.sh From f5884bb413ffa19987302df44cfe34f738d93fdf Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Thu, 18 Jan 2024 13:34:21 -0500 Subject: [PATCH 2/2] Modify TLS parsing rules for Durango (#2458) Signed-off-by: Stephen Buttolph Co-authored-by: Filippo Valsorda --- indexer/examples/p-chain/main.go | 3 +- indexer/examples/x-chain-blocks/main.go | 3 +- network/network.go | 10 +- network/peer/peer.go | 18 ++- network/peer/test_peer.go | 1 + network/peer/upgrader.go | 36 ++--- staking/asn1.go | 28 +++- staking/certificate.go | 10 +- staking/large_rsa_key.sig | Bin 2048 -> 0 bytes staking/parse.go | 172 +++++++++++++++++++++++- staking/parse_test.go | 89 ++++++++++++ staking/verify.go | 39 ++---- staking/verify_test.go | 30 ----- vms/proposervm/batched_vm.go | 2 +- vms/proposervm/block/block.go | 15 ++- vms/proposervm/block/build.go | 9 +- vms/proposervm/block/option.go | 4 +- vms/proposervm/block/parse.go | 9 +- vms/proposervm/block/parse_test.go | 56 +++++--- vms/proposervm/state/block_state.go | 7 +- vms/proposervm/vm.go | 2 +- 21 files changed, 422 insertions(+), 121 deletions(-) delete mode 100644 staking/large_rsa_key.sig create mode 100644 staking/parse_test.go delete mode 100644 staking/verify_test.go diff --git a/indexer/examples/p-chain/main.go b/indexer/examples/p-chain/main.go index 9f8966d21546..80f0e1aeed8e 100644 --- a/indexer/examples/p-chain/main.go +++ b/indexer/examples/p-chain/main.go @@ -10,6 +10,7 @@ import ( "time" "github.com/ava-labs/avalanchego/indexer" + "github.com/ava-labs/avalanchego/version" "github.com/ava-labs/avalanchego/wallet/subnet/primary" platformvmblock "github.com/ava-labs/avalanchego/vms/platformvm/block" @@ -34,7 +35,7 @@ func main() { } platformvmBlockBytes := container.Bytes - proposerVMBlock, err := proposervmblock.Parse(container.Bytes) + proposerVMBlock, err := proposervmblock.Parse(container.Bytes, version.DefaultUpgradeTime) if err == nil { platformvmBlockBytes = proposerVMBlock.Block() } diff --git a/indexer/examples/x-chain-blocks/main.go b/indexer/examples/x-chain-blocks/main.go index 92c5f7ad34e7..e25693b62998 100644 --- a/indexer/examples/x-chain-blocks/main.go +++ b/indexer/examples/x-chain-blocks/main.go @@ -10,6 +10,7 @@ import ( "time" "github.com/ava-labs/avalanchego/indexer" + "github.com/ava-labs/avalanchego/version" "github.com/ava-labs/avalanchego/vms/proposervm/block" "github.com/ava-labs/avalanchego/wallet/chain/x" "github.com/ava-labs/avalanchego/wallet/subnet/primary" @@ -32,7 +33,7 @@ func main() { continue } - proposerVMBlock, err := block.Parse(container.Bytes) + proposerVMBlock, err := block.Parse(container.Bytes, version.DefaultUpgradeTime) if err != nil { log.Fatalf("failed to parse proposervm block: %s\n", err) } diff --git a/network/network.go b/network/network.go index 51c2d223829e..1e13ed45b9c2 100644 --- a/network/network.go +++ b/network/network.go @@ -273,6 +273,12 @@ func NewNetwork( IPSigner: peer.NewIPSigner(config.MyIPPort, config.TLSKey), } + // Invariant: We delay the activation of durango during the TLS handshake to + // avoid gossiping any TLS certs that anyone else in the network may + // consider invalid. Recall that if a peer gossips an invalid cert, the + // connection is terminated. + durangoTime := version.GetDurangoTime(config.NetworkID) + durangoTimeWithClockSkew := durangoTime.Add(config.MaxClockDifference) onCloseCtx, cancel := context.WithCancel(context.Background()) n := &network{ config: config, @@ -283,8 +289,8 @@ func NewNetwork( inboundConnUpgradeThrottler: throttling.NewInboundConnUpgradeThrottler(log, config.ThrottlerConfig.InboundConnUpgradeThrottlerConfig), listener: listener, dialer: dialer, - serverUpgrader: peer.NewTLSServerUpgrader(config.TLSConfig, metrics.tlsConnRejected), - clientUpgrader: peer.NewTLSClientUpgrader(config.TLSConfig, metrics.tlsConnRejected), + serverUpgrader: peer.NewTLSServerUpgrader(config.TLSConfig, metrics.tlsConnRejected, durangoTimeWithClockSkew), + clientUpgrader: peer.NewTLSClientUpgrader(config.TLSConfig, metrics.tlsConnRejected, durangoTimeWithClockSkew), onCloseCtx: onCloseCtx, onCloseCtxCancel: cancel, diff --git a/network/peer/peer.go b/network/peer/peer.go index db2cb5485b04..403bea5220ba 100644 --- a/network/peer/peer.go +++ b/network/peer/peer.go @@ -1194,10 +1194,22 @@ func (p *peer) handlePeerList(msg *p2p.PeerList) { close(p.onFinishHandshake) } - // the peers this peer told us about - discoveredIPs := make([]*ips.ClaimedIPPort, len(msg.ClaimedIpPorts)) + // Invariant: We do not account for clock skew here, as the sender of the + // certificate is expected to account for clock skew during the activation + // of Durango. + durangoTime := version.GetDurangoTime(p.NetworkID) + beforeDurango := time.Now().Before(durangoTime) + discoveredIPs := make([]*ips.ClaimedIPPort, len(msg.ClaimedIpPorts)) // the peers this peer told us about for i, claimedIPPort := range msg.ClaimedIpPorts { - tlsCert, err := staking.ParseCertificate(claimedIPPort.X509Certificate) + var ( + tlsCert *staking.Certificate + err error + ) + if beforeDurango { + tlsCert, err = staking.ParseCertificate(claimedIPPort.X509Certificate) + } else { + tlsCert, err = staking.ParseCertificatePermissive(claimedIPPort.X509Certificate) + } if err != nil { p.Log.Debug("message with invalid field", zap.Stringer("nodeID", p.id), diff --git a/network/peer/test_peer.go b/network/peer/test_peer.go index 7bd58344ab86..04cfd93aaa7a 100644 --- a/network/peer/test_peer.go +++ b/network/peer/test_peer.go @@ -65,6 +65,7 @@ func StartTestPeer( clientUpgrader := NewTLSClientUpgrader( tlsConfg, prometheus.NewCounter(prometheus.CounterOpts{}), + version.GetDurangoTime(networkID), ) peerID, conn, cert, err := clientUpgrader.Upgrade(conn) diff --git a/network/peer/upgrader.go b/network/peer/upgrader.go index dd973af1b825..9341922175cd 100644 --- a/network/peer/upgrader.go +++ b/network/peer/upgrader.go @@ -7,6 +7,7 @@ import ( "crypto/tls" "errors" "net" + "time" "github.com/prometheus/client_golang/prometheus" @@ -29,36 +30,40 @@ type Upgrader interface { type tlsServerUpgrader struct { config *tls.Config invalidCerts prometheus.Counter + durangoTime time.Time } -func NewTLSServerUpgrader(config *tls.Config, invalidCerts prometheus.Counter) Upgrader { +func NewTLSServerUpgrader(config *tls.Config, invalidCerts prometheus.Counter, durangoTime time.Time) Upgrader { return &tlsServerUpgrader{ config: config, invalidCerts: invalidCerts, + durangoTime: durangoTime, } } func (t *tlsServerUpgrader) Upgrade(conn net.Conn) (ids.NodeID, net.Conn, *staking.Certificate, error) { - return connToIDAndCert(tls.Server(conn, t.config), t.invalidCerts) + return connToIDAndCert(tls.Server(conn, t.config), t.invalidCerts, t.durangoTime) } type tlsClientUpgrader struct { config *tls.Config invalidCerts prometheus.Counter + durangoTime time.Time } -func NewTLSClientUpgrader(config *tls.Config, invalidCerts prometheus.Counter) Upgrader { +func NewTLSClientUpgrader(config *tls.Config, invalidCerts prometheus.Counter, durangoTime time.Time) Upgrader { return &tlsClientUpgrader{ config: config, invalidCerts: invalidCerts, + durangoTime: durangoTime, } } func (t *tlsClientUpgrader) Upgrade(conn net.Conn) (ids.NodeID, net.Conn, *staking.Certificate, error) { - return connToIDAndCert(tls.Client(conn, t.config), t.invalidCerts) + return connToIDAndCert(tls.Client(conn, t.config), t.invalidCerts, t.durangoTime) } -func connToIDAndCert(conn *tls.Conn, invalidCerts prometheus.Counter) (ids.NodeID, net.Conn, *staking.Certificate, error) { +func connToIDAndCert(conn *tls.Conn, invalidCerts prometheus.Counter, durangoTime time.Time) (ids.NodeID, net.Conn, *staking.Certificate, error) { if err := conn.Handshake(); err != nil { return ids.EmptyNodeID, nil, nil, err } @@ -72,17 +77,18 @@ func connToIDAndCert(conn *tls.Conn, invalidCerts prometheus.Counter) (ids.NodeI // Invariant: ParseCertificate is used rather than CertificateFromX509 to // ensure that signature verification can assume the certificate was // parseable according the staking package's parser. - peerCert, err := staking.ParseCertificate(tlsCert.Raw) - if err != nil { - invalidCerts.Inc() - return ids.EmptyNodeID, nil, nil, err + // + // TODO: Remove pre-Durango parsing after v1.11.x has activated. + var ( + peerCert *staking.Certificate + err error + ) + if time.Now().Before(durangoTime) { + peerCert, err = staking.ParseCertificate(tlsCert.Raw) + } else { + peerCert, err = staking.ParseCertificatePermissive(tlsCert.Raw) } - - // We validate the certificate here to attempt to make the validity of the - // peer certificate as clear as possible. Specifically, a node running a - // prior version using an invalid certificate should not be able to report - // healthy. - if err := staking.ValidateCertificate(peerCert); err != nil { + if err != nil { invalidCerts.Inc() return ids.EmptyNodeID, nil, nil, err } diff --git a/staking/asn1.go b/staking/asn1.go index a21ebc7ff88c..afd817a95cd6 100644 --- a/staking/asn1.go +++ b/staking/asn1.go @@ -6,6 +6,7 @@ package staking import ( "crypto" "crypto/x509" + "encoding/asn1" "fmt" // Explicitly import for the crypto.RegisterHash init side-effects. @@ -14,11 +15,28 @@ import ( _ "crypto/sha256" ) -// Ref: https://github.com/golang/go/blob/go1.19.12/src/crypto/x509/x509.go#L326-L350 -var signatureAlgorithmVerificationDetails = map[x509.SignatureAlgorithm]x509.PublicKeyAlgorithm{ - x509.SHA256WithRSA: x509.RSA, - x509.ECDSAWithSHA256: x509.ECDSA, -} +var ( + // Ref: https://github.com/golang/go/blob/go1.19.12/src/crypto/x509/x509.go#L433-L452 + // + // RFC 3279, 2.3 Public Key Algorithms + // + // pkcs-1 OBJECT IDENTIFIER ::== { iso(1) member-body(2) us(840) + // rsadsi(113549) pkcs(1) 1 } + // + // rsaEncryption OBJECT IDENTIFIER ::== { pkcs1-1 1 } + oidPublicKeyRSA = asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 1, 1} + // RFC 5480, 2.1.1 Unrestricted Algorithm Identifier and Parameters + // + // id-ecPublicKey OBJECT IDENTIFIER ::= { + // iso(1) member-body(2) us(840) ansi-X9-62(10045) keyType(2) 1 } + oidPublicKeyECDSA = asn1.ObjectIdentifier{1, 2, 840, 10045, 2, 1} + + // Ref: https://github.com/golang/go/blob/go1.19.12/src/crypto/x509/x509.go#L326-L350 + signatureAlgorithmVerificationDetails = map[x509.SignatureAlgorithm]x509.PublicKeyAlgorithm{ + x509.SHA256WithRSA: x509.RSA, + x509.ECDSAWithSHA256: x509.ECDSA, + } +) func init() { if !crypto.SHA256.Available() { diff --git a/staking/certificate.go b/staking/certificate.go index 53d86a748b97..b3e1a511f63f 100644 --- a/staking/certificate.go +++ b/staking/certificate.go @@ -3,11 +3,15 @@ package staking -import "crypto/x509" +import ( + "crypto" + "crypto/x509" +) type Certificate struct { - Raw []byte - PublicKey any + Raw []byte + PublicKey crypto.PublicKey + // TODO: Remove after v1.11.x activates. SignatureAlgorithm x509.SignatureAlgorithm } diff --git a/staking/large_rsa_key.sig b/staking/large_rsa_key.sig deleted file mode 100644 index 61000a9903cfd642fa3130da4f85881ebfb8f43e..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 2048 zcmV+b2>kPMVuz>0LLX_JB4Rxwmf~~%Xw^SRfM5v=gT;&2{tAvS z&O$y_2aS0G99O-y1k5Oec>Hu<7Hr$#jb;mzA1H*Tf4k27Ev;(6qmT-SH(^6Q_Rk6z za}FHoL|idFRi1%V;NVb3Pw&?fyV8L= zpJT}YinNNgD5NY-jR|!}ppH2C+koCkaRDk}84mQlqWbEedTv@Udl~=c3rtg3QBipd zO;?M=oD2#x+u!QAbN_SPi3ZyBmKVkuJHig+Sn$gL%tvK&GLu`LlK(Lsis@}aQotXM zJFI6KjfN~!1-w8xW~|rYd*Xw5 zOQ<6&uxML+s!3(bhh%zz5>21CjDNc#=Wo0?{2e|Hx=2&ukrrW1EinylkOg&Q(8N4W zNNgD9PRI+aEtCqBvS2>5B6=UOdEc7BQ!X50UN4N;V2XHg-$}Z;9CqrB~`69czr#EWp_Rr+)}^Z zOT-#SO|l<-5PA5xXnr0-7i*pWW_!l$I6W#`Q1u_)C-vh*Gq6?OnUW=6+4Sdg!0m1p zO0HNtA;#%ar=DojUB$IyFh zrAw;&iB=z>{af@Z91`)tR>MbeQ)y^00_l{y4|kga2`Bwb;5WcZfJ{BvU!J+zXIRrz znLfmeoQ+t+-$=t9v!hZlI2Q{{spvAv^Aq|oW7#omms1(2P$m^_RPkm+Mt1E=7DOk2 zEb$Yzw6k5Su6%piX!4bfEwdI3N zC9)q2)-y&+X#sA0FnmkNB0oJK^_0SeKmeDpq)^xJgx+t8=T*R+FY-jL$KJeme z3}71OA6zNg*&_`IMpZ&8-kZ25k-o$6q_n@J@spb~ndhKwAf$W)&vJUA&W_fENvMDE z29}#=&0WV z2Z|Uh0804pu1`6n?POSXz?11Wwmhe9XKl8^ZUZ@j^9@)fo5+x~L$+884XR#C_rwZ2Gt{fQaD%fjffSd=gCrrn^eOsJ5gyGUruECTQB^=-NC z6L_kjWzC=4Og%E!&qy6_tE5W3o)LQd^)8*xs#0|6Axrjgdr@`h53JLtete6HgXedgfeNn^pEmpQmj%aWDM-(9HVO-O!vX|nY;E-?GJ_z zjyI_@2>FR-Fq~+4?>jB0LWc#vBREpe`6lF|j@E`s1!KI!MO> zwJhKbDjyXke`W(QSn7fUYB1+nEzI${6V@!+>q%^+>0>cT{z+2}=XLmnG+IJc z)|QdFLS+EHJ6aPWVcHd#$O(tD_agBWyO@()+RA83UfBv`rR@=fk;tw@2fcGCq(PUb zAQE__HZY{qUY)-JR8i|~nmL6AU0r%sg5B8CSVTvH_djo`?-%2lOqzZdraAWkBCHJw~;aftx{U6XvSYt1J7r%)if*DY7KNOnzkHw4v*$<0>}NpL4k zE{hUQVd=u9V41E9g(IKQIoycing7Jwiv6^MbKFb3$o0$yMuKc_OWAIXUdgF>-KNo| zULv0~ufVqWpRYe?{OSG969^}SAxiSe1dEtakosm;HM8L+>DDqkxOf#zW0EV$5ErJQ za&IPh-{VBqh6@iKN%0}LFlfpsFswL|HjC)~irI)~A6XpCq^2rZz}EByGCir^JRWhL zp>IJoXw4`PZa$iEds?lDuEC@Zc0+lnhlFT>2q4(c?B%2F12a0)0FuNNSBcKcae5zjO%6nkiYI|L0=Y1j-^{G)NdIeBu|MWA+Rg)7Ef& zwWza$ZE`(uO{e4FmFvA652$jt~!iDqGxkW$v8C*27U$419&Ufyx=Fx?; z3)rUcLQpTw&9@if5fbkgc$Y1GI)h=3&b7hhTFv&fl0U-4IhZb#78{iDrA(U`kGhc5 eShhHiq3tib1FXJZIB*UCvmaBS&ZT&-Cn|XI>E;#y diff --git a/staking/parse.go b/staking/parse.go index 6de6b63dd4dd..fd21c3cbe38e 100644 --- a/staking/parse.go +++ b/staking/parse.go @@ -3,13 +3,179 @@ package staking -import "crypto/x509" +import ( + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rsa" + "crypto/x509" + "encoding/asn1" + "errors" + "fmt" + "math/big" + + "golang.org/x/crypto/cryptobyte" + + cryptobyte_asn1 "golang.org/x/crypto/cryptobyte/asn1" + + "github.com/ava-labs/avalanchego/utils/units" +) + +const ( + MaxCertificateLen = 2 * units.KiB + + allowedRSASmallModulusLen = 2048 + allowedRSALargeModulusLen = 4096 + allowedRSAPublicExponentValue = 65537 +) + +var ( + ErrCertificateTooLarge = fmt.Errorf("staking: certificate length is greater than %d", MaxCertificateLen) + ErrMalformedCertificate = errors.New("staking: malformed certificate") + ErrMalformedTBSCertificate = errors.New("staking: malformed tbs certificate") + ErrMalformedVersion = errors.New("staking: malformed version") + ErrMalformedSerialNumber = errors.New("staking: malformed serial number") + ErrMalformedSignatureAlgorithmIdentifier = errors.New("staking: malformed signature algorithm identifier") + ErrMalformedIssuer = errors.New("staking: malformed issuer") + ErrMalformedValidity = errors.New("staking: malformed validity") + ErrMalformedSPKI = errors.New("staking: malformed spki") + ErrMalformedPublicKeyAlgorithmIdentifier = errors.New("staking: malformed public key algorithm identifier") + ErrMalformedSubjectPublicKey = errors.New("staking: malformed subject public key") + ErrMalformedOID = errors.New("staking: malformed oid") + ErrInvalidRSAPublicKey = errors.New("staking: invalid RSA public key") + ErrInvalidRSAModulus = errors.New("staking: invalid RSA modulus") + ErrInvalidRSAPublicExponent = errors.New("staking: invalid RSA public exponent") + ErrRSAModulusNotPositive = errors.New("staking: RSA modulus is not a positive number") + ErrUnsupportedRSAModulusBitLen = errors.New("staking: unsupported RSA modulus bitlen") + ErrRSAModulusIsEven = errors.New("staking: RSA modulus is an even number") + ErrUnsupportedRSAPublicExponent = errors.New("staking: unsupported RSA public exponent") + ErrFailedUnmarshallingEllipticCurvePoint = errors.New("staking: failed to unmarshal elliptic curve point") + ErrUnknownPublicKeyAlgorithm = errors.New("staking: unknown public key algorithm") +) // ParseCertificate parses a single certificate from the given ASN.1 DER data. +// +// TODO: Remove after v1.11.x activates. func ParseCertificate(der []byte) (*Certificate, error) { - cert, err := x509.ParseCertificate(der) + x509Cert, err := x509.ParseCertificate(der) if err != nil { return nil, err } - return CertificateFromX509(cert), nil + stakingCert := CertificateFromX509(x509Cert) + return stakingCert, ValidateCertificate(stakingCert) +} + +// ParseCertificatePermissive parses a single certificate from the given ASN.1. +// +// This function does not validate that the certificate is valid to be used +// against normal TLS implementations. +// +// Ref: https://github.com/golang/go/blob/go1.19.12/src/crypto/x509/parser.go#L789-L968 +func ParseCertificatePermissive(bytes []byte) (*Certificate, error) { + if len(bytes) > MaxCertificateLen { + return nil, ErrCertificateTooLarge + } + + input := cryptobyte.String(bytes) + // Consume the length and tag bytes. + if !input.ReadASN1(&input, cryptobyte_asn1.SEQUENCE) { + return nil, ErrMalformedCertificate + } + + // Read the "to be signed" certificate into input. + if !input.ReadASN1(&input, cryptobyte_asn1.SEQUENCE) { + return nil, ErrMalformedTBSCertificate + } + if !input.SkipOptionalASN1(cryptobyte_asn1.Tag(0).Constructed().ContextSpecific()) { + return nil, ErrMalformedVersion + } + if !input.SkipASN1(cryptobyte_asn1.INTEGER) { + return nil, ErrMalformedSerialNumber + } + if !input.SkipASN1(cryptobyte_asn1.SEQUENCE) { + return nil, ErrMalformedSignatureAlgorithmIdentifier + } + if !input.SkipASN1(cryptobyte_asn1.SEQUENCE) { + return nil, ErrMalformedIssuer + } + if !input.SkipASN1(cryptobyte_asn1.SEQUENCE) { + return nil, ErrMalformedValidity + } + if !input.SkipASN1(cryptobyte_asn1.SEQUENCE) { + return nil, ErrMalformedIssuer + } + + // Read the "subject public key info" into input. + if !input.ReadASN1(&input, cryptobyte_asn1.SEQUENCE) { + return nil, ErrMalformedSPKI + } + + // Read the public key algorithm identifier. + var pkAISeq cryptobyte.String + if !input.ReadASN1(&pkAISeq, cryptobyte_asn1.SEQUENCE) { + return nil, ErrMalformedPublicKeyAlgorithmIdentifier + } + var pkAI asn1.ObjectIdentifier + if !pkAISeq.ReadASN1ObjectIdentifier(&pkAI) { + return nil, ErrMalformedOID + } + + // Note: Unlike the x509 package, we require parsing the public key. + + var spk asn1.BitString + if !input.ReadASN1BitString(&spk) { + return nil, ErrMalformedSubjectPublicKey + } + publicKey, signatureAlgorithm, err := parsePublicKey(pkAI, spk) + return &Certificate{ + Raw: bytes, + SignatureAlgorithm: signatureAlgorithm, + PublicKey: publicKey, + }, err +} + +// Ref: https://github.com/golang/go/blob/go1.19.12/src/crypto/x509/parser.go#L215-L306 +func parsePublicKey(oid asn1.ObjectIdentifier, publicKey asn1.BitString) (crypto.PublicKey, x509.SignatureAlgorithm, error) { + der := cryptobyte.String(publicKey.RightAlign()) + switch { + case oid.Equal(oidPublicKeyRSA): + pub := &rsa.PublicKey{N: new(big.Int)} + if !der.ReadASN1(&der, cryptobyte_asn1.SEQUENCE) { + return nil, 0, ErrInvalidRSAPublicKey + } + if !der.ReadASN1Integer(pub.N) { + return nil, 0, ErrInvalidRSAModulus + } + if !der.ReadASN1Integer(&pub.E) { + return nil, 0, ErrInvalidRSAPublicExponent + } + + if pub.N.Sign() <= 0 { + return nil, 0, ErrRSAModulusNotPositive + } + + if bitLen := pub.N.BitLen(); bitLen != allowedRSALargeModulusLen && bitLen != allowedRSASmallModulusLen { + return nil, 0, fmt.Errorf("%w: %d", ErrUnsupportedRSAModulusBitLen, bitLen) + } + if pub.N.Bit(0) == 0 { + return nil, 0, ErrRSAModulusIsEven + } + if pub.E != allowedRSAPublicExponentValue { + return nil, 0, fmt.Errorf("%w: %d", ErrUnsupportedRSAPublicExponent, pub.E) + } + return pub, x509.SHA256WithRSA, nil + case oid.Equal(oidPublicKeyECDSA): + namedCurve := elliptic.P256() + x, y := elliptic.Unmarshal(namedCurve, der) + if x == nil { + return nil, 0, ErrFailedUnmarshallingEllipticCurvePoint + } + return &ecdsa.PublicKey{ + Curve: namedCurve, + X: x, + Y: y, + }, x509.ECDSAWithSHA256, nil + default: + return nil, 0, ErrUnknownPublicKeyAlgorithm + } } diff --git a/staking/parse_test.go b/staking/parse_test.go new file mode 100644 index 000000000000..e9006e4ddcc0 --- /dev/null +++ b/staking/parse_test.go @@ -0,0 +1,89 @@ +// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package staking + +import ( + "testing" + + _ "embed" + + "github.com/stretchr/testify/require" +) + +var ( + //go:embed large_rsa_key.cert + largeRSAKeyCert []byte + + parsers = []struct { + name string + parse func([]byte) (*Certificate, error) + }{ + { + name: "ParseCertificate", + parse: ParseCertificate, + }, + { + name: "ParseCertificatePermissive", + parse: ParseCertificatePermissive, + }, + } +) + +func TestParseCheckLargeCert(t *testing.T) { + for _, parser := range parsers { + t.Run(parser.name, func(t *testing.T) { + _, err := parser.parse(largeRSAKeyCert) + require.ErrorIs(t, err, ErrCertificateTooLarge) + }) + } +} + +func BenchmarkParse(b *testing.B) { + tlsCert, err := NewTLSCert() + require.NoError(b, err) + + bytes := tlsCert.Leaf.Raw + for _, parser := range parsers { + b.Run(parser.name, func(b *testing.B) { + for i := 0; i < b.N; i++ { + _, err = parser.parse(bytes) + require.NoError(b, err) + } + }) + } +} + +func FuzzParseCertificate(f *testing.F) { + tlsCert, err := NewTLSCert() + require.NoError(f, err) + + f.Add(tlsCert.Leaf.Raw) + f.Add(largeRSAKeyCert) + f.Fuzz(func(t *testing.T, certBytes []byte) { + require := require.New(t) + + // Verify that any certificate that can be parsed by ParseCertificate + // can also be parsed by ParseCertificatePermissive. + { + strictCert, err := ParseCertificate(certBytes) + if err == nil { + permissiveCert, err := ParseCertificatePermissive(certBytes) + require.NoError(err) + require.Equal(strictCert, permissiveCert) + } + } + + // Verify that any certificate that can't be parsed by + // ParseCertificatePermissive also can't be parsed by ParseCertificate. + { + cert, err := ParseCertificatePermissive(certBytes) + if err == nil { + require.NoError(ValidateCertificate(cert)) + } else { + _, err = ParseCertificate(certBytes) + require.Error(err) //nolint:forbidigo + } + } + }) +} diff --git a/staking/verify.go b/staking/verify.go index 5fc8d77278e1..dd4255455ff0 100644 --- a/staking/verify.go +++ b/staking/verify.go @@ -11,28 +11,13 @@ import ( "crypto/x509" "errors" "fmt" - - "github.com/ava-labs/avalanchego/utils/units" -) - -// MaxRSAKeyBitLen is the maximum RSA key size in bits that we are willing to -// parse. -// -// https://github.com/golang/go/blob/go1.19.12/src/crypto/tls/handshake_client.go#L860-L862 -const ( - MaxCertificateLen = 16 * units.KiB - MaxRSAKeyByteLen = units.KiB - MaxRSAKeyBitLen = 8 * MaxRSAKeyByteLen ) var ( - ErrCertificateTooLarge = fmt.Errorf("staking: certificate length is greater than %d", MaxCertificateLen) - ErrUnsupportedAlgorithm = errors.New("staking: cannot verify signature: unsupported algorithm") - ErrPublicKeyAlgoMismatch = errors.New("staking: signature algorithm specified different public key type") - ErrInvalidRSAPublicKey = errors.New("staking: invalid RSA public key") - ErrInvalidECDSAPublicKey = errors.New("staking: invalid ECDSA public key") - ErrECDSAVerificationFailure = errors.New("staking: ECDSA verification failure") - ErrED25519VerificationFailure = errors.New("staking: Ed25519 verification failure") + ErrUnsupportedAlgorithm = errors.New("staking: cannot verify signature: unsupported algorithm") + ErrPublicKeyAlgoMismatch = errors.New("staking: signature algorithm specified different public key type") + ErrInvalidECDSAPublicKey = errors.New("staking: invalid ECDSA public key") + ErrECDSAVerificationFailure = errors.New("staking: ECDSA verification failure") ) // CheckSignature verifies that the signature is a valid signature over signed @@ -41,10 +26,6 @@ var ( // Ref: https://github.com/golang/go/blob/go1.19.12/src/crypto/x509/x509.go#L793-L797 // Ref: https://github.com/golang/go/blob/go1.19.12/src/crypto/x509/x509.go#L816-L879 func CheckSignature(cert *Certificate, msg []byte, signature []byte) error { - if err := ValidateCertificate(cert); err != nil { - return err - } - hasher := crypto.SHA256.New() _, err := hasher.Write(msg) if err != nil { @@ -67,6 +48,8 @@ func CheckSignature(cert *Certificate, msg []byte, signature []byte) error { // ValidateCertificate verifies that this certificate conforms to the required // staking format assuming that it was already able to be parsed. +// +// TODO: Remove after v1.11.x activates. func ValidateCertificate(cert *Certificate) error { if len(cert.Raw) > MaxCertificateLen { return ErrCertificateTooLarge @@ -82,8 +65,14 @@ func ValidateCertificate(cert *Certificate) error { if pubkeyAlgo != x509.RSA { return signaturePublicKeyAlgoMismatchError(pubkeyAlgo, pub) } - if bitLen := pub.N.BitLen(); bitLen > MaxRSAKeyBitLen { - return fmt.Errorf("%w: bitLen=%d > maxBitLen=%d", ErrInvalidRSAPublicKey, bitLen, MaxRSAKeyBitLen) + if bitLen := pub.N.BitLen(); bitLen != allowedRSALargeModulusLen && bitLen != allowedRSASmallModulusLen { + return fmt.Errorf("%w: %d", ErrUnsupportedRSAModulusBitLen, bitLen) + } + if pub.N.Bit(0) == 0 { + return ErrRSAModulusIsEven + } + if pub.E != allowedRSAPublicExponentValue { + return fmt.Errorf("%w: %d", ErrUnsupportedRSAPublicExponent, pub.E) } return nil case *ecdsa.PublicKey: diff --git a/staking/verify_test.go b/staking/verify_test.go deleted file mode 100644 index 7fcf0ba8d624..000000000000 --- a/staking/verify_test.go +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package staking - -import ( - "testing" - - _ "embed" - - "github.com/stretchr/testify/require" -) - -var ( - //go:embed large_rsa_key.cert - largeRSAKeyCert []byte - //go:embed large_rsa_key.sig - largeRSAKeySig []byte -) - -func TestCheckSignatureLargePublicKey(t *testing.T) { - require := require.New(t) - - cert, err := ParseCertificate(largeRSAKeyCert) - require.NoError(err) - - msg := []byte("TODO: put something clever") - err = CheckSignature(cert, msg, largeRSAKeySig) - require.ErrorIs(err, ErrInvalidRSAPublicKey) -} diff --git a/vms/proposervm/batched_vm.go b/vms/proposervm/batched_vm.go index ff1ce6a597a0..0bf514827193 100644 --- a/vms/proposervm/batched_vm.go +++ b/vms/proposervm/batched_vm.go @@ -101,7 +101,7 @@ func (vm *VM) BatchedParseBlock(ctx context.Context, blks [][]byte) ([]snowman.B ) for ; blocksIndex < len(blks); blocksIndex++ { blkBytes := blks[blocksIndex] - statelessBlock, err := statelessblock.Parse(blkBytes) + statelessBlock, err := statelessblock.Parse(blkBytes, vm.DurangoTime) if err != nil { break } diff --git a/vms/proposervm/block/block.go b/vms/proposervm/block/block.go index fccbbe2e3718..0f5b374391f7 100644 --- a/vms/proposervm/block/block.go +++ b/vms/proposervm/block/block.go @@ -28,7 +28,7 @@ type Block interface { Block() []byte Bytes() []byte - initialize(bytes []byte) error + initialize(bytes []byte, durangoTime time.Time) error } type SignedBlock interface { @@ -76,7 +76,7 @@ func (b *statelessBlock) Bytes() []byte { return b.bytes } -func (b *statelessBlock) initialize(bytes []byte) error { +func (b *statelessBlock) initialize(bytes []byte, durangoTime time.Time) error { b.bytes = bytes // The serialized form of the block is the unsignedBytes followed by the @@ -91,13 +91,18 @@ func (b *statelessBlock) initialize(bytes []byte) error { return nil } - cert, err := staking.ParseCertificate(b.StatelessBlock.Certificate) + // TODO: Remove durangoTime after v1.11.x has activated. + var err error + if b.timestamp.Before(durangoTime) { + b.cert, err = staking.ParseCertificate(b.StatelessBlock.Certificate) + } else { + b.cert, err = staking.ParseCertificatePermissive(b.StatelessBlock.Certificate) + } if err != nil { return fmt.Errorf("%w: %w", errInvalidCertificate, err) } - b.cert = cert - b.proposer = ids.NodeIDFromCert(cert) + b.proposer = ids.NodeIDFromCert(b.cert) return nil } diff --git a/vms/proposervm/block/build.go b/vms/proposervm/block/build.go index 7baa89205342..b13255c91dd1 100644 --- a/vms/proposervm/block/build.go +++ b/vms/proposervm/block/build.go @@ -35,7 +35,10 @@ func BuildUnsigned( if err != nil { return nil, err } - return block, block.initialize(bytes) + + // Invariant: The durango timestamp isn't used here because the certificate + // is empty. + return block, block.initialize(bytes, time.Time{}) } func Build( @@ -121,5 +124,7 @@ func BuildOption( if err != nil { return nil, err } - return block, block.initialize(bytes) + + // Invariant: The durango timestamp isn't used. + return block, block.initialize(bytes, time.Time{}) } diff --git a/vms/proposervm/block/option.go b/vms/proposervm/block/option.go index c80651b621fc..7edb39bd429f 100644 --- a/vms/proposervm/block/option.go +++ b/vms/proposervm/block/option.go @@ -4,6 +4,8 @@ package block import ( + "time" + "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/utils/hashing" ) @@ -32,7 +34,7 @@ func (b *option) Bytes() []byte { return b.bytes } -func (b *option) initialize(bytes []byte) error { +func (b *option) initialize(bytes []byte, _ time.Time) error { b.id = hashing.ComputeHash256Array(bytes) b.bytes = bytes return nil diff --git a/vms/proposervm/block/parse.go b/vms/proposervm/block/parse.go index cf275134d888..bf9b44adf1f4 100644 --- a/vms/proposervm/block/parse.go +++ b/vms/proposervm/block/parse.go @@ -3,9 +3,12 @@ package block -import "fmt" +import ( + "fmt" + "time" +) -func Parse(bytes []byte) (Block, error) { +func Parse(bytes []byte, durangoTime time.Time) (Block, error) { var block Block parsedVersion, err := Codec.Unmarshal(bytes, &block) if err != nil { @@ -14,7 +17,7 @@ func Parse(bytes []byte) (Block, error) { if parsedVersion != CodecVersion { return nil, fmt.Errorf("expected codec version %d but got %d", CodecVersion, parsedVersion) } - return block, block.initialize(bytes) + return block, block.initialize(bytes, durangoTime) } func ParseHeader(bytes []byte) (Header, error) { diff --git a/vms/proposervm/block/parse_test.go b/vms/proposervm/block/parse_test.go index 1d04271c9fec..148bac82c0a6 100644 --- a/vms/proposervm/block/parse_test.go +++ b/vms/proposervm/block/parse_test.go @@ -43,14 +43,19 @@ func TestParse(t *testing.T) { require.NoError(err) builtBlockBytes := builtBlock.Bytes() - - parsedBlockIntf, err := Parse(builtBlockBytes) - require.NoError(err) - - parsedBlock, ok := parsedBlockIntf.(SignedBlock) - require.True(ok) - - equal(require, chainID, builtBlock, parsedBlock) + durangoTimes := []time.Time{ + timestamp.Add(time.Second), // Durango not activated yet + timestamp.Add(-time.Second), // Durango activated + } + for _, durangoTime := range durangoTimes { + parsedBlockIntf, err := Parse(builtBlockBytes, durangoTime) + require.NoError(err) + + parsedBlock, ok := parsedBlockIntf.(SignedBlock) + require.True(ok) + + equal(require, chainID, builtBlock, parsedBlock) + } } func TestParseDuplicateExtension(t *testing.T) { @@ -60,8 +65,16 @@ func TestParseDuplicateExtension(t *testing.T) { blockBytes, err := hex.DecodeString(blockHex) require.NoError(err) - _, err = Parse(blockBytes) + // Note: The above blockHex specifies 123 as the block's timestamp. + timestamp := time.Unix(123, 0) + durangoNotYetActivatedTime := timestamp.Add(time.Second) + durangoAlreadyActivatedTime := timestamp.Add(-time.Second) + + _, err = Parse(blockBytes, durangoNotYetActivatedTime) require.ErrorIs(err, errInvalidCertificate) + + _, err = Parse(blockBytes, durangoAlreadyActivatedTime) + require.NoError(err) } func TestParseHeader(t *testing.T) { @@ -97,7 +110,7 @@ func TestParseOption(t *testing.T) { builtOptionBytes := builtOption.Bytes() - parsedOption, err := Parse(builtOptionBytes) + parsedOption, err := Parse(builtOptionBytes, time.Time{}) require.NoError(err) equalOption(require, builtOption, parsedOption) @@ -115,14 +128,19 @@ func TestParseUnsigned(t *testing.T) { require.NoError(err) builtBlockBytes := builtBlock.Bytes() - - parsedBlockIntf, err := Parse(builtBlockBytes) - require.NoError(err) - - parsedBlock, ok := parsedBlockIntf.(SignedBlock) - require.True(ok) - - equal(require, ids.Empty, builtBlock, parsedBlock) + durangoTimes := []time.Time{ + timestamp.Add(time.Second), // Durango not activated yet + timestamp.Add(-time.Second), // Durango activated + } + for _, durangoTime := range durangoTimes { + parsedBlockIntf, err := Parse(builtBlockBytes, durangoTime) + require.NoError(err) + + parsedBlock, ok := parsedBlockIntf.(SignedBlock) + require.True(ok) + + equal(require, ids.Empty, builtBlock, parsedBlock) + } } func TestParseGibberish(t *testing.T) { @@ -130,6 +148,6 @@ func TestParseGibberish(t *testing.T) { bytes := []byte{0, 1, 2, 3, 4, 5} - _, err := Parse(bytes) + _, err := Parse(bytes, time.Time{}) require.ErrorIs(err, codec.ErrUnknownVersion) } diff --git a/vms/proposervm/state/block_state.go b/vms/proposervm/state/block_state.go index 88382f0abc67..862d492b925b 100644 --- a/vms/proposervm/state/block_state.go +++ b/vms/proposervm/state/block_state.go @@ -17,6 +17,7 @@ import ( "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/units" "github.com/ava-labs/avalanchego/utils/wrappers" + "github.com/ava-labs/avalanchego/version" "github.com/ava-labs/avalanchego/vms/proposervm/block" ) @@ -109,7 +110,11 @@ func (s *blockState) GetBlock(blkID ids.ID) (block.Block, choices.Status, error) } // The key was in the database - blk, err := block.Parse(blkWrapper.Block) + // + // Invariant: Blocks stored on disk were previously accepted by this node. + // Because the durango activation relaxes TLS cert parsing rules, we assume + // it is always activated here. + blk, err := block.Parse(blkWrapper.Block, version.DefaultUpgradeTime) if err != nil { return nil, choices.Unknown, err } diff --git a/vms/proposervm/vm.go b/vms/proposervm/vm.go index 99dc045be66d..9f77a658b55d 100644 --- a/vms/proposervm/vm.go +++ b/vms/proposervm/vm.go @@ -708,7 +708,7 @@ func (vm *VM) setLastAcceptedMetadata(ctx context.Context) error { } func (vm *VM) parsePostForkBlock(ctx context.Context, b []byte) (PostForkBlock, error) { - statelessBlock, err := statelessblock.Parse(b) + statelessBlock, err := statelessblock.Parse(b, vm.DurangoTime) if err != nil { return nil, err }