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

[BUG] Darwin: Try to use our own certificates failed during GenerateNOCChain - no nocSigner #26240

Closed
robinmo opened this issue Apr 25, 2023 · 23 comments

Comments

@robinmo
Copy link
Contributor

robinmo commented Apr 25, 2023

Reproduction steps

During commissioning process we create MTRDeviceControllerStartupParams through method

- (instancetype)initWithIPK:(NSData *)ipk
         operationalKeypair:(id<MTRKeypair>)operationalKeypair
     operationalCertificate:(MTRCertificateDERBytes)operationalCertificate
    intermediateCertificate:(MTRCertificateDERBytes _Nullable)intermediateCertificate
            rootCertificate:(MTRCertificateDERBytes)rootCertificate;

failed with the log

] <Notice>: Commissioning stage next step: 'ValidateCSR' -> 'GenerateNOCChain'
Apr 21 14:56:45 iPhone Deco(Matter)[12196] <Notice>: Performing next commissioning step 'GenerateNOCChain'
Apr 21 14:56:45 iPhone Deco(Matter)[12196] <Notice>: Getting certificate chain for the device from the issuer
Apr 21 14:56:45 iPhone Deco(Matter)[12196] <Error>: Unable to process Op CSR
Apr 21 14:56:45 iPhone Deco(Matter)[12196] <Error>: Failed to process the certificate signing request
Apr 21 14:56:45 iPhone Deco(Matter)[12196] <Notice>: Error on commissioning step 'GenerateNOCChain': '/Library/Caches/com.apple.xbs/Sources/CHIPFramework/connectedhomeip/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm:89: CHIP Error 0x00000003: Incorrect state'
Apr 21 14:56:45 iPhone Deco(Matter)[12196] <Error>: Failed to perform commissioning step 11
Apr 21 14:56:45 iPhone Deco(Matter)[12196] <Notice>: Going from commissioning step 'GenerateNOCChain' with lastErr = '/Library/Caches/com.apple.xbs/Sources/CHIPFramework/connectedhomeip/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm:89: CHIP Error 0x00000003: Incorrect state' -> 'Cleanup'
Apr 21 14:56:45 iPhone Deco(Matter)[12196] <Notice>: Performing next commissioning step 'Cleanup' with completion status = '/Library/Caches/com.apple.xbs/Sources/CHIPFramework/connectedhomeip/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm:89: CHIP Error 0x00000003: Incorrect state'
Apr 21 14:56:45 iPhone Deco(Matter)[12196] <Notice>: Expiring failsafe on proxy 0x10f929e00

Read the source code and found:
in the init function implementation, forget to set the value for _nocSigner

we try to fix this by add the follow code and it works

