Replace old pkcs7 library with mozilla's (#116)

While investigating a pkcs7 issue, and chatting with @groob, it sounded like go.mozilla.org/pkcs7 was the place to be. This reverts the not-usable SCEPEncryptionAlgorithm stuff (#51) and swaps to mozilla's library. **Note:** This likely needs more tests. Especially with the addition of p7.Verify
micromdm · Nov 10, 2020 · f3adbb7 · f3adbb7
1 parent c7e7a9d
commit f3adbb7
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 32 deletions.
diff --git a/cmd/scepclient/csr.go b/cmd/scepclient/csr.go
@@ -20,7 +20,6 @@ const (
 type csrOptions struct {
 	cn, org, country, ou, locality, province, challenge string
 	key                                                 *rsa.PrivateKey
-	sigAlgo                                             x509.SignatureAlgorithm
 }
 
 func loadOrMakeCSR(path string, opts *csrOptions) (*x509.CertificateRequest, error) {
@@ -44,7 +43,7 @@ func loadOrMakeCSR(path string, opts *csrOptions) (*x509.CertificateRequest, err
 	template := x509util.CertificateRequest{
 		CertificateRequest: x509.CertificateRequest{
 			Subject:            subject,
-			SignatureAlgorithm: opts.sigAlgo,
+			SignatureAlgorithm: x509.SHA1WithRSA,
 		},
 	}
 	if opts.challenge != "" {

diff --git a/cmd/scepclient/scepclient.go b/cmd/scepclient/scepclient.go
@@ -14,10 +14,9 @@ import (
 	"strings"
 	"time"
 
-	"github.com/fullsailor/pkcs7"
 	"github.com/go-kit/kit/log"
 	"github.com/go-kit/kit/log/level"
-	"github.com/micromdm/scep/client"
+	scepclient "github.com/micromdm/scep/client"
 	"github.com/micromdm/scep/scep"
 	"github.com/pkg/errors"
 )
@@ -163,17 +162,11 @@ func run(cfg runCfg) error {
 		recipients = r
 	}
 
-	var algo int
-	if client.Supports("AES") || client.Supports("SCEPStandard") {
-		algo = pkcs7.EncryptionAlgorithmAES128GCM
-	}
-
 	tmpl := &scep.PKIMessage{
-		MessageType:             msgType,
-		Recipients:              recipients,
-		SignerKey:               key,
-		SignerCert:              signerCert,
-		SCEPEncryptionAlgorithm: algo,
+		MessageType: msgType,
+		Recipients:  recipients,
+		SignerKey:   key,
+		SignerCert:  signerCert,
 	}
 
 	if cfg.challenge != "" && msgType == scep.PKCSReq {

diff --git a/go.mod b/go.mod
@@ -2,7 +2,6 @@ module github.com/micromdm/scep
 
 require (
 	github.com/boltdb/bolt v1.3.1
-	github.com/fullsailor/pkcs7 v0.0.0-20170716202841-43549d8ee32d
 	github.com/go-kit/kit v0.4.0
 	github.com/go-logfmt/logfmt v0.3.0 // indirect
 	github.com/go-stack/stack v1.6.0 // indirect
@@ -11,8 +10,7 @@ require (
 	github.com/groob/finalizer v0.0.0-20170707115354-4c2ed49aabda
 	github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515 // indirect
 	github.com/pkg/errors v0.8.0
+	go.mozilla.org/pkcs7 v0.0.0-20200128120323-432b2356ecb1
 	golang.org/x/net v0.0.0-20170726083632-f5079bd7f6f7 // indirect
 	golang.org/x/sys v0.0.0-20170728174421-0f826bdd13b5 // indirect
 )
-
-replace github.com/fullsailor/pkcs7 => github.com/groob/pkcs7 v0.0.0-20180824154052-36585635cb64
diff --git a/go.sum b/go.sum
@@ -12,12 +12,12 @@ github.com/gorilla/mux v1.4.0 h1:N6R8isjoRv7IcVVlf0cTBbo0UDc9V6ZXWEm0HQoQmLo=
 github.com/gorilla/mux v1.4.0/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs=
 github.com/groob/finalizer v0.0.0-20170707115354-4c2ed49aabda h1:5ikpG9mYCMFiZX0nkxoV6aU2IpCHPdws3gCNgdZeEV0=
 github.com/groob/finalizer v0.0.0-20170707115354-4c2ed49aabda/go.mod h1:MyndkAZd5rUMdNogn35MWXBX1UiBigrU8eTj8DoAC2c=
-github.com/groob/pkcs7 v0.0.0-20180824154052-36585635cb64 h1:1ALD84dEnUxPKZENhUAeQ0tuJ+s3PuL85pV95B0Ekfk=
-github.com/groob/pkcs7 v0.0.0-20180824154052-36585635cb64/go.mod h1:mEOMQ8C7oeXY3LnE2jy4UkLAqrW9rrpwiP5U4hVV+MY=
 github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515 h1:T+h1c/A9Gawja4Y9mFVWj2vyii2bbUNDw3kt9VxK2EY=
 github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc=
 github.com/pkg/errors v0.8.0 h1:WdK/asTD0HN+q6hsWO3/vpuAkAr+tw6aNJNDFFf0+qw=
 github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
+go.mozilla.org/pkcs7 v0.0.0-20200128120323-432b2356ecb1 h1:A/5uWzF44DlIgdm/PQFwfMkW0JX+cIcQi/SwLAmZP5M=
+go.mozilla.org/pkcs7 v0.0.0-20200128120323-432b2356ecb1/go.mod h1:SNgMg+EgDFwmvSmLRTNKC5fegJjB7v23qTQ0XLGUNHk=
 golang.org/x/net v0.0.0-20170726083632-f5079bd7f6f7 h1:1Pw+ZX4dmGORIwGkTwnUr7RFuMhfpCYHXRZNF04XPYs=
 golang.org/x/net v0.0.0-20170726083632-f5079bd7f6f7/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/sys v0.0.0-20170728174421-0f826bdd13b5 h1:NAjcSWsnFBcOQGn/lxvHouhL7iPC53X8+znVzzQkAEg=

diff --git a/scep/scep.go b/scep/scep.go
@@ -14,10 +14,10 @@ import (
 	"encoding/base64"
 	"math/big"
 
-	"github.com/fullsailor/pkcs7"
 	"github.com/go-kit/kit/log"
 	"github.com/go-kit/kit/log/level"
 	"github.com/pkg/errors"
+	"go.mozilla.org/pkcs7"
 
 	"github.com/micromdm/scep/crypto/x509util"
 )
@@ -175,8 +175,6 @@ type PKIMessage struct {
 	SignerKey  *rsa.PrivateKey
 	SignerCert *x509.Certificate
 
-	SCEPEncryptionAlgorithm int
-
 	logger log.Logger
 }
 
@@ -217,6 +215,10 @@ func ParsePKIMessage(data []byte, opts ...Option) (*PKIMessage, error) {
 		return nil, err
 	}
 
+	if err := p7.Verify(); err != nil {
+		return nil, err
+	}
+
 	var tID TransactionID
 	if err := p7.UnmarshalSignedAttribute(oidSCEPtransactionID, &tID); err != nil {
 		return nil, err
@@ -315,15 +317,8 @@ func (msg *PKIMessage) DecryptPKIEnvelope(cert *x509.Certificate, key *rsa.Priva
 		return err
 	}
 
-	algo, err := p7.EncryptionAlgorithm()
-	if err != nil {
-		return err
-	}
-	msg.SCEPEncryptionAlgorithm = algo
-
 	logKeyVals := []interface{}{
 		"msg", "decrypt pkiEnvelope",
-		"encryption_algorithm", algo,
 	}
 	defer func() { level.Debug(msg.logger).Log(logKeyVals...) }()
 
@@ -450,7 +445,7 @@ func (msg *PKIMessage) SignCSR(crtAuth *x509.Certificate, keyAuth *rsa.PrivateKe
 	}
 
 	// encrypt degenerate data using the original messages recipients
-	e7, err := pkcs7.Encrypt(deg, msg.p7.Certificates, pkcs7.WithEncryptionAlgorithm(msg.SCEPEncryptionAlgorithm))
+	e7, err := pkcs7.Encrypt(deg, msg.p7.Certificates)
 	if err != nil {
 		return nil, err
 	}
@@ -547,7 +542,7 @@ func NewCSRRequest(csr *x509.CertificateRequest, tmpl *PKIMessage, opts ...Optio
 	}
 
 	derBytes := csr.Raw
-	e7, err := pkcs7.Encrypt(derBytes, tmpl.Recipients, pkcs7.WithEncryptionAlgorithm(tmpl.SCEPEncryptionAlgorithm))
+	e7, err := pkcs7.Encrypt(derBytes, tmpl.Recipients)
 	if err != nil {
 		return nil, err
 	}
@@ -571,7 +566,6 @@ func NewCSRRequest(csr *x509.CertificateRequest, tmpl *PKIMessage, opts ...Optio
 	level.Debug(conf.logger).Log(
 		"msg", "creating SCEP CSR request",
 		"transaction_id", tID,
-		"encryption_algorithm", tmpl.SCEPEncryptionAlgorithm,
 		"signer_cn", tmpl.SignerCert.Subject.CommonName,
 	)