fix: resolve local LMS enforcement issue for AMT19 above#1257
Conversation
Added LMS certificate check for loopback connections. Return local LMS TLS status to AMT cloud/console. Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
nbmaiti
force-pushed
the
lms_local_tls_amt19
branch
from
cb31c4f to
d6305b7
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } else { | ||
| // default tls config if device is in ACM or CCM | ||
| log.Trace("Setting default TLS Config for ACM/CCM mode") | ||
| if skipAMTCertCheck { | ||
| log.Trace("Skipping AMT certificate verification for ACM/CCM mode (loopback TLS)") | ||
| } else { |
There was a problem hiding this comment.
In GetTLSConfig(), the ACM/CCM branch leaves default TLS verification enabled (InsecureSkipVerify stays false when skipAMTCertCheck is false) and does not set VerifyPeerCertificate. For AMT 19+ loopback TLS where LMS presents a self-signed/untrusted certificate, this will still fail the handshake. Consider always enabling InsecureSkipVerify for loopback and performing certificate validation via VerifyPeerCertificate/VerifyCertificates for all modes when skipAMTCertCheck is false.
|
|
||
| if !bytes.Equal(cert.RawSubject, cert.RawIssuer) { | ||
| return errors.New("single AMT loopback certificate is not self-signed") | ||
| } |
There was a problem hiding this comment.
The self-signed check compares cert.RawSubject and cert.RawIssuer, which is not sufficient to prove the certificate is actually self-signed (same DN can still be signed by another key). Use a signature check (e.g., cert.CheckSignatureFrom(cert)) before treating it as self-signed.
| // Propagate local TLS enforcement status detected in AMTBaseCmd.AfterApply | ||
| ctx.LocalTLSEnforced = cmd.LocalTLSEnforced |
There was a problem hiding this comment.
runRemoteActivation propagates LocalTLSEnforced into the shared Context, but Context.ControlMode is still never set and RemoteActivateCmd later forwards it into the RPS request. Propagate the detected control mode as well (e.g., ctx.ControlMode = cmd.GetControlMode()) so RPS/LMS TLS config selection is based on the real AMT mode.
| // Propagate local TLS enforcement status detected in AMTBaseCmd.AfterApply | |
| ctx.LocalTLSEnforced = cmd.LocalTLSEnforced | |
| // Propagate activation state detected in AMTBaseCmd.AfterApply | |
| ctx.LocalTLSEnforced = cmd.LocalTLSEnforced | |
| ctx.ControlMode = cmd.GetControlMode() |
| Verbose: ctx.Verbose, | ||
| SkipCertCheck: ctx.SkipCertCheck, | ||
| SkipAmtCertCheck: ctx.SkipAMTCertCheck, | ||
| LocalTLSEnforced: cmd.LocalTLSEnforced, |
There was a problem hiding this comment.
This RPS request does not populate ControlMode. Since RPS executor uses ControlMode to build the LMS TLS config, leaving it as the zero value can cause incorrect TLS behavior when local TLS is enforced. Set ControlMode from the detected AMT mode (e.g., cmd.GetControlMode() / cmd.ControlMode).
| LocalTLSEnforced: cmd.LocalTLSEnforced, | |
| LocalTLSEnforced: cmd.LocalTLSEnforced, | |
| ControlMode: cmd.GetControlMode(), |
| Verbose: service.context.Verbose, | ||
| SkipCertCheck: service.context.SkipCertCheck, | ||
| SkipAmtCertCheck: service.context.SkipAMTCertCheck, | ||
| LocalTLSEnforced: service.context.LocalTLSEnforced, | ||
| ControlMode: service.context.ControlMode, |
There was a problem hiding this comment.
ControlMode is taken from service.context.ControlMode here, but Context.ControlMode does not appear to be populated anywhere (AMTBaseCmd stores control mode on the command, not the Context). This likely means RPS always receives ControlMode=0; propagate the detected mode into the Context before invoking RemoteActivateCmd, or set ControlMode explicitly when building this request.
This PR addresses activation failures on AMT 19+ where local LMS connections must use TLS and Go’s default TLS verification rejects the AMT self-signed certificate; it also propagates local TLS-enforcement status to RPS/cloud payloads.
Changes:
fixes #1163