Conversation
Checking org rules permission on setup; Added method for getting private keys from EKM;
|
|
what exactly does the error say? there’s an error string and stack trace in
the response
…On Thursday, July 15, 2021, Evgenii Kievsky ***@***.***> wrote:
@tomholub <https://github.com/tomholub>
1. GET <ekm>/v1/keys/private returns 500 right now.
2. Could you help with messages for errors? (please check
Localizable.string updates)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#395 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABQDZEKXOZLEKRYSIGY4W7LTX3N5RANCNFSM5ANREBIQ>
.
--
--
Tom James Holub <http://holub.me/>
|
I mean the errors for checking OrgRules fields. Now I just copy pasted from issue: "organisational_rules_autoimport_or_autogen_with_private_key_manager_error" = "Combination of rules (key_manager_url set but PRV_AUTOIMPORT_OR_AUTOGEN is missing) is not supported on this platform"; |
|
the org rules errors look good
but what about the error 500 on there ekm - please look at the error and
stack trace that is on response json.
On Thursday, July 15, 2021, Evgenii Kievsky ***@***.***>
wrote:
… what exactly does the error say? there’s an error string and stack trace
in the response
… <#m_-962982834939293584_>
On Thursday, July 15, 2021, Evgenii Kievsky *@*.***> wrote: @tomholub
<https://github.com/tomholub> https://github.com/tomholub 1. GET
/v1/keys/private returns 500 right now. 2. Could you help with messages for
errors? (please check Localizable.string updates) — You are receiving this
because you were mentioned. Reply to this email directly, view it on GitHub
<#395 (comment)
<#395 (comment)>>,
or unsubscribe https://github.com/notifications/unsubscribe-auth/
ABQDZEKXOZLEKRYSIGY4W7LTX3N5RANCNFSM5ANREBIQ .
-- -- Tom James Holub http://holub.me/
I mean the errors for checking OrgRules fields. Now I just copy pasted
from issue:
"organisational_rules_autoimport_or_autogen_with_private_key_manager_error"
= "Combination of rules (key_manager_url set but PRV_AUTOIMPORT_OR_AUTOGEN
is missing) is not supported on this platform";
"organisational_rules_autogen_passphrase_quitely_error" = "Combination of
rules (PRV_AUTOIMPORT_OR_AUTOGEN + PASS_PHRASE_QUIET_AUTOGEN) is not
supported on this platform";
"organisational_rules_forbid_storing_passphrase_error" = "Combination of
rules (PRV_AUTOIMPORT_OR_AUTOGEN + missing FORBID_STORING_PASS_PHRASE) is
not supported on this platform";
"organisational_rules_must_submit_attester_error" = "Combination of rules
(PRV_AUTOIMPORT_OR_AUTOGEN + ENFORCE_ATTESTER_SUBMIT) is not supported on
this platform";
"organisational_rules_can_create_keys_error" = "Combination of rules
(PRV_AUTOIMPORT_OR_AUTOGEN + missing NO_PRV_CREATE) is not supported on
this platform";
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#395 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABQDZEO4U5JBOW2BWKDEAWLTX3SLNANCNFSM5ANREBIQ>
.
--
--
Tom James Holub <http://holub.me/>
|
|
|
|
I am not really sure is it really secure to send token through comments in github :/ |
|
got it. looks like you accidentally submitted some http body? there should
be no body
…On Thursday, July 15, 2021, Evgenii Kievsky ***@***.***> wrote:
I am not really sure is it really secure to send token through comments in
github :/
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#395 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABQDZENAHJCE2KJ3KXPK3RDTX4QMLANCNFSM5ANREBIQ>
.
--
--
Tom James Holub <http://holub.me/>
|
|
looks like the token is in wrong format. please send me the token to my
email to check
…On Thursday, July 15, 2021, Evgenii Kievsky ***@***.***> wrote:
I am not really sure is it really secure to send token through comments in
github :/
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#395 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABQDZENAHJCE2KJ3KXPK3RDTX4QMLANCNFSM5ANREBIQ>
.
--
--
Tom James Holub <http://holub.me/>
|
The body is empty |
Expiration of ID tokens is independent of your session. The ones from Google expire in 1 hour by default. That's why you can only use them to pull keys from EKM during the setup flow, and not periodically later - it has to be immediately after login, so they don't have a chance to expire. |
tomholub
left a comment
tomholub
left a comment
There was a problem hiding this comment.
Thank you - notes below
...tionality/Services/Organisational Rules Service/OrganisationalRulesPersmissionsService.swift
Outdated
Show resolved
Hide resolved
FlowCrypt/Functionality/Services/Organisational Rules Service/OrganisationalRulesService.swift
Outdated
Show resolved
Hide resolved
FlowCrypt/Functionality/Services/Organisational Rules Service/OrganisationalRulesService.swift
Outdated
Show resolved
Hide resolved
|
@tomholub Is there an email that will return a key calling /v1/keys/private? |
|
I've sent you email credentials for your company email evgenii@ to your email. @seisvelas please help me log this |
|
I can't really how do I create private key to get it through /v1/keys/private |
|
You already have a keypair set up on your company email. When you call that endpoint with ID Token of that account, it will respond with your keypair. Download the FlowCrypt browser extension and set it up on that account - it will automatically load your private key from the EKM. This is what the iOS app should also do. If you're not sure, in the browser, open the network tab and evaluate the requests between browser extension and the ekm during setup.
|
tomholub
left a comment
tomholub
left a comment
There was a problem hiding this comment.
Thank you - this is what I wanted, looking at the code.
Please update based on comments. Thanks!
| let urlString = urlRequest.url?.absoluteString ?? "??" | ||
| let message = "URLSession.call status:\(status) ms:\(trace.finish()) \(urlMethod) \(urlString)" | ||
| let headers = urlRequest.allHTTPHeaderFields ?? [:] | ||
| let message = "URLSession.call status:\(status) ms:\(trace.finish()) \(urlMethod) \(urlString), headers: \(headers)" |
There was a problem hiding this comment.
I wonder if printing request headers here would leak user credentials somewhere dangerous. @martgil is this a problem on iOS?
There was a problem hiding this comment.
There should not be any printing on production build, we need to check that in our Logger.
There was a problem hiding this comment.
I would highly suggest getting rid of the debug information on a production build. Printing the request headers alone even it contains sensitive information isn't a threat yet. An attacker needs physical access or required a network listener to exploit it. Nevertheless, @ekievsky is correct that there shouldn't be any debug logs on the production build.
There was a problem hiding this comment.
@ekievsky by "should not be printing" did you mean it does not print any in production build, or that it would be bad if it printed?
We'll have to be very clear about this - I'm making an issue to verify that it in fact does not print it in a production build.
FlowCrypt/Functionality/Services/Organisational Rules Service/ClientConfigurationService.swift
Outdated
Show resolved
Hide resolved
FlowCrypt/Functionality/Services/Organisational Rules Service/OrganisationalRulesService.swift
Outdated
Show resolved
Hide resolved
FlowCrypt/Functionality/Services/Organisational Rules Service/OrganisationalRulesService.swift
Outdated
Show resolved
Hide resolved
FlowCrypt/Functionality/Services/Organisational Rules Service/OrganisationalRulesService.swift
Outdated
Show resolved
Hide resolved
…om:FlowCrypt/flowcrypt-ios into feature/issue-385-prepwork-ekm-integration
tomholub
left a comment
tomholub
left a comment
There was a problem hiding this comment.
Please see updated comments. Errors or missing values should not be ignored by substituting them with empty strings. The errors should be dealt with directly.
|
hm, in scope of ClientConfiguration you said that any field of it may be
null and keyManagerUrl is the field of ClientConfiguration. What should we
do in this case?
Handling of errors depends on context that the code runs in.
In this case, keyManagerUrl can’t possibly be null, because it was just
checked to be non null in code that preceded this line. That’s why we are
in this method now - thanks to a non null keyManagerUrl and a bunch of
other preconditions.
If the value now suddenly became null, then something is very, very wrong.
The method was called from the wrong context, or maybe we have some other
hidden bug somewhere. If we wrote our code correctly, a bill here should
never happen. Therefore, if null does happen, i’d crash the app so that
we’re know about it.
Unless there is some possibility in Swift that variable we just checked for
null leading up to this code can suddenly become null on it’s own? Is that
ever possible?
…On Monday, July 19, 2021, Evgenii Kievsky ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In FlowCrypt/Controllers/Setup/SetupInitialViewController.swift
<#395 (comment)>
:
> .then { [weak self] result in
- if result.keys.isNotEmpty {
- self?.showToast(
- "organisational_rules_ekm_private_keys_message".localizeWithArguments(result.keys.count, result.urlString ?? "")
- )
- }
+ let urlString = self?.emailKeyManagerApi.getPrivateKeysUrlString() ?? ""
hm, in scope of ClientConfiguration you said that any field of it may be
null and keyManagerUrl is the field of ClientConfiguration. What should
we do in this case?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#395 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABQDZENUMR25V3G5O7IFY5DTYRZYZANCNFSM5ANREBIQ>
.
--
--
Tom James Holub <http://holub.me/>
|
|
yeah, you are right, we just got the request from back end using this url |
tomholub
left a comment
tomholub
left a comment
There was a problem hiding this comment.
Code looks good - thank you. I'll test this
tomholub
left a comment
tomholub
left a comment
There was a problem hiding this comment.
When I log in with flowcrypt.compatibility account, it tells me that I have a wrong combination of OrgRules key_manager_url set but PRV_AUTOIMPORT_OR_AUTOGEN is missing).
But that is definitely wrong, because that is an account on Gmail domain. Accounts on Gmail domain are supposed to receive default empty OrgRules, just {flags: []}, which does not have key_manager_url set.
FlowCrypt/Functionality/Services/Organisational Rules Service/ClientConfigurationService.swift
Outdated
Show resolved
Hide resolved
FlowCrypt/Functionality/Services/Organisational Rules Service/ClientConfigurationService.swift
Outdated
Show resolved
Hide resolved
4 participants
This PR:
Created org rules permission services.
Checking org rules permission on setup.
Added method for getting private keys from EKM.
close #385
Tests :
To be filled by reviewers
I have reviewed that this PR... (tick whichever items you personally focused on during this review):