Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set digest algorithm to SHA256 to fix the insecure algorithm SHA1-RSA error. #191

Closed
wants to merge 8 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
235 changes: 232 additions & 3 deletions scep/scep.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"crypto/x509"
"encoding/asn1"
"encoding/base64"
"reflect"

"github.com/micromdm/scep/v2/cryptoutil"
"github.com/micromdm/scep/v2/cryptoutil/x509util"
Expand Down Expand Up @@ -222,6 +223,113 @@ type CSRReqMessage struct {
ChallengePassword string
}

// Original implementation without intune hack
// ParsePKIMessage unmarshals a PKCS#7 signed data into a PKI message struct
// func ParsePKIMessage(data []byte, opts ...Option) (*PKIMessage, error) {
// conf := &config{logger: log.NewNopLogger()}
// for _, opt := range opts {
// opt(conf)
// }

// // parse PKCS#7 signed data
// p7, err := pkcs7.Parse(data)
// if err != nil {
// return nil, err
// }

// if len(conf.caCerts) > 0 {
// // According to RFC #2315 Section 9.1, it is valid that the server sends fewer
// // certificates than necessary, if it is expected that those verifying the
// // signatures have an alternate means of obtaining necessary certificates.
// // In SCEP case, an alternate means is to use GetCaCert request.
// // Note: The https://github.com/jscep/jscep implementation logs a warning if
// // no certificates were found for signers in the PKCS #7 received from the
// // server, but the certificates obtained from GetCaCert request are still
// // used for decoding the message.
// p7.Certificates = conf.caCerts
// }

// if err := p7.Verify(); err != nil {
// return nil, err
// }

// var tID TransactionID
// if err := p7.UnmarshalSignedAttribute(oidSCEPtransactionID, &tID); err != nil {
// return nil, err
// }

// var msgType MessageType
// if err := p7.UnmarshalSignedAttribute(oidSCEPmessageType, &msgType); err != nil {
// return nil, err
// }

// msg := &PKIMessage{
// TransactionID: tID,
// MessageType: msgType,
// Raw: data,
// p7: p7,
// logger: conf.logger,
// }

// // log relevant key-values when parsing a pkiMessage.
// logKeyVals := []interface{}{
// "msg", "parsed scep pkiMessage",
// "scep_message_type", msgType,
// "transaction_id", tID,
// }
// level.Debug(msg.logger).Log(logKeyVals...)

// if err := msg.parseMessageType(); err != nil {
// return nil, err
// }

// return msg, nil
// }

// return content length and new offset
func parseLength(bytes []byte, offset int) (int, int, error) {
b := bytes[offset]
offset++
// class := int(b >> 6)
// isCompound := b&0x20 == 0x20
// tag := int(b & 0x1f)
// fmt.Println(class)
// fmt.Println(isCompound)
// fmt.Println(tag)

b = bytes[offset]
offset++
length := 0

if b&0x80 == 0 {
length = int(b & 0x7f)
return length, offset, nil
}

numBytes := int(b & 0x7f)
if numBytes == 0 {
return 0, 0, errors.New("scep: numBytes cannot be 0")
}
for i := 0; i < numBytes; i++ {
if offset >= len(bytes) {
return 0, 0, errors.New("scep: truncated tag or length")
}
b = bytes[offset]
offset++
if length >= 1<<23 {
return 0, 0, errors.New("scep: we can't shift length up without overflowing")
}
length <<= 8
length |= int(b)
if length == 0 {
return 0, 0, errors.New("scep: DER requires that lengths be minimal")
}
}

return length, offset, nil
}