- (instancetype)initWithIPK:(NSData *)ipk
         operationalKeypair:(id<MTRKeypair>)operationalKeypair
     operationalCertificate:(MTRCertificateDERBytes)operationalCertificate
    intermediateCertificate:(MTRCertificateDERBytes _Nullable)intermediateCertificate
            rootCertificate:(MTRCertificateDERBytes)rootCertificate
{
    ```
...
    _ipk = [ipk copy];
// we add this line
    _nocSigner = operationalKeypair;
    NSLog(@"Matter step====_nocSigner = operationalKeypair;");
...
}

Could you please help to fix this issue?

Bug prevalence

Whenever I do this

GitHub hash of the SDK that was being used

9c0ca13 - Fix Linux standalone job. (#23897)

Platform

darwin

Platform Version(s)

iOS 16.2

Anything else?

No response

@bzbarsky-apple
Copy link
Contributor

bzbarsky-apple commented Apr 25, 2023

During commissioning process we create MTRDeviceControllerStartupParams through method

@robinmo You're using the init method meant to be used for the "This controller will not have a way to sign its own certificates" case, which is why it requires you to provide the controller's certificates.

If you then use this controller to commission things, it will not be able to sign a NOC for the commissionee itself, since it has no way to sign certificates. I will adjust the documentation to make this clearer.

You have two options here:

  1. If you do want the controller to issue its own certificates, you should be initializing it via
    - (instancetype)initWithIPK:(NSData *)ipk
                   fabricID:(NSNumber *)fabricID
                  nocSigner:(id<MTRKeypair>)nocSigner;
    
    You can then still provide explicit certificates if you want to, by just setting the relevant fields of the startup params.
  2. If you do not want the controller to issue its own certificates, and want to use your own external CA instead, but you want the controller to commission things, you should init the startup params as you do now, but then set the operationalCertificateIssuer and operationalCertificateIssuerQueue on the startup params, and use that to issue certificates as needed.

@bzbarsky-apple
Copy link
Contributor

@robinmo #26251 to improve the documentation.

@robinmo
Copy link
Contributor Author

robinmo commented Apr 25, 2023

#26251

👍Many thanks, @bzbarsky-apple
I will try that tomorrow.

By the way, as the old day you mentation here

image

we try to use CHIP matter framework below 15.4, and change some code, it can commission sucess. I am a litter curious about what is the demand of this limitation of 15.4 ?

@bzbarsky-apple
Copy link
Contributor

@robinmo I don't know how to answer that question without knowing exactly what "change some code" entailed.

@robinmo
Copy link
Contributor Author

robinmo commented Apr 25, 2023

@robinmo I don't know how to answer that question without knowing exactly what "change some code" entailed.

Wait a moment. I am clone this repo on my personal mac. If done, I will show the code changed.

@robinmo
Copy link
Contributor Author

robinmo commented Apr 25, 2023

@bzbarsky-apple
Tired of the internet speed😪, maybe tomorrow to show the code.

@robinmo
Copy link
Contributor Author

robinmo commented Apr 26, 2023

@bzbarsky-apple
Check.
explicit link with CoreNFC.framework and set status to optional
Matter CoreNFC.framework

@bzbarsky-apple
Copy link
Contributor

OK, but how are you starting your controllers there, etc?

@robinmo
Copy link
Contributor Author

robinmo commented Apr 26, 2023

OK, but how are you starting your controllers there, etc?

@bzbarsky-apple same as the sample code

@bzbarsky-apple
Copy link
Contributor

bzbarsky-apple commented Apr 26, 2023

So you are saying that:

  1. You are using
    - (instancetype)initWithIPK:(NSData *)ipk
             operationalKeypair:(id<MTRKeypair>)operationalKeypair
         operationalCertificate:(MTRCertificateDERBytes)operationalCertificate
        intermediateCertificate:(MTRCertificateDERBytes _Nullable)intermediateCertificate
                rootCertificate:(MTRCertificateDERBytes)rootCertificate;
    
    when creating the controller startup params.
  2. You are then able to commission things using that controller when running on things before 15.4?

If that's not what you are saying, then what exactly are you saying?

@robinmo
Copy link
Contributor Author

robinmo commented Apr 26, 2023

@bzbarsky-apple
yeb, that's it.

@bzbarsky-apple
Copy link
Contributor

@robinmo I don't see how that's physically possible. Commissioning involved handing out an operational certificate, and a controller initialized as above has no way to do that...

@robinmo
Copy link
Contributor Author

robinmo commented Apr 30, 2023 via email

@robinmo
Copy link
Contributor Author

robinmo commented May 4, 2023

@bzbarsky-apple

noc

iShot_2023-05-04_11 04 27

startup params

iShot_2023-05-04_11 04 27

@bzbarsky-apple
Copy link
Contributor

@robinmo That's the same screenshot twice, and apart from showing that you're using the same key for the root and icac (?) I'm not sure what it's supposed to show. I'm also not sure why you're attaching code screenshots instead of just attaching text files....

@robinmo
Copy link
Contributor Author

robinmo commented May 4, 2023

@bzbarsky-apple
Sorry for my inattention.
the second one:

iShot_2023-05-04_11 03 36

As the doc

Prepare to initialize a controller with a complete operational certificate chain. This initialization method should be used when none of the certificate-signing private keys are available locally.

We just want to make our certificates(RAC, ICAC, NOC) in matter ecosystem.

@robinmo
Copy link
Contributor Author

robinmo commented May 4, 2023

@bzbarsky-apple
codes

- (nullable TAUDeviceControllerStartupParams *)_getCertsStartupParamsWithIPK:(NSData *)ipk nocSigner:(id<MTRKeypair>)nocSigner
{
    MTRCertificateDERBytes rac = self.matter.preCommissionInfo.rac;
    MTRCertificateDERBytes icac = self.matter.preCommissionInfo.icac;
    MTRCertificateDERBytes noc = [self signNOCForMatterIoT];
//    NSData *rac = self.matter.preCommissionInfo.rac;
//    NSData *icac = self.matter.preCommissionInfo.icac;
//    NSData *noc = [self signNOCForMatterIoT];
    
    if (noc == nil)
    {
        TPLog(@"Matter step====NOC生成失败❌");
        return nil;
    }
    
#if MATTER_TEST
    // TODO: - test
    NSError *err1 = nil;
    NSError *err2 = nil;
    
    rac = [MTRCertificates createRootCertificate:self.keyPair issuerID:@(self.matter.preCommissionInfo.appNodeId) fabricID:@(self.matter.preCommissionInfo.fabricId)  error:&err1];
    
    if (err1 != nil)
    {
        return nil;
    }
    icac = [MTRCertificates createIntermediateCertificate:self.keyPair rootCertificate:rac intermediatePublicKey:self.keyPair.publicKey issuerID:@(self.matter.preCommissionInfo.appNodeId) fabricID:@(self.matter.preCommissionInfo.fabricId) error:&err2];
    if (err2 != nil)
    {
        return nil;
    }
    
#endif
    
    return [[TAUDeviceControllerStartupParams alloc] initWithIPK:ipk
                                              operationalKeypair:nocSigner
                                          operationalCertificate:noc
                                         intermediateCertificate:icac
                                                 rootCertificate:rac];
//    return [[TAUDeviceControllerStartupParams alloc] initWithOperationalKeypair:nocSigner
//                                                         operationalCertificate:noc
//                                                        intermediateCertificate:icac
//                                                                rootCertificate:rac
//                                                                            ipk:ipk];
   
}

for TAUDeviceControllerStartupParams

@synthesize nocSigner = _nocSigner;

- (instancetype)initWithIPK:(NSData *)ipk
         operationalKeypair:(id<MTRKeypair>)operationalKeypair
     operationalCertificate:(MTRCertificateDERBytes)operationalCertificate
    intermediateCertificate:(MTRCertificateDERBytes _Nullable)intermediateCertificate
            rootCertificate:(MTRCertificateDERBytes)rootCertificate
{
    self = [super initWithIPK:ipk
           operationalKeypair:operationalKeypair
       operationalCertificate:operationalCertificate
      intermediateCertificate:intermediateCertificate
              rootCertificate:rootCertificate];
    if (self)
    {
//        self.nocSigner = operationalKeypair;
        self.keys = operationalKeypair;
        NSLog(@"Matter step====_nocSigner = operationalKeypair;");
    }
    NSLog(@"Matter step====_nocSigner = operationalKeypair;");
    return self;
}

- (id<MTRKeypair>)nocSigner
{
    return self.keys;
}

NOC 
- (nullable MTRCertificateDERBytes)signNOCForMatterIoT
//- (nullable NSData *)signNOCForMatterIoT
{
    
    FabricKeys *keyPair = self.keyPair;
    // TODO: - 从服务器返回的数据
    MTRCertificateDERBytes icac = self.matter.preCommissionInfo.icac;
//    NSData *icac = self.matter.preCommissionInfo.icac;
    NSNumber *fabricID = @(self.matter.preCommissionInfo.fabricId);
    NSNumber *decoNodeID = @(self.matter.preCommissionInfo.deviceNodeId);
    
    
#if MATTER_TEST
    MTRLog(@"使用模拟数据❌❌❌❌❌❌")
    MTRCertificateDERBytes rac;
    
    // TODO: - test
    NSError *err1 = nil;
    NSError *err2 = nil;
    
    rac = [MTRCertificates createRootCertificate:self.keyPair issuerID:@(self.matter.preCommissionInfo.appNodeId) fabricID:@(self.matter.preCommissionInfo.fabricId)  error:&err1];
    
    if (err1 != nil)
    {
        TPLog(@"Matter step====RAC生成失败");
        return nil;
    }
    icac = [MTRCertificates createIntermediateCertificate:self.keyPair rootCertificate:rac intermediatePublicKey:self.keyPair.publicKey issuerID:@(self.matter.preCommissionInfo.appNodeId) fabricID:@(self.matter.preCommissionInfo.fabricId) error:&err2];
    if (err2 != nil)
    {
        TPLog(@"Matter step====ICAC生成失败");
        return nil;
    }
#endif
    
    // TODO: - 具体作用待查
//    NSArray *caseAuthenticatedTags = nil;
    NSSet *caseAuthenticatedTags = nil;
    
    NSError *error = nil;
    // nodeID为admin权限角色的node id这里传Deco的
    MTRCertificateDERBytes NOC = [MTRCertificates createOperationalCertificate:keyPair
                                                            signingCertificate:icac
                                                          operationalPublicKey:keyPair.publicKey
                                                                      fabricID:fabricID
                                                                        nodeID:decoNodeID
                                                         caseAuthenticatedTags:caseAuthenticatedTags
                                                                         error:&error];
//    NSData *NOC = [MTRCertificates generateOperationalCertificate:keyPair
//                                               signingCertificate:icac
//                                             operationalPublicKey:keyPair.publicKey
//                                                         fabricId:fabricID
//                                                           nodeId:decoNodeID
//                                            caseAuthenticatedTags:caseAuthenticatedTags
//                                                            error:&error];
    if (error != nil)
    {
        MTRLog(@"签发IOT NOC失败:%@", error);
        return nil;
    }
    return NOC;
}

@bzbarsky-apple
Copy link
Contributor

I had commented here, but that comment seems to be missing... this bit:

- (id<MTRKeypair>)nocSigner
{
    return self.keys;
```

where `self.keys` are the _operational_ keys in the `TAUDeviceControllerStartupParams` is why things were "working", but that's just the same incorrect code as is being proposed in #26247.  So its sounds like the only reason things "worked" is that `TAUDeviceControllerStartupParams` was being used instead of `MTRDeviceControllerStartupParams`, and it has that incorrect code.

@robinmo
Copy link
Contributor Author

robinmo commented May 9, 2023

I had commented here, but that comment seems to be missing... this bit:

- (id<MTRKeypair>)nocSigner
{
    return self.keys;

where self.keys are the operational keys in the TAUDeviceControllerStartupParams is why things were "working", but that's just the same incorrect code as is being proposed in #26247. So its sounds like the only reason things "worked" is that TAUDeviceControllerStartupParams was being used instead of MTRDeviceControllerStartupParams, and it has that incorrect code.

@bzbarsky-apple
I couldn't really get the point what is the correct way. Could you pls show how to use full certificates chain(to use our own rac, icac, noc).
Additional, I found the noc valid days is 3652460*60, what could happlen when the noc deprecated?

@bzbarsky-apple
Copy link
Contributor

Could you pls show how to use full certificates chain(to use our own rac, icac, noc).

See #26240 (comment)

Additional, I found the noc valid days is 3652460*60, what could happlen when the noc deprecated?

Things stop working. You need to update the NOC before then. See also #26403

@robinmo
Copy link
Contributor Author

robinmo commented May 10, 2023

@bzbarsky-apple

- (instancetype)initWithIPK:(NSData *)ipk
               fabricID:(NSNumber *)fabricID
              nocSigner:(id<MTRKeypair>)nocSigner;

This API only contains ipk, fabricID, nocSigner, ipk is a hardcode value in sample app, and nocSigner is keypairs, these are non of business with RAC. How could it involved with full ceritificates chain.

Another question about MTRUnpairDeviceWithID:

void MTRUnpairDeviceWithID(uint64_t deviceId)
{
    MTRSetDevicePaired(deviceId, NO);
    MTRGetConnectedDeviceWithID(deviceId, ^(MTRBaseDevice * _Nullable device, NSError * _Nullable error) {
        if (error) {
            NSLog(@"Failed to unpair device %llu still removing from CHIPTool. %@", deviceId, error);
            return;
        }
        NSLog(@"Attempting to unpair device %llu", deviceId);
        MTRBaseClusterOperationalCredentials * opCredsCluster =
            [[MTRBaseClusterOperationalCredentials alloc] initWithDevice:device endpoint:0 queue:dispatch_get_main_queue()];
        [opCredsCluster
            readAttributeCurrentFabricIndexWithCompletionHandler:^(NSNumber * _Nullable value, NSError * _Nullable error) {
                if (error) {
                    NSLog(@"Failed to get current fabric index for device %llu still removing from CHIPTool. %@", deviceId, error);
                    return;
                }
                MTROperationalCredentialsClusterRemoveFabricParams * params =
                    [[MTROperationalCredentialsClusterRemoveFabricParams alloc] init];
                params.fabricIndex = value;
                [opCredsCluster removeFabricWithParams:params
                                     completionHandler:^(MTROperationalCredentialsClusterNOCResponseParams * _Nullable data,
                                         NSError * _Nullable error) {
                                         if (error) {
                                             NSLog(@"Failed to remove current fabric index %@ for device %llu. %@",
                                                 params.fabricIndex, deviceId, error);
                                             return;
                                         }
                                         NSLog(@"Successfully unpaired deviceId %llu", deviceId);
                                     }];
            }];
    });
}

for

[opCredsCluster removeFabricWithParams:params
                                     completionHandler:

This command is used by Administrative Nodes to remove a given fabric index and delete all associated fabric-scoped data.
image

/**
 * Command RemoveFabric
 *
 * This command is used by Administrative Nodes to remove a given fabric index and **delete all associated fabric-scoped data**.
 */
- (void)removeFabricWithParams:(MTROperationalCredentialsClusterRemoveFabricParams *)params
                    completion:(void (^)(MTROperationalCredentialsClusterNOCResponseParams * _Nullable data,
                                   NSError * _Nullable error))completion MTR_NEWLY_AVAILABLE;

Is it the right way to unpair device?

@bzbarsky-apple
Copy link
Contributor

bzbarsky-apple commented May 10, 2023

How could it involved with full ceritificates chain.

The certificate properties on the startup params are writable. You init the startup params (which only requires the smallest set of values that might be needed to get proper functioning), then you can set those certificate properties to whatever you want. If you set them in a way that is not compatible with the nocSigner you passed in, you will get an error, of course.

I did clearly say "You can then still provide explicit certificates if you want to, by just setting the relevant fields of the startup params." above, no?

Is it the right way to unpair device?

Well, if by "unpair" you mean "remove my fabric from the device", then yes: you get the fabric index the device is using for your fabric and then you RemoveFabric that index.

@robinmo
Copy link
Contributor Author

robinmo commented May 10, 2023

@bzbarsky-apple

I did clearly say "You can then still provide explicit certificates if you want to, by just setting the relevant fields of the startup params." above, no?

Got it. Sorry for ignoring that and many thanks for your reply👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment