openbmc / phosphor-certificate-manager

Apache License 2.0
4 stars 4 forks source link

Generate CSR fails when Challenge Password is given #16

Open lucaspAMI opened 3 years ago

lucaspAMI commented 3 years ago

The Generate CSR fails when given Challenge Password. There is also no information in the Error given to indicate that this is the issue.

trevor-cockrell commented 2 years ago

This is caused by a check in bmcweb, in this file: https://github.com/openbmc/bmcweb/blob/3a2d042432168ad1b555e4fc9f13c2ae0d35e0c7/redfish-core/lib/certificate_service.hpp#L286

                // bmcweb has no way to store or decode a private key challenge
                // password, which will likely cause bmcweb to crash on startup
                // if this is not set on a post so not allowing the user to set
                // value
                if (*optChallengePassword != "")
                {
                    messages::actionParameterNotSupported(
                        asyncResp->res, "GenerateCSR", "ChallengePassword");
                    return;
                }

I'm not sure if the challenge password was misunderstood here (or if I am misunderstanding), but the challenge password should not be used or decoded by bmcweb as it is simply a shared secret between the certificate-issuer and the one generating the CSR. It isn't encoded any more than the rest of a CSR is -- you can easily pull the challenge password with the rest of the data from a CSR.

The way I understand it is that if a certificate is requested to be revoked then the issuer can request confirmation via the previously received challenge password and, once provided the appropriate password, move forward with revoking the certificate.

ojayanth commented 2 years ago

@edtanous

edtanous commented 2 years ago

From the code it looks like you should be getting an ActionParameterNotSupported message in the response. Is that not showing?

Looking at the code snippet above and comment, that does look correct, the OpenBMC implementation doesn’t support a challenge password at this time.

“ you can easily pull the challenge password with the rest of the data from a CSR.”

this statement seems incorrect to me. This would require the bmc to store the password in plaintext, which is counter to modern security principals. While an argument could be made that this is ok, that would need to be done as part of a patchset to enable this as a feature.

unless the error message isn’t propagating (in which case we need to sort that out) this doesn’t look like a bug to me, but a missing feature, and more importantly, a missing piece in the security model for how the bmc can store secrets like this.

trevor-cockrell commented 2 years ago

@edtanous the info I added here is based on the PKCS spec for ChallengePassword: https://datatracker.ietf.org/doc/html/rfc2985 in section 5.4.1.

  5.4.1 Challenge password

   The challengePassword attribute type specifies a password by which an
   entity may request certificate revocation.  The interpretation of
   challenge passwords is intended to be specified by certificate
   issuers etc; no particular interpretation is required.

   challengePassword ATTRIBUTE ::= {
           WITH SYNTAX DirectoryString {pkcs-9-ub-challengePassword}
           EQUALITY MATCHING RULE caseExactMatch
           SINGLE VALUE TRUE
           ID pkcs-9-at-challengePassword
   }

   A challenge-password attribute must have a single attribute value.

   ChallengePassword attribute values generated in accordance with this
   version of this document SHOULD use the PrintableString encoding
   whenever possible.  If internationalization issues make this
   impossible, the UTF8String alternative SHOULD be used.  PKCS #9-
   attribute processing systems MUST be able to recognize and process
   all string types in DirectoryString values.

   Note - Version 1.1 of this document defined challengePassword as
   having the syntax CHOICE {PrintableString, T61String}, but did
   contain a note explaining that this might be changed to a CHOICE of
   different string types in the future See also Note 2 in section
   5.2.3.

Additionally, using the current implementation of OpenBMC and OpenSSL with the DBus interface (to generate a test LDAP CSR for example):

busctl call xyz.openbmc_project.Certs.Manager.Client.Ldap /xyz/openbmc_project/certs/client/ldap xyz.openbmc_project.Certs.CSR.Create GenerateCSR "asssssssssxssassssss" \
"1" \
 "alt_name_1" \
"TESTPASS" \
"CityHere!" \
"IP_Here!" \
"" \
"US" \
"" \
"" \
"" \
"2048" \
"" \
"RSA" \
0 \
"OrgHere!" \
"OrgUnitHere!" \
"StateHere!" \
"" \
""

And then:

busctl call xyz.openbmc_project.Certs.Manager.Client.Ldap /xyz/openbmc_project/certs/client/ldap/csr xyz.openbmc_project.Certs.CSR CSR

returns