// Implementation with intune hack
// ParsePKIMessage unmarshals a PKCS#7 signed data into a PKI message struct
func ParsePKIMessage(data []byte, opts ...Option) (*PKIMessage, error) {
conf := &config{logger: log.NewNopLogger()}
Expand All @@ -247,6 +355,60 @@ func ParsePKIMessage(data []byte, opts ...Option) (*PKIMessage, error) {
p7.Certificates = conf.caCerts
}

//hack for intune
//we are going to parse the ASN.1 indefinite-length content and replace the p7.content
if len(p7.Content) == 1000 {
// getting the raw bytes of the signed data
v := reflect.ValueOf(*p7)
signedData := v.FieldByName("raw")
unwrappedSignedData := signedData.Elem()
rawContentInfo := unwrappedSignedData.FieldByName("ContentInfo")
content := rawContentInfo.FieldByName("Content")
rawBytes := content.FieldByName("Bytes")
bytes := rawBytes.Bytes()

offset := 0
wrapperLength, offset, err := parseLength(bytes, 0)

if err != nil {
return nil, err
}

//should we apply the hack?
if wrapperLength > 1000+offset {

//parse the real signed data
realData := make([]byte, 0)

totalLength := len(bytes)

for offset < totalLength {
chunkSize, chunkOffset, err := parseLength(bytes, offset)

if chunkSize == 0 {
return nil, errors.New("scep: chunk size should not be 0")
}

offset = chunkOffset + chunkSize

if err != nil {
return nil, err
}

chunkData := make([]byte, chunkSize)

for i := 0; i < chunkSize; i++ {
chunkData[i] = bytes[chunkOffset+i]
}

realData = append(realData, chunkData...)
}

//replace the content
p7.Content = realData
}
}

if err := p7.Verify(); err != nil {
return nil, err
}
Expand Down Expand Up @@ -321,7 +483,7 @@ func (msg *PKIMessage) parseMessageType() error {
}
msg.CertRepMessage = cr
return nil
case PKCSReq, UpdateReq, RenewalReq:
case PKCSReq, UpdateReq, RenewalReq, CertPoll:
var sn SenderNonce
if err := msg.p7.UnmarshalSignedAttribute(oidSCEPsenderNonce, &sn); err != nil {
return err
Expand All @@ -331,7 +493,7 @@ func (msg *PKIMessage) parseMessageType() error {
}
msg.SenderNonce = sn
return nil
case GetCRL, GetCert, CertPoll:
case GetCRL, GetCert:
return errNotImplemented
default:
return errUnknownMessageType
Expand Down Expand Up @@ -380,7 +542,9 @@ func (msg *PKIMessage) DecryptPKIEnvelope(cert *x509.Certificate, key *rsa.Priva
}
logKeyVals = append(logKeyVals, "has_challenge", cp != "")
return nil
case GetCRL, GetCert, CertPoll:
case CertPoll:
return nil
case GetCRL, GetCert:
return errNotImplemented
default:
return errUnknownMessageType
Expand Down Expand Up @@ -422,6 +586,8 @@ func (msg *PKIMessage) Fail(crtAuth *x509.Certificate, keyAuth *rsa.PrivateKey,
return nil, err
}

sd.SetDigestAlgorithm(pkcs7.OIDDigestAlgorithmSHA256)

// sign the attributes
if err := sd.AddSigner(crtAuth, keyAuth, config); err != nil {
return nil, err
Expand All @@ -447,7 +613,65 @@ func (msg *PKIMessage) Fail(crtAuth *x509.Certificate, keyAuth *rsa.PrivateKey,
}

return crepMsg, nil
}

func (msg *PKIMessage) Pending(crtAuth *x509.Certificate, keyAuth *rsa.PrivateKey) (*PKIMessage, error) {
config := pkcs7.SignerInfoConfig{
ExtraSignedAttributes: []pkcs7.Attribute{
{
Type: oidSCEPtransactionID,
Value: msg.TransactionID,
},
{
Type: oidSCEPpkiStatus,
Value: PENDING,
},
{
Type: oidSCEPmessageType,
Value: CertRep,
},
{
Type: oidSCEPsenderNonce,
Value: msg.SenderNonce,
},
{
Type: oidSCEPrecipientNonce,
Value: msg.SenderNonce,
},
},
}

sd, err := pkcs7.NewSignedData(nil)
if err != nil {
return nil, err
}

sd.SetDigestAlgorithm(pkcs7.OIDDigestAlgorithmSHA256)

// sign the attributes
if err := sd.AddSigner(crtAuth, keyAuth, config); err != nil {
return nil, err
}

certRepBytes, err := sd.Finish()
if err != nil {
return nil, err
}

cr := &CertRepMessage{
PKIStatus: PENDING,
RecipientNonce: RecipientNonce(msg.SenderNonce),
}

// create a CertRep message from the original
crepMsg := &PKIMessage{
Raw: certRepBytes,
TransactionID: msg.TransactionID,
MessageType: CertRep,
CertRepMessage: cr,
}

return crepMsg, nil
}

// Success returns a new PKIMessage with CertRep data using an already-issued certificate
Expand Down Expand Up @@ -501,6 +725,9 @@ func (msg *PKIMessage) Success(crtAuth *x509.Certificate, keyAuth *rsa.PrivateKe
if err != nil {
return nil, err
}

signedData.SetDigestAlgorithm(pkcs7.OIDDigestAlgorithmSHA256)

// add the certificate into the signed data type
// this cert must be added before the signedData because the recipient will expect it
// as the first certificate in the array
Expand Down Expand Up @@ -581,6 +808,8 @@ func NewCSRRequest(csr *x509.CertificateRequest, tmpl *PKIMessage, opts ...Optio
return nil, err
}

signedData.SetDigestAlgorithm(pkcs7.OIDDigestAlgorithmSHA256)

// create transaction ID from public key hash
tID, err := newTransactionID(csr.PublicKey)
if err != nil {
Expand Down