"-----BEGIN CERTIFICATE REQUEST-----\nMIIC3zCCAccCAQEwgZkxEzARBgNVHREMCmFsdF9uYW1lXzExFzAVBgkqhkiG9w0B\nCQcMCFRFU1RQQVNTMRIwEAYDVQQHDAlDaXR5SGVyZSExETAPBgNVBAMMCElQX0hl\ncmUhMQswCQYDVQQGEwJVUzENMAsGBCsOAwIMA1JTQTERMA8GA1UECgwIT3JnSGVy\nZSExEzARBgNVBAgMClN0YXRlSGVyZSEwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAw\nggEKAoIBAQDJ9SPXPBPh0mLGU7MbVi1RWtZtrFR4pqB86rxQ7aiCIyFwOA9ul42B\nD/f+443qC3pO2E1y+maEEIHbYf/ItfNIg0L5Tw8MZ0TzT3zJPK8J0tlZ9ivL8ICO\nyD73b2Xxm19PgIgCaeIl3FcC8rKIi7ln8kxqdkfM7Ob3BghpEcVwYhd6ipm/hEny\nNR0hplt2Zp66aDdsTgx3LPex1QTUHF9hz9RT2zK+e+1ulCQeq5CHDAvh3UpnLCdE\njnLfEQ7AuNBhhgsBuKtye+6DDLsUiR0kzIuNrBXE8sUVjE32jzSQa3DD+Ndy5JKO\nZJ58svxXIi+XaynN2fe+THFLv9hi09OBAgMBAAGgADANBgkqhkiG9w0BAQsFAAOC\nAQEAfAj7Mpzk/C/l8+/S3SK3P/FuuRzRxrTa+dA2jPXEuog7zdAj8NPFqeZBnuKn\n0Jdd90ffNzrGK6fHbSn2zjrMy/XsaQaWWnjG/sROqbTcjSRi5/wq6mnVF3sKSNFh\nBE4OqIqo0ZZi+vZfFcCidVJBJV5vP3N5wA92Sk6ir650eA8GajzjvPFaiys1bJGf\nfd9r3+6rKpGKEsSoXpsnQd1DEAQmG/FQufs/CgvzSsrGJF9KDcw+MN2+9tRkcEsz\nVe0+sJoRs/rDtL7TkEJcZ8+6Twm0K+xOCjg85OPH1Pk63Mw7imIlA9RhHbaby19R\n8FqjjmqxnYhHTFFsy4RyxhPkRA==\n-----END CERTIFICATE REQUEST-----\n"

Converting \n to actual newlines and uploading to a cert decoding site returns:

Subject
RDN Value
subjectAltName  alt_name_1
challengePassword   TESTPASS
Locality (L)    CityHere!
Common Name (CN)    IP_Here!
Country (C) US
algorithm   RSA
Organization (O)    OrgHere!
State (ST)  StateHere!
Properties
Property    Value
Subject ST = StateHere!,O = OrgHere!,algorithm = RSA,C = US,CN = IP_Here!,L = CityHere!,challengePassword = TESTPASS,subjectAltName = alt_name_1
Key Size    2048 bits
Fingerprint (SHA-1) 5B:57:FC:FF:BC:C6:A7:C6:67:A8:33:97:A1:3E:5E:CC:54:5C:32:5D
Fingerprint (MD5)   94:4E:3C:2A:3E:B3:84:76:56:34:93:D9:7B:87:BB:05
SANS    

Which is why I assumed that the challengePassword is allowed to be decoded from the CSR. I see what you mean about storing in plaintext, however.

Further support in the current implementation:

https://github.com/openbmc/phosphor-certificate-manager/blob/6dd1c2ad86507202dd55c533559e253f846e99c8/certs_manager.hpp#L93 Lines 93-94:

     *  @param[in] challengePassword - The challenge password to be applied to
     *      the certificate for revocation requests.

and https://github.com/openbmc/phosphor-certificate-manager/blob/6dd1c2ad86507202dd55c533559e253f846e99c8/certs_manager.cpp#L366

Where the challenge password is added (unless I am mistaken) as an attribute type (challengePassword) to the OpenSSL request.

There's a lot I don't know about certificates, so I've just been going off the specs+current implementation and this is how I understand the issue. Hopefully I'm not way off-base!

trevor-cockrell commented 2 years ago

I think I see where I got off-track. 😄 I read too fast and see that the point being made was to not save the challengePassword as plaintext (even in the context of CSR encoding). I guess my question then is how would a certificate issuer then decode a challenge password in order to authenticate a certificate revoke request?

Either way, shouldn't this also mean that bmcweb wouldn't need to store or decode this challenge since it should only be used by the issuer?

Finally, just confirming that the error message does function as expected:

{
  "error": {
    "@Message.ExtendedInfo": [
      {
        "@odata.type": "#Message.v1_1_1.Message",
        "Message": "The parameter GenerateCSR for the action ChallengePassword is not supported on the target resource.",
        "MessageArgs": [
          "GenerateCSR",
          "ChallengePassword"
        ],
        "MessageId": "Base.1.8.1.ActionParameterNotSupported",
        "MessageSeverity": "Warning",
        "Resolution": "Remove the parameter supplied and resubmit the request if the operation failed."
      }
    ],
    "code": "Base.1.8.1.ActionParameterNotSupported",
    "message": "The parameter GenerateCSR for the action ChallengePassword is not supported on the target resource."
  }
}
edtanous commented 2 years ago

I guess my question then is how would a certificate issuer then decode a challenge password in order to authenticate a certificate revoke request?

Considering OpenBMC doesn't support CRLs at all at this time, I'm going to guess it doesn't. Or I'm not quite understanding what you're getting at.

Like I said before, an argument could definitely be made that storing the challenge password on the BMC is reasonable, and if your security model doesn't allow it, users can opt to not use that API capability to no ill effect. I suspect this bug can be closed, and we can continue this discussion in the context of some patches to enable this use case